Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) #3255

sydseter · 2025-04-16T14:12:07Z

This PR closes #2960.

Overview of additions:

Tests

MASTG-TEST-0234-1: Correct implementation of server certificate verification
Summary: Verifies that any checkServerTrusted override in the app throws a CertificateException on invalid certificates rather than merely logging, ensuring proper TLS certificate validation to prevent MITM attacks.
MASTG-TEST-0234-2: Correct implementation of server hostname verification
Summary: Ensures the app uses HostnameVerifier.verify() (or equivalent) rather than always returning true (e.g. via ALLOW_ALL_HOSTNAME_VERIFIER), so hostname mismatches are caught and connections aren’t silently accepted.
MASTG-TEST-0234-3: Correct use of SSL error handling for webviews
Summary: Checks that WebViewClient.onReceivedSslError handlers do not simply call handler.proceed() without throwing or blocking on SSL errors, thereby avoiding the silent acceptance of invalid certificates in WebViews.
MASTG-TEST-0234-4: Correct use of SSL error handling for webviews
Summary: Validates that the app’s targetSdk is 24 or higher; if it’s below 24 (e.g. 23), the system may allow user-added CAs and the presence of <certificates src="user" /> in the network security config is flagged as insecure.

Demos

MASTG-DEMO-0033: Improper use of checkServerTrusted
Summary: A Kotlin sample that installs an all-trusting X509TrustManager whose checkServerTrusted method only logs warnings (never throws), demonstrating how certificate validation can be disabled.
MASTG-DEMO-0034: Improper use of the HostnameVerifier
Summary: Shows a sample where a custom HostnameVerifier always returns true (and uses the deprecated ALLOW_ALL_HOSTNAME_VERIFIER), illustrating how hostname checks can be bypassed.
MASTG-DEMO-0035: Improper use of the onReceivedSslError handler
Summary: Provides a WebViewClient example overriding onReceivedSslError to log SSL errors but then unconditionally calling handler.proceed(), thereby ignoring TLS errors.
MASTG-DEMO-0036: Targeting API versions that allow the user to use insecure CAs
Summary: Uses a build.gradle.kts with targetSdk = 23 to illustrate that older API levels permit user-imported CAs, potentially trusting insecure certificates.
MASTG-DEMO-0037: Network security config allows certificates imported on the user's behalf
Summary: Contains a network_security_config.xml that includes <certificates src="user" />, demonstrating how the app can be configured to trust user-added CAs.

…pp to reverse engineer and adding demos for the tests.

sydseter · 2025-04-17T14:50:22Z

@cpholguera Have a look when you have the time.

sydseter · 2025-04-17T14:55:59Z

We need to do a renumbering of the tests. I wasn't sure which numbers to give them. These should completely replace MASTG-TEST-0021.

cpholguera · 2025-04-17T21:03:07Z

Awesome! I'll take a look when I'm back from vacation 🌴 Thanks a lot!

…/sydseter/3255

cpholguera

Thank you so much, @sydseter, this is really good! Please take a look at my feedback, and let me know if I can answer any questions.

Review Tip:

Please use the "Commit Suggestions" button to incorporate the suggestions as much as possible, as indicated here. This way, we can easily track the comments and suggestions, and they will be automatically resolved.

cpholguera · 2025-05-14T16:38:59Z

tests-beta/android/MASVS-NETWORK/MASTG-TEST-0234-1.md

+
+You will find two instances of `checkServerTrusted` within the `MainActivity` smali file. There are calls to null checks for each of the parameters `chain` and `authType` as required by the method signature and the invocation of a static log function `w` denoted by `invoke-static`, but there is no indication that a `CertificateException` can be thrown beside an annotation connected to the `checkServerTrusted` method signature.
+
+## Evaluation


Evaluation is missing, "The test fails if ..."

Here you should be specific mentioning the exact APIs, but nothing about smali etc. That's for the demo.

If "just because the app has calls to checkServerTrusted" is not enough to fail and the code must be inspected in some specific (MANUAL) way you can indicate that here. We would like to identify such cases. We have a lot of demos where we can clearly pass or fail them but in this case maybe we need to mark them as "review". So a human will have to look at the provided locations, and inspect the code according to the criteria that you write in here.

cpholguera · 2025-05-14T16:41:07Z

tests-beta/android/MASVS-NETWORK/MASTG-TEST-0234-1.md

+
+## Observation
+
+You will find two instances of `checkServerTrusted` within the `MainActivity` smali file. There are calls to null checks for each of the parameters `chain` and `authType` as required by the method signature and the invocation of a static log function `w` denoted by `invoke-static`, but there is no indication that a `CertificateException` can be thrown beside an annotation connected to the `checkServerTrusted` method signature.


You will find two instances of checkServerTrusted within the MainActivity smali file. There are calls to null checks for each of the parameters chain and authType ...

