Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Port MASTG test 0019 (by @guardsquare) #3030

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Port MASTG test 0019 (by @guardsquare) #3030

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d3cb11b
Port MASTG test 0019
titze Nov 4, 2024
1f8be5d
Fix markdown
titze Nov 5, 2024
355ea51
Review feedback
titze Nov 5, 2024
4127f2c
Add Frida to trace traffic
titze Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions tests-beta/android/MASVS-NETWORK/MASTG-TEST-0x19-1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
title: HTTP URLs
platform: android
id: MASTG-TEST-0x19-1
type: [static]
weakness: MASWE-0050
---

## Overview

The app should not contain any HTTP URLs which might be used for communicating with a server.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather explain that an app may have hardcoded HTTP URLs in the app binary, libs binaries and other places within the APK. And note that those URLs may be an indication of insecure connections but not always.

Limitations (draft):

  • the test does not determine if the connections are actually made
  • if those connections are actually insecure depends on other factors such as the security configs (NSC, libs configs, etc.).
  • anything else?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the first bullet. I already added a 3rd step to (likely manually) verify that the URLs are actually used for communication.
Otherwise quite a few HTTP URLs probably will show up.


## Steps

1. Reverse engineer the app (@MASTG-TECH-0017).
2. Run a static analysis (@MASTG-TECH-0014) tool and look for any `http://` URLs.
3. Verify the found URLs are actually used for communication.

## Observation

The output contains a list of URLs which are used for communication.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The output contains a list of URLs which are used for communication.
The output contains a list of URLs potentially used for communication.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the 3rd step, they should not be potential anymore. If we show all potential ones, I am not sure the test case is so helpful.


## Evaluation

The test case fails if any HTTP URLs are used for communication.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explain how we'd know that and refer to techniques (traffic trace, hooking networking APIs, static looking for networking APIs and inspecting the code, etc.). You could have an sentence and then some bullet points for those cases with a short explanation for each of them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other comments, I think we have a different understanding of this test :)

25 changes: 25 additions & 0 deletions tests-beta/android/MASVS-NETWORK/MASTG-TEST-0x19-2.md
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a separate test for..?

  • Using low-level APIs (e.g. Socket) to set up a custom HTTP connection.

This could also be a test for https://mas.owasp.org/MASWE/MASVS-NETWORK/MASWE-0049/ maybe we need to make weakness: a list.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I added a draft test for this.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
title: SSLSockets without Hostname Verification
platform: android
id: MASTG-TEST-0x19-2
type: [static]
weakness: MASWE-0050
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't https://mas.owasp.org/MASWE/MASVS-NETWORK/MASWE-0052/ be a better fit?

---

## Overview

