From d896c72e9d5547fd1e76dbed7fa99367338b3c01 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 16 Jan 2025 12:11:17 -0800 Subject: [PATCH 1/3] Check basic integry instead of CTS profile match --- webauthn/registration/formats/android_safetynet.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webauthn/registration/formats/android_safetynet.py b/webauthn/registration/formats/android_safetynet.py index cef6bdf..b37ff31 100644 --- a/webauthn/registration/formats/android_safetynet.py +++ b/webauthn/registration/formats/android_safetynet.py @@ -139,7 +139,7 @@ def verify_android_safetynet( # by following the steps in the SafetyNet online documentation. x5c = [base64url_to_bytes(cert) for cert in header.x5c] - if not payload.cts_profile_match: + if not payload.basic_integrity: raise InvalidRegistrationResponse("Could not verify device integrity (SafetyNet)") if verify_timestamp_ms: From e602e6f7b1b6f53e65c062a840a0aa429c12afa0 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 16 Jan 2025 13:28:11 -0800 Subject: [PATCH 2/3] Update unittests --- ...registration_response_android_safetynet.py | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/tests/test_verify_registration_response_android_safetynet.py b/tests/test_verify_registration_response_android_safetynet.py index 9eaa7f7..2b1fd68 100644 --- a/tests/test_verify_registration_response_android_safetynet.py +++ b/tests/test_verify_registration_response_android_safetynet.py @@ -2,6 +2,8 @@ from unittest.mock import MagicMock, patch from webauthn.helpers import parse_attestation_object, parse_registration_credential_json +from webauthn.helpers.structs import AttestationStatement +from webauthn.helpers.exceptions import InvalidRegistrationResponse from webauthn.registration.formats.android_safetynet import ( verify_android_safetynet, ) @@ -45,3 +47,143 @@ def test_verify_attestation_android_safetynet( ) assert verified is True + + @patch("OpenSSL.crypto.X509StoreContext.verify_certificate") + @patch("base64.b64encode") + @patch("cbor2.loads") + def test_verify_attestation_android_safetynet_basic_integrity_true_cts_profile_match_false( + self, + mock_cbor2_loads: MagicMock, + mock_b64encode: MagicMock, + mock_verify_certificate: MagicMock, + ): + """ + We're not working with a full WebAuthn response here so we have to mock out some values + because all we really want to test is that a response + """ + mock_cbor2_loads.return_value = {"authData": bytes()} + mock_b64encode.return_value = "3N7YJmISsFM0cdvMAYcHcw==".encode("utf-8") + mock_verify_certificate.return_value = True + + # basicIntegrity: True, ctsProfileMatch: False + jws_result_only_fail_cts_check = ( + "eyJ4NWMiOiBbIk1JSURXekNDQWtNQ0FRb3dEUVlKS29aSWh2Y05BUUVMQlFBd2NqRUxNQWtHQT" + "FVRUJoTUNWVk14Q3pBSkJnTlZCQWdNQWsxSk1Rc3dDUVlEVlFRSERBSkJRVEVVTUJJR0ExVUVD" + "Z3dMUkhWdlUyVmpkWEpwZEhreEdUQVhCZ05WQkFzTUVGTmhabVYwZVU1bGRGUmxjM1JwYm1jeE" + "dEQVdCZ05WQkFNTUQxTmhabVYwZVc1bGRGUmxjM1JEUVRBZUZ3MHhPVEV3TVRneU1ESTJOVEZh" + "RncwME5qQXpNakF5TURJMk5URmFNSFV4Q3pBSkJnTlZCQVlUQWxWVE1Rc3dDUVlEVlFRSURBSk" + "5TVEVMTUFrR0ExVUVCd3dDUVVFeEZEQVNCZ05WQkFvTUMwUjFiMU5sWTNWeWFYUjVNUmt3RndZ" + "RFZRUUxEQkJUWVdabGRIbE9aWFJVWlhOMGFXNW5NUnN3R1FZRFZRUUREQkpoZEhSbGMzUXVZVz" + "VrY205cFpDNWpiMjB3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFR" + "RGNvL0dIbDQzNU8yQkZlTlVhdDJtbi9ZNVdGMk1RQWZiUWxwcVVWc2xVTjlZTnJSV1FYVFlYN3" + "pLTjE3U2RRcmVaU05uZTN2dDVWY0o2ZjZNK1o3NGRTUHpnOVBlN3dSWEVxMVk2aDNEQWVEdGN6" + "VGZGdWdOM3ArRWJhK01LcWkxL29UOHpzUysyL3RzVnpDVTJjNDd5QlQrT1ZRYTBTaUJsRjJlej" + "F3QkQ1VFFJRCt4VjJwOWNmWW5sYzBYSmpnZzFyRGVuR05xUm9zdERqeDJqTWViWG5vK05RM2Zj" + "N21HTHFrb2R0QmZEbWNHcHhxM3c5alBQQy9jbTZTaHlNM2g5ZXR1bzdHbFZVelhuOXQ3djU4RX" + "ZKTWJySkc2MHorT0ZTYTlJbG93U3NYMDlPbzBlaHhYRlpLbklXdisyMGtVNE1IcVZKcDIzeFhi" + "SElXZS9uZndEQWdNQkFBRXdEUVlKS29aSWh2Y05BUUVMQlFBRGdnRUJBRmpzVVJCRXJOMGUwNU" + "JZRlMyRUU3YkZ5VnNVMUhQT0NNZ21TN0s0Nm1OZFdGUXZNT3lzdEdrUkQ3S2YramlxdmF6eWVy" + "NUdVbllSOXZCZVJrVko3TkZmY2gvd05JKzBwaTdUNDk2WmNBM0JKemp4STVxcnROZFBIY1FsNk" + "dLQThHZmQ1ZzNFQ3JUNjhoN1paQ2hJUXlHVUxKTVdwVkljL3dPT1FqNTNieEZQRTJ0U0VWQlhp" + "SUx6Tnh4NkxuZUwxaWdaMEZzdVdoU3dnRVArVXA0WFBYN3ZQbXZoczBDb3pUOHNXbG9BOEJzbG" + "dDZGlpeVI5ZThGQTR5RG5VTTFRWnBxMkFNUlBMc3ZJcDVnQndXYVNnejQxaUo0Qk5pOE1rWkJP" + "cklBckV0UzVxYzFIamN4ZklXaURoUjR5MTJqcEhud1Y0ZXpVdHNtVCtwdjFpVUQwUWVzPSJdLC" + "AiYWxnIjogIlJTMjU2In0.eyJ0aW1lc3RhbXBNcyI6IDE1NDM4NDk4NjQ5NzAsICJhZHZpY2Ui" + "OiAiTE9DS19CT09UTE9BREVSIiwgIm5vbmNlIjogIjNON1lKbUlTc0ZNMGNkdk1BWWNIY3c9PS" + "IsICJhcGtQYWNrYWdlTmFtZSI6ICJjb20uZHVvc2VjdXJpdHkuZHVvbW9iaWxlLmRlYnVnIiwg" + "ImFwa0RpZ2VzdFNoYTI1NiI6ICJIVm94ZlBNM1BwYkJaQkRmcWxORGt0Lyt3aXNhTTlrY0Exb2" + "l1NEhabDZJPSIsICJjdHNQcm9maWxlTWF0Y2giOiBmYWxzZSwgImJhc2ljSW50ZWdyaXR5Ijog" + "dHJ1ZSwgImFwa0NlcnRpZmljYXRlRGlnZXN0U2hhMjU2IjogWyJweUVSMGp6cnJWcU1KdW1pUW" + "pGUjdSVS9SdEVLbGkxckxGUEVUMGpPZnlrPSJdfQ.WJhEXK1a2mNycdH_bYYkXhvkADLRsxLaX" + "RzglwYpQXKgHuJap6x1UmWkFiygrgbd6jFfRGqGhifjubgfjHMkrMOJhA723hJNKKvfp-voZYS" + "TILmFsb1LrXjYyaut8V1POWgt3cw4HKfWXgKE2hw-KGkaD9Mrq1vBfXn8LSEkJsv7TyGtkiIbW" + "cYw0wEym7H6CyVFygwyx2B7fVz02Y15IYjz7NuHj3f9OMCScO70mGrvw7BPwaVs4LSNv8zEFOg" + "S2W1MzvpXwq1KMFvrcka7C4t5vyOhMMYwY6BWEnAGcx5_tpJsqegXTgTHSrr4TFQJzsa-H8wb1" + "YaxlMcRVSqOew" + ) + + attestation_statement = AttestationStatement( + ver="0", + response=jws_result_only_fail_cts_check.encode("ascii"), + ) + + verified = verify_android_safetynet( + attestation_statement=attestation_statement, + attestation_object=bytes(), + client_data_json=bytes(), + pem_root_certs_bytes=[], + verify_timestamp_ms=False, + ) + + assert verified is True + + @patch("OpenSSL.crypto.X509StoreContext.verify_certificate") + @patch("base64.b64encode") + @patch("cbor2.loads") + def test_raise_attestation_android_safetynet_basic_integrity_false_cts_profile_match_false( + self, + mock_cbor2_loads: MagicMock, + mock_b64encode: MagicMock, + mock_verify_certificate: MagicMock, + ): + """ + We're not working with a full WebAuthn response here so we have to mock out some values + because all we really want to test is that a response + """ + mock_cbor2_loads.return_value = {"authData": bytes()} + mock_b64encode.return_value = "NumMA+QH27ik6Mu737RgWg==".encode("utf-8") + mock_verify_certificate.return_value = True + + # basicIntegrity: False, ctsProfileMatch: False + jws_result_fail = ( + "eyJ4NWMiOiBbIk1JSURXekNDQWtNQ0FRb3dEUVlKS29aSWh2Y05BUUVMQlFBd2NqRUxNQWtHQT" + "FVRUJoTUNWVk14Q3pBSkJnTlZCQWdNQWsxSk1Rc3dDUVlEVlFRSERBSkJRVEVVTUJJR0ExVUVD" + "Z3dMUkhWdlUyVmpkWEpwZEhreEdUQVhCZ05WQkFzTUVGTmhabVYwZVU1bGRGUmxjM1JwYm1jeE" + "dEQVdCZ05WQkFNTUQxTmhabVYwZVc1bGRGUmxjM1JEUVRBZUZ3MHhPVEV3TVRneU1ESTJOVEZh" + "RncwME5qQXpNakF5TURJMk5URmFNSFV4Q3pBSkJnTlZCQVlUQWxWVE1Rc3dDUVlEVlFRSURBSk" + "5TVEVMTUFrR0ExVUVCd3dDUVVFeEZEQVNCZ05WQkFvTUMwUjFiMU5sWTNWeWFYUjVNUmt3RndZ" + "RFZRUUxEQkJUWVdabGRIbE9aWFJVWlhOMGFXNW5NUnN3R1FZRFZRUUREQkpoZEhSbGMzUXVZVz" + "VrY205cFpDNWpiMjB3Z2dFaU1BMEdDU3FHU0liM0RRRUJBUVVBQTRJQkR3QXdnZ0VLQW9JQkFR" + "RGNvL0dIbDQzNU8yQkZlTlVhdDJtbi9ZNVdGMk1RQWZiUWxwcVVWc2xVTjlZTnJSV1FYVFlYN3" + "pLTjE3U2RRcmVaU05uZTN2dDVWY0o2ZjZNK1o3NGRTUHpnOVBlN3dSWEVxMVk2aDNEQWVEdGN6" + "VGZGdWdOM3ArRWJhK01LcWkxL29UOHpzUysyL3RzVnpDVTJjNDd5QlQrT1ZRYTBTaUJsRjJlej" + "F3QkQ1VFFJRCt4VjJwOWNmWW5sYzBYSmpnZzFyRGVuR05xUm9zdERqeDJqTWViWG5vK05RM2Zj" + "N21HTHFrb2R0QmZEbWNHcHhxM3c5alBQQy9jbTZTaHlNM2g5ZXR1bzdHbFZVelhuOXQ3djU4RX" + "ZKTWJySkc2MHorT0ZTYTlJbG93U3NYMDlPbzBlaHhYRlpLbklXdisyMGtVNE1IcVZKcDIzeFhi" + "SElXZS9uZndEQWdNQkFBRXdEUVlKS29aSWh2Y05BUUVMQlFBRGdnRUJBRmpzVVJCRXJOMGUwNU" + "JZRlMyRUU3YkZ5VnNVMUhQT0NNZ21TN0s0Nm1OZFdGUXZNT3lzdEdrUkQ3S2YramlxdmF6eWVy" + "NUdVbllSOXZCZVJrVko3TkZmY2gvd05JKzBwaTdUNDk2WmNBM0JKemp4STVxcnROZFBIY1FsNk" + "dLQThHZmQ1ZzNFQ3JUNjhoN1paQ2hJUXlHVUxKTVdwVkljL3dPT1FqNTNieEZQRTJ0U0VWQlhp" + "SUx6Tnh4NkxuZUwxaWdaMEZzdVdoU3dnRVArVXA0WFBYN3ZQbXZoczBDb3pUOHNXbG9BOEJzbG" + "dDZGlpeVI5ZThGQTR5RG5VTTFRWnBxMkFNUlBMc3ZJcDVnQndXYVNnejQxaUo0Qk5pOE1rWkJP" + "cklBckV0UzVxYzFIamN4ZklXaURoUjR5MTJqcEhud1Y0ZXpVdHNtVCtwdjFpVUQwUWVzPSJdLC" + "AiYWxnIjogIlJTMjU2In0.eyJ0aW1lc3RhbXBNcyI6IDE1NDM4NTAzNjAyMTQsICJhZHZpY2Ui" + "OiAiUkVTVE9SRV9UT19GQUNUT1JZX1JPTSIsICJub25jZSI6ICJOdW1NQStRSDI3aWs2TXU3Mz" + "dSZ1dnPT0iLCAiYXBrUGFja2FnZU5hbWUiOiAiY29tLmR1b3NlY3VyaXR5LmR1b21vYmlsZS5k" + "ZWJ1ZyIsICJhcGtEaWdlc3RTaGEyNTYiOiAiYzhFd2NMQUVRNHIycVlzanBDdE9NOUR1QjZyZ0" + "E3WWxjTXBOZm9kSHo0bz0iLCAiY3RzUHJvZmlsZU1hdGNoIjogZmFsc2UsICJiYXNpY0ludGVn" + "cml0eSI6IGZhbHNlLCAiYXBrQ2VydGlmaWNhdGVEaWdlc3RTaGEyNTYiOiBbInB5RVIwanpycl" + "ZxTUp1bWlRakZSN1JVL1J0RUtsaTFyTEZQRVQwak9meWs9Il19.UgwRHy2UMio8eN2Y994Kyzi" + "wqlpzDwRIybYiem4dj8BYWC3Ta48BAR0NN45TDdsGvDGUujVo0LSayfTcgo-vbilz5Y7LWCEgb" + "GoAFhoDDPAMPtthrYTnGDVfhjHTVo00AxsZVgL-HZOD0KecqWcOL8-DWARl3rTAjBWqfon7ZC2" + "IaxzJVrcWtyhPyKdzVB5hJ4NPKIAPlCUkMVUzPY9Xhg1DFLmvaIv8qcZo8xpY0JZDm9cxR1GwP" + "4OVdwMd5seh5483VEpqAmzX7NcZ0aoiMl5PhLGgzHZTrsd1Mc-RZqgc3hAYjnubxONN8vOWGzP" + "gI2Vzgr4VzLOZsWfYwKSR5g" + ) + + attestation_statement = AttestationStatement( + ver="0", + response=jws_result_fail.encode("ascii"), + ) + + with self.assertRaisesRegex( + InvalidRegistrationResponse, + "Could not verify device integrity", + ): + verify_android_safetynet( + attestation_statement=attestation_statement, + attestation_object=bytes(), + client_data_json=bytes(), + pem_root_certs_bytes=[], + verify_timestamp_ms=False, + ) From ed37f0d3fd07ae4de28faf066768c698059ee41e Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 16 Jan 2025 14:05:42 -0800 Subject: [PATCH 3/3] Finish my thoughts --- tests/test_verify_registration_response_android_safetynet.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_verify_registration_response_android_safetynet.py b/tests/test_verify_registration_response_android_safetynet.py index 2b1fd68..72104e7 100644 --- a/tests/test_verify_registration_response_android_safetynet.py +++ b/tests/test_verify_registration_response_android_safetynet.py @@ -59,7 +59,7 @@ def test_verify_attestation_android_safetynet_basic_integrity_true_cts_profile_m ): """ We're not working with a full WebAuthn response here so we have to mock out some values - because all we really want to test is that a response + because all we really want to test is that such a response is allowed through """ mock_cbor2_loads.return_value = {"authData": bytes()} mock_b64encode.return_value = "3N7YJmISsFM0cdvMAYcHcw==".encode("utf-8") @@ -128,7 +128,7 @@ def test_raise_attestation_android_safetynet_basic_integrity_false_cts_profile_m ): """ We're not working with a full WebAuthn response here so we have to mock out some values - because all we really want to test is that a response + because all we really want to test is that a response fails the basicIntegrity check """ mock_cbor2_loads.return_value = {"authData": bytes()} mock_b64encode.return_value = "NumMA+QH27ik6Mu737RgWg==".encode("utf-8")