Something like this sounds very specific to be at the test level. It almost reads as if you're already in a demo and you're looking at the code. We're at a higher (/ abstract) level here.

The observation here simply describes the output of the steps above and starts with "The output contains".

Suggested change

You will find two instances of `checkServerTrusted` within the `MainActivity` smali file. There are calls to null checks for each of the parameters `chain` and `authType` as required by the method signature and the invocation of a static log function `w` denoted by `invoke-static`, but there is no indication that a `CertificateException` can be thrown beside an annotation connected to the `checkServerTrusted` method signature.

The output contains a list of locations where `checkServerTrusted` is used.

cpholguera · 2025-05-16T06:43:08Z

.github/workflows/build-android-demos.yml

We can remove this for now.

cpholguera · 2025-05-16T06:44:13Z

apps/android/MASTG-APP-0018.md

We can remove this, it isn't needed as each demo app is built automatically using the default base apps.

cpholguera · 2025-05-16T09:36:52Z

demos/android/MASVS-NETWORK/MASTG-DEMO-0036/MASTG-DEMO-0036.md

Since we're not creating demos for older versions, we can remove this one completely. For now, we agreed to support "current version - 5":

Currently MASTG min supported Android version: Android 11, API Level 30.

If a demo requires a min/targetSdk version lower than that it won't be added.

The current app has minSdk = 29 so we'll soon update this to 30.

We can keep the test and specify the maximum API level to which it applies.

https://docs.google.com/document/d/1veyzE4cVTSnIsKB1DOPUSMhjXow_MtJOtgHeo5HVoho/edit?tab=t.0#heading=h.iiij6wmkrdl9

cpholguera · 2025-05-17T10:20:21Z

demos/android/MASVS-NETWORK/MASTG-DEMO-0035/MASTG-DEMO-0035.md

+Review each of the reported instances.
+
+- Line 79-92 contains the `onReceivedSslError` method. At the end, on line 91, there is a `handler.proceed()`. There are no exceptions being thrown which means that TLS errors are being ignored.


You could also include this as additional proof.

Add file logcat.txt:

2025-05-17 12:28:31.039 7213-7278 chromium org.owasp.mastestapp E [ERROR:ssl_client_socket_impl.cc(992)] handshake failed; returned -1, SSL error code 1, net_error -201 2025-05-17 12:28:31.048 7213-7213 MastgTestWebView org.owasp.mastestapp E SSL errors onReceivedSslError: The date of the certificate is invalid. 2025-05-17 12:28:31.048 7213-7213 MastgTestWebView org.owasp.mastestapp E primary error: 4 certificate: Issued to: OU=Insubordinate,O=Insecure CA,1.2.840.113549.1.9.1=#161a696e73656375726540746c737265766f636174696f6e2e6f7267,C=NO,ST=Buskerud,CN=tlsexpired.no; Issued by: 1.2.840.113549.1.9.1=#16126a6f68616e4073796473657465722e636f6d,CN=sydseter.com,OU=Sydseter,O=Sydseter,L=Drammen,ST=Drammen,C=NO; on URL: https://tlsexpired.no/

Suggested change

Review each of the reported instances.

- Line 79-92 contains the `onReceivedSslError` method. At the end, on line 91, there is a `handler.proceed()`. There are no exceptions being thrown which means that TLS errors are being ignored.

The test fails because of the presence of the `handler.proceed()` on line 91 in the `onReceivedSslError` method (lines 79-92), as well as the absence of exceptions being thrown.

By doing this, the app is effectively ignoring every TLS error even though we can see that the expired certificate error is logged (see @MASTG-TECH-0009):

{{ logcat.txt }}

cpholguera · 2025-05-17T10:37:04Z

demos/android/MASVS-NETWORK/MASTG-DEMO-0035/MastgTestWebView.kt

+package org.owasp.mastestapp
+
+import android.net.http.SslError
+import android.util.Log
+import android.webkit.SslErrorHandler
+import android.webkit.WebView
+import android.webkit.WebViewClient
+import android.content.Context
+
+class MastgTestWebView (private val context: Context,) {
+
+    fun mastgTest(webView: WebView) {
+        webView.apply {
+            webViewClient = object : WebViewClient() {
+                override fun onReceivedSslError(
+                    view: WebView,
+                    handler: SslErrorHandler,
+                    error: SslError
+                ) {
+                    var message = "SSL Certificate error."
+                    when (error.getPrimaryError()) {
+                        SslError.SSL_UNTRUSTED -> message =
+                            "The certificate authority is not trusted."
+
+                        SslError.SSL_EXPIRED -> message = "The certificate has expired."
+                        SslError.SSL_IDMISMATCH -> message = "The certificate Hostname mismatch."
+                        SslError.SSL_NOTYETVALID -> message = "The certificate is not yet valid."
+                        SslError.SSL_DATE_INVALID -> message =
+                            "The date of the certificate is invalid"
+                    }
+                    Log.w(null, "SSL errors onReceivedSslError: ".plus(message))
+                    Log.w(null, error.toString())
+
+                    handler.proceed()
+                }
+
+            }
+            loadUrl("https://tlsexpired.no")
+        }
+    }
+}