`SSLSocket` does not perform hostname verification (see ["Android documentation"](https://developer.android.com/privacy-and-security/security-ssl#WarningsSslSocket)). This needs to be implemented securely by the app itself.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we generalize this test to be "SSLSockets not Properly Verifying Hostnames" or similar we can include a reference to https://developer.android.com/privacy-and-security/risks/unsafe-hostname and shortly explain the cases mentioned when even if the app seems to perform hostname verification (because it uses the related classes/methods) it's doing it incorrectly.

It'd be good to mention the related classes/methods such as getDefaultHostnameVerifier or HostnameVerifier#verify


## Steps

1. Reverse engineer the app (@MASTG-TECH-0017).
2. Run a static analysis (@MASTG-TECH-0014) tool and look for all usages of `SSLSocket`.
3. Verify each usage performans manual hostname verification correctly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After changing the above, we should update the steps and be more specific. We can tell them to search for uses of SSLSocket that are not using HostnameVerifier, or that are using it but potentially incorrectly (as indicated above).

Examples (not for linking here, but maybe to use later for demos):


## Observation

The output contains a list locations where `SSLSocket` is used and if hostname verification is done correctly for each.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The output contains a list locations where `SSLSocket` is used and if hostname verification is done correctly for each.
The output contains a list of locations where `SSLSocket` is used and does not perform hostname verification or does so incorrectly.


## Evaluation

The test case fails if any hostname verification is missing, or implemented insecurely.
cpholguera marked this conversation as resolved.
Show resolved Hide resolved
28 changes: 28 additions & 0 deletions tests-beta/android/MASVS-NETWORK/MASTG-TEST-0x19-3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
title: Cleartext traffic permitted
platform: android
id: MASTG-TEST-0x19-3
type: [static]
weakness: MASWE-0050
---

## Overview

Since Android 9 (API level 28) cleartext HTTP traffic is blocked by default (thanks to the [default Network Security Configuration](../../../Document/0x05g-Testing-Network-Communication.md#default-configurations)) but there are multiple ways in which an application can still send it:

- Setting the [`android:usesCleartextTraffic`](https://developer.android.com/guide/topics/manifest/application-element#usesCleartextTraffic "Android documentation - usesCleartextTraffic flag") attribute of the `<application>` tag in the AndroidManifest.xml file. Note that this flag is ignored in case the Network Security Configuration is configured.
- Configuring the Network Security Configuration to enable cleartext traffic by setting the `cleartextTrafficPermitted` attribute to true on `<domain-config>` elements.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Configuring the Network Security Configuration to enable cleartext traffic by setting the `cleartextTrafficPermitted` attribute to true on `<domain-config>` elements.
- Configuring the [Network Security Configuration to enable cleartext traffic](https://developer.android.com/privacy-and-security/security-config#CleartextTrafficPermitted) by setting the `cleartextTrafficPermitted` attribute to true on `<domain-config>` elements.


## Steps

1. Reverse engineer the app (@MASTG-TECH-0017).
2. Verify `usesCleartextTraffic` is not set to `true` in the AndroidManifest.xml
3. Inspect the AndroidManifest.xml, and check if a `networkSecurityConfig` is set in the `<application>` tag. If yes, inspect the referenced file, and make sure `cleartextTrafficPermitted` is not set to `true` for any domain.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
3. Inspect the AndroidManifest.xml, and check if a `networkSecurityConfig` is set in the `<application>` tag. If yes, inspect the referenced file, and make sure `cleartextTrafficPermitted` is not set to `true` for any domain.
3. Inspect the AndroidManifest.xml, and check if a `networkSecurityConfig` is set in the `<application>` tag. If yes, inspect the referenced file, and make sure that `cleartextTrafficPermitted` is not set to `true`
- globally in the `<base-config>` element.
- or for specific domains in their `<domain-config>` elements.


## Observation

The output contains a list of domains for which cleartext traffic is enabled.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More accurate like this I think.

Suggested change
The output contains a list of domains for which cleartext traffic is enabled.
The output contains a list of configurations allowing for cleartext traffic.


## Evaluation

The test case fails if any cleartext traffic is permitted.
17 changes: 17 additions & 0 deletions tests-beta/android/MASVS-NETWORK/MASTG-TEST-0x19-4.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
title: Cleartext traffic is allowed for cross-platform frameworks
platform: android
id: MASTG-TEST-0x19-4
type: [static]
weakness: MASWE-0050
---

## Overview

Cross-platform frameworks (e.g. Flutter, React native, ...), typically have their own implementations for HTTP libraries, where cleartext traffic can be allowed.

## Steps

## Observation

## Evaluation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this as long as there's a new Issue. Please mention the issue in the PR description.

Suggested change
weakness: MASWE-0050
---
## Overview
Cross-platform frameworks (e.g. Flutter, React native, ...), typically have their own implementations for HTTP libraries, where cleartext traffic can be allowed.
## Steps
## Observation
## Evaluation
weakness: MASWE-0050
status: draft
note: Cross-platform frameworks (e.g. Flutter, React native, ...), typically have their own implementations for HTTP libraries, where cleartext traffic can be allowed.
---

28 changes: 28 additions & 0 deletions tests-beta/android/MASVS-NETWORK/MASTG-TEST-0x19-5.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
title: Cleartext traffic observed
platform: network
id: MASTG-TEST-0x19-5
type: [dynamic]
weakness: MASWE-0050
---

## Overview

Intercept the tested app's incoming and outgoing network traffic and make sure that this traffic is encrypted.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rephrase to highlight more why we're doing this test, what's the value/benefit. For example, this are REAL cleartext connections not just potential ones (as it is the case if we just retrieve the hardcoded URLs).

Limitations:

  • thanks to doing this we can know what domains are contacted in cleartext but we don't know which location in the code is producing this traffic, right? (we can add a note in the Evaluation)
  • this depends on how much of the app you can exercise so you cannot expect full coverage


## Steps

You can use one of the following approaches:
cpholguera marked this conversation as resolved.
Show resolved Hide resolved

- Set up @MASTG-TECH-0010 (for Android) or @MASTG-TECH-0062 (for iOS) to capture all traffic and make sure no communication is done in cleartext.
- Capture all traffic with an interception proxy like @MASTG-TOOL-0077, @MASTG-TOOL-0079, or @MASTG-TOOL-0097 and make sure no request is done in cleartext. Interception proxies like Burp and OWASP ZAP will show HTTP(S) traffic only. You can, however, use a Burp plugin such as [Burp-non-HTTP-Extension](https://github.com/summitt/Burp-Non-HTTP-Extension "Burp-non-HTTP-Extension") or the tool [mitm-relay](https://github.com/jrmdev/mitm_relay "mitm-relay") to decode and visualize communication via XMPP and other protocols.

Note: Some applications may not function correctly with proxies like Burp and OWASP ZAP because of Certificate Pinning. In such a scenario, you can still use the other technique.

## Observation

The output contains a list of cleartext network requests.

## Evaluation

The test case fails if any cleartext requests are logged.
3 changes: 3 additions & 0 deletions tests/android/MASVS-NETWORK/MASTG-TEST-0019.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ title: Testing Data Encryption on the Network
masvs_v1_levels:
- L1
- L2
status: deprecated
covered_by: [MASTG-TEST-0x19-1,MASTG-TEST-0x19-2,,MASTG-TEST-0x19-3,MASTG-TEST-0x19-4,MASTG-TEST-0x19-5]
deprecation_note: New version available in MASTG V2
---

## Overview
Expand Down