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.

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

Port MASTG test 0019 (by @guardsquare) #3030

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

Conversation

titze
Copy link
Collaborator

@titze titze commented Nov 4, 2024

Thank you for submitting a Pull Request to the OWASP MASTG. Please make sure that:

  • Your contribution is written in the 2nd person (e.g. you)
  • Your contribution is written in an active present form for as much as possible.
  • You have made sure that the reference section is up to date (e.g. please add sources you have used, make sure that the references to MITRE/MASVS/etc. are up to date)
  • Your contribution has proper formatted markdown and/or code
  • Any references to website have been formatted as [TEXT](URL “NAME”)
  • You verified/tested the effectiveness of your contribution (e.g.: is the code really an effective remediation? Please verify it works!)

If your PR is related to an issue. Please end your PR test with the following line:
This PR closes #2959.

Adds two new draft tests:

  • Using low-level APIs (e.g. Socket) to set up a custom HTTP connection.
  • Cleartext traffic is allowed for cross-platform frameworks

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.

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.

Comment on lines 15 to 17
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):

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.


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.

Comment on lines 6 to 17
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.
---


## 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MASTG v1->v2 MASTG-TEST-0019: Testing Data Encryption on the Network (android)
2 participants