Some highlights:

Extracted the message logic into a getSslErrorMessage(error) helper for better testing output (after running semgrep on the reversed version).

Turned logs into errors with the tag ("MastgTestWebView") and used $ to access message.

Used Kotlin’s property access (error.primaryError) instead of the Java-style error.getPrimaryError().

Suggested change

package org.owasp.mastestapp

import android.net.http.SslError

import android.util.Log

import android.webkit.SslErrorHandler

import android.webkit.WebView

import android.webkit.WebViewClient

import android.content.Context

class MastgTestWebView (private val context: Context,) {

fun mastgTest(webView: WebView) {

webView.apply {

webViewClient = object : WebViewClient() {

override fun onReceivedSslError(

view: WebView,

handler: SslErrorHandler,

error: SslError

) {

var message = "SSL Certificate error."

when (error.getPrimaryError()) {

SslError.SSL_UNTRUSTED -> message =

"The certificate authority is not trusted."

SslError.SSL_EXPIRED -> message = "The certificate has expired."

SslError.SSL_IDMISMATCH -> message = "The certificate Hostname mismatch."

SslError.SSL_NOTYETVALID -> message = "The certificate is not yet valid."

SslError.SSL_DATE_INVALID -> message =

"The date of the certificate is invalid"

}

Log.w(null, "SSL errors onReceivedSslError: ".plus(message))

Log.w(null, error.toString())

handler.proceed()

}

}

loadUrl("https://tlsexpired.no")

}

}

}

package org.owasp.mastestapp

import android.content.Context

import android.net.http.SslError

import android.util.Log

import android.webkit.SslErrorHandler

import android.webkit.WebView

import android.webkit.WebViewClient

class MastgTestWebView(private val context: Context) {

fun mastgTest(webView: WebView) {

webView.apply {

webViewClient = object : WebViewClient() {

override fun onReceivedSslError(

view: WebView,

handler: SslErrorHandler,

error: SslError

) {

val message = getSslErrorMessage(error)

Log.e("MastgTestWebView", "SSL errors onReceivedSslError: $message")

Log.e("MastgTestWebView", error.toString())

handler.proceed()

}

}

loadUrl("https://tlsexpired.no")

}

}

private fun getSslErrorMessage(error: SslError): String = when (error.primaryError) {

SslError.SSL_UNTRUSTED -> "The certificate authority is not trusted."

SslError.SSL_EXPIRED -> "The certificate has expired."

SslError.SSL_IDMISMATCH -> "The certificate Hostname mismatch."

SslError.SSL_NOTYETVALID -> "The certificate is not yet valid."

SslError.SSL_DATE_INVALID -> "The date of the certificate is invalid."

else -> "SSL Certificate error."

}

}

cpholguera · 2025-05-17T11:06:16Z

rules/mastg-android-network-onreceivedsslerror.yml

+    message: Improper use of onReceivedSslError handler
+    match:
+      any:
+        - all:
+          - not: |
+              override fun onReceivedSslError(...) {
+              ...
+              throw $CLASS(...)
+              ...
+          - inside: |
+              override fun onReceivedSslError(...) {
+              ...
+          -  handler.proceed()
+        -  override fun onReceivedSslError(...) { handler.proceed() }


This way it works with the reversed code:

Suggested change

message: Improper use of onReceivedSslError handler

match:

any:

- all:

- not: |

override fun onReceivedSslError(...) {

...

throw $CLASS(...)

...

- inside: |

override fun onReceivedSslError(...) {

...

- handler.proceed()

- override fun onReceivedSslError(...) { handler.proceed() }

message: Improper use of onReceivedSslError handler

patterns:

- pattern: |

$RETURN onReceivedSslError(..., SslErrorHandler $HANDLER, ...) {

...

$HANDLER.proceed();

...

}

- pattern-not-inside: |

$RETURN onReceivedSslError(..., SslErrorHandler $HANDLER, ...) {

...

throw new $EXCEPTION(...);

...

}

cpholguera · 2025-05-17T11:06:56Z

rules/mastg-android-network-checkservertrusted.yml

+        - https://github.com/OWASP/owasp-mastg/blob/master/Document/0x05g-Testing-Network-Communication.md
+      summary: This rule looks for the use of checkServerTrusted and ensures it throws an exception instead of silently muting invalid server certificates
+    message: Improper Server Certificate verification detected.
+    match:


TODO: update to work with the reversed code

cpholguera · 2025-05-17T11:07:06Z

rules/mastg-android-network-hostname-verification.yml

+        - https://github.com/OWASP/owasp-mastg/blob/master/Document/0x05g-Testing-Network-Communication.md#pinning-using-custom-trustmanagers
+      summary: This rule looks for the use of HostnameVerifier and ensure it doesn't just return true which would mean that it mutes all server host name issues
+    message: Improper server hostname verification detected
+    match:


TODO: update to work with the reversed code

sydseter added 11 commits April 16, 2025 16:11

MASTG-TEST-0021-2960 Adding tests to replace MSTG-TEST-0234. Adding a…

f148094

…pp to reverse engineer and adding demos for the tests.

Add rules and demos related to MASTG-TEST-0021

9e8133a

Fix linting.

5ab3ebb

Add remaining test case

c02f65f

Add overview for test case

f149086

Fix demos

6d2305d

Adapt tests

2e4285d

Adapt the demos to work with the MAS app

4ca37c3

Rewrite tests to fit with the MAS app

b68cc60

Add build.gradle file in order to target api 23

d90ab04

Remove cruft from the network_security_config.

04b53c6

sydseter and others added 4 commits April 18, 2025 07:11

Fix the samples

6cfc2ab

Fix the app name

5e06903

Fix the app name

87adce7

Merge branch 'master' of https://github.com/OWASP/owasp-mastg into pr…

1e0e4b1

…/sydseter/3255

cpholguera added the MASTG Refactor label May 15, 2025

cpholguera changed the title ~~MASTG-TEST-0021-2960 Adding tests to replace MSTG-TEST-0234. Adding app to reverse engineer and adding demos for the tests.~~ Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) May 15, 2025

cpholguera self-requested a review May 15, 2025 14:29

cpholguera requested changes May 17, 2025

View reviewed changes

cpholguera added the changes-requested label May 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) #3255

Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) #3255

Uh oh!

sydseter commented Apr 16, 2025 •

edited by cpholguera

Loading

Uh oh!

sydseter commented Apr 17, 2025

Uh oh!

sydseter commented Apr 17, 2025

Uh oh!

cpholguera commented Apr 17, 2025

Uh oh!

cpholguera left a comment

Uh oh!

cpholguera May 14, 2025

Uh oh!

cpholguera May 14, 2025

Uh oh!

cpholguera May 16, 2025

Uh oh!

cpholguera May 16, 2025

Uh oh!

cpholguera May 16, 2025

Uh oh!

cpholguera May 17, 2025

Uh oh!

cpholguera May 17, 2025

Uh oh!

cpholguera May 17, 2025

Uh oh!

cpholguera May 17, 2025

Uh oh!

cpholguera May 17, 2025

Uh oh!

Uh oh!


		You will find two instances of `checkServerTrusted` within the `MainActivity` smali file. There are calls to null checks for each of the parameters `chain` and `authType` as required by the method signature and the invocation of a static log function `w` denoted by `invoke-static`, but there is no indication that a `CertificateException` can be thrown beside an annotation connected to the `checkServerTrusted` method signature.

		## Evaluation


		## Observation

		You will find two instances of `checkServerTrusted` within the `MainActivity` smali file. There are calls to null checks for each of the parameters `chain` and `authType` as required by the method signature and the invocation of a static log function `w` denoted by `invoke-static`, but there is no indication that a `CertificateException` can be thrown beside an annotation connected to the `checkServerTrusted` method signature.

		Review each of the reported instances.

		- Line 79-92 contains the `onReceivedSslError` method. At the end, on line 91, there is a `handler.proceed()`. There are no exceptions being thrown which means that TLS errors are being ignored.

-Review each of the reported instances.
-- Line 79-92 contains the `onReceivedSslError` method. At the end, on line 91, there is a `handler.proceed()`. There are no exceptions being thrown which means that TLS errors are being ignored.
+The test fails because of the presence of the `handler.proceed()` on line 91 in the `onReceivedSslError` method (lines 79-92), as well as the absence of exceptions being thrown.
+By doing this, the app is effectively ignoring every TLS error even though we can see that the expired certificate error is logged (see @MASTG-TECH-0009):
+{{ logcat.txt }}

Uh oh!

Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) #3255

Are you sure you want to change the base?

Port MASTG-TEST-0021: Testing Endpoint Identify Verification (android) #3255

Uh oh!

Conversation

sydseter commented Apr 16, 2025 • edited by cpholguera Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests

Demos

Uh oh!

sydseter commented Apr 17, 2025

Uh oh!

sydseter commented Apr 17, 2025

Uh oh!

cpholguera commented Apr 17, 2025

Uh oh!

cpholguera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sydseter commented Apr 16, 2025 •

edited by cpholguera

Loading