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

[POC][WIP] Use cryptography cert validation instead of PyOpenSSL #246

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bluetech
Copy link

The cryptography package in version 42 added support for certificate validation. I had in mind to use it in py_webauthn to replace the pyOpenSSL dependency, which seems beneficial to me given py_webauthn already depends on cryptography.

However it turned out that the initial support wasn't sufficient. But in current git (not yet released), cryptography extended the API to allow for custom extension policies, which make it possible to use in py_webauthn. This PR is a proof of concept of this.

Some notes:

  • This will only be usable once cryptography makes the next release. For now, I've used the wheel from here for testing.

  • Even then, the cert verification API is still marked experimental so is subject to change.

  • I haven't looked closely at hardening the verification policy. We should at least match what pyOpenSSL checks. I think that py_webauthn does verify the extensions itself though.

  • A lot of the tests rely on monkeypatching to mock out the verification because the certificates in the test had expired. But the cryptograhpy API accepts a time parameter, allowing to fix the time for the test and have the verification succeed without monkeypatching. I have added a time parameter to the public API (with default now) as it seems very useful to me.

  • There is a problem with one of the tests, tests/test_verify_registration_response_android_safetynet.py::TestVerifyRegistrationResponseAndroidSafetyNet::test_verify_attestation_android_safetynet_basic_integrity_true_cts_profile_match_false. This test is using an X509 v1 certificate, but cryptography only supports v3 certificates. I am not sure if this cert is realistic or was generated just for the test.

Here is the cert:

Details

Certificate:
    Data:
        Version: 1 (0x0)
        Serial Number: 10 (0xa)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C=US, ST=MI, L=AA, O=DuoSecurity, OU=SafetyNetTesting, CN=SafetynetTestCA
        Validity
            Not Before: Oct 18 20:26:51 2019 GMT
            Not After : Mar 20 20:26:51 2046 GMT
        Subject: C=US, ST=MI, L=AA, O=DuoSecurity, OU=SafetyNetTesting, CN=attest.android.com
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:dc:a3:f1:87:97:8d:f9:3b:60:45:78:d5:1a:b7:
                    69:a7:fd:8e:56:17:63:10:01:f6:d0:96:9a:94:56:
                    c9:54:37:d6:0d:ad:15:90:5d:36:17:ef:32:8d:d7:
                    b4:9d:42:b7:99:48:d9:de:de:fb:79:55:c2:7a:7f:
                    a3:3e:67:be:1d:48:fc:e0:f4:f7:bb:c1:15:c4:ab:
                    56:3a:87:70:c0:78:3b:5c:cd:37:c5:ba:03:77:a7:
                    e1:1b:6b:e3:0a:aa:2d:7f:a1:3f:33:b1:2f:b6:fe:
                    db:15:cc:25:36:73:8e:f2:05:3f:8e:55:06:b4:4a:
                    20:65:17:67:b3:d7:00:43:e5:34:08:0f:ec:55:da:
                    9f:5c:7d:89:e5:73:45:c9:8e:08:35:ac:37:a7:18:
                    da:91:a2:cb:43:8f:1d:a3:31:e6:d7:9e:8f:8d:43:
                    77:dc:ee:61:8b:aa:4a:1d:b4:17:c3:99:c1:a9:c6:
                    ad:f0:f6:33:cf:0b:f7:26:e9:28:72:33:78:7d:7a:
                    db:a8:ec:69:55:53:35:e7:f6:de:ef:e7:c1:2f:24:
                    c6:eb:24:6e:b4:cf:e3:85:49:af:48:96:8c:12:b1:
                    7d:3d:3a:8d:1e:87:15:c5:64:a9:c8:5a:ff:b6:d2:
                    45:38:30:7a:95:26:9d:b7:c5:76:c7:21:67:bf:9d:
                    fc:03
                Exponent: 65537 (0x10001)
    Signature Algorithm: sha256WithRSAEncryption
    Signature Value:
        58:ec:51:10:44:ac:dd:1e:d3:90:58:15:2d:84:13:b6:c5:c9:
        5b:14:d4:73:ce:08:c8:26:4b:b2:b8:ea:63:5d:58:54:2f:30:
        ec:ac:b4:69:11:0f:b2:9f:fa:38:aa:bd:ac:f2:7a:be:46:52:
        76:11:f6:f0:5e:46:45:49:ec:d1:5f:72:1f:f0:34:8f:b4:a6:
        2e:d3:e3:de:99:70:0d:c1:27:38:f1:23:9a:ab:b4:d7:4f:1d:
        c4:25:e8:62:80:f0:67:dd:e6:0d:c4:0a:b4:fa:f2:1e:d9:64:
        28:48:43:21:94:2c:93:16:a5:52:1c:ff:03:8e:42:3e:77:6f:
        11:4f:13:6b:52:11:50:57:88:82:f3:37:1c:7a:2e:77:8b:d6:
        28:19:d0:5b:2e:5a:14:b0:80:43:fe:52:9e:17:3d:7e:ef:3e:
        6b:e1:b3:40:a8:cd:3f:2c:5a:5a:00:f0:1b:25:80:27:62:8b:
        24:7d:7b:c1:40:e3:20:e7:50:cd:50:66:9a:b6:00:c4:4f:2e:
        cb:c8:a7:98:01:c1:66:92:83:3e:35:88:9e:01:36:2f:0c:91:
        90:4e:ac:80:2b:12:d4:b9:a9:cd:47:8d:cc:5f:21:68:83:85:
        1e:32:d7:68:e9:1e:7c:15:e1:ec:d4:b6:c9:93:fa:9b:f5:89:
        40:f4:41:eb
-----BEGIN CERTIFICATE-----
MIIDWzCCAkMCAQowDQYJKoZIhvcNAQELBQAwcjELMAkGA1UEBhMCVVMxCzAJBgNV
BAgMAk1JMQswCQYDVQQHDAJBQTEUMBIGA1UECgwLRHVvU2VjdXJpdHkxGTAXBgNV
BAsMEFNhZmV0eU5ldFRlc3RpbmcxGDAWBgNVBAMMD1NhZmV0eW5ldFRlc3RDQTAe
Fw0xOTEwMTgyMDI2NTFaFw00NjAzMjAyMDI2NTFaMHUxCzAJBgNVBAYTAlVTMQsw
CQYDVQQIDAJNSTELMAkGA1UEBwwCQUExFDASBgNVBAoMC0R1b1NlY3VyaXR5MRkw
FwYDVQQLDBBTYWZldHlOZXRUZXN0aW5nMRswGQYDVQQDDBJhdHRlc3QuYW5kcm9p
ZC5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDco/GHl435O2BF
eNUat2mn/Y5WF2MQAfbQlpqUVslUN9YNrRWQXTYX7zKN17SdQreZSNne3vt5VcJ6
f6M+Z74dSPzg9Pe7wRXEq1Y6h3DAeDtczTfFugN3p+Eba+MKqi1/oT8zsS+2/tsV
zCU2c47yBT+OVQa0SiBlF2ez1wBD5TQID+xV2p9cfYnlc0XJjgg1rDenGNqRostD
jx2jMebXno+NQ3fc7mGLqkodtBfDmcGpxq3w9jPPC/cm6ShyM3h9etuo7GlVUzXn
9t7v58EvJMbrJG60z+OFSa9IlowSsX09Oo0ehxXFZKnIWv+20kU4MHqVJp23xXbH
IWe/nfwDAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAFjsURBErN0e05BYFS2EE7bF
yVsU1HPOCMgmS7K46mNdWFQvMOystGkRD7Kf+jiqvazyer5GUnYR9vBeRkVJ7NFf
ch/wNI+0pi7T496ZcA3BJzjxI5qrtNdPHcQl6GKA8Gfd5g3ECrT68h7ZZChIQyGU
LJMWpVIc/wOOQj53bxFPE2tSEVBXiILzNxx6LneL1igZ0FsuWhSwgEP+Up4XPX7v
Pmvhs0CozT8sWloA8BslgCdiiyR9e8FA4yDnUM1QZpq2AMRPLsvIp5gBwWaSgz41
iJ4BNi8MkZBOrIArEtS5qc1HjcxfIWiDhR4y12jpHnwV4ezUtsmT+pv1iUD0Qes=
-----END CERTIFICATE-----

@CLAassistant
Copy link

CLAassistant commented Feb 17, 2025

CLA assistant check
All committers have signed the CLA.

@MasterKale
Copy link
Collaborator

Hey @bluetech sorry to leave you hanging for so long. Thank you for the POC PR! I wonder, would using cryptography for X.509 validation mean this project could drop pyOpenSSL as a dependency? I'm always down to remove dependencies 🤔

A lot of the tests rely on monkeypatching to mock out the verification because the certificates in the test had expired. But the cryptograhpy API accepts a time parameter, allowing to fix the time for the test and have the verification succeed without monkeypatching. I have added a time parameter to the public API (with default now) as it seems very useful to me.

Since you submitted this PR I managed to get rid of a lot of the mock cert validation in PR #247. It required writing a very bespoke decorator for any tests with expired certs, so if cryptography makes that easier too then I'd be interested to look more into this when it gets updated.

There is a problem with one of the tests, tests/test_verify_registration_response_android_safetynet.py::TestVerifyRegistrationResponseAndroidSafetyNet::test_verify_attestation_android_safetynet_basic_integrity_true_cts_profile_match_false. This test is using an X509 v1 certificate, but cryptography only supports v3 certificates. I am not sure if this cert is realistic or was generated just for the test.

iirc that response was generated by an Android dev colleague internally. I had to test against it for #241 because I couldn't generate such responses on my own with the Android devices I have on hand. If this test (and probably test_raise_attestation_android_safetynet_basic_integrity_false_cts_profile_match_false too) then I can ask my colleague how feasible it would be to generate such responses using X.509 v3 format.

@bluetech
Copy link
Author

bluetech commented Mar 8, 2025

Thanks for taking a look.

I wonder, would using cryptography for X.509 validation mean this project could drop pyOpenSSL as a dependency?

Yes!

It required writing a very bespoke decorator for any tests with expired certs, so if cryptography makes that easier too then I'd be interested to look more into this when it gets updated.

Yes, the cryptography API allows passing the current time as a parameter so mocking isn't needed.

If this test (and probably test_raise_attestation_android_safetynet_basic_integrity_false_cts_profile_match_false too) then I can ask my colleague how feasible it would be to generate such responses using X.509 v3 format.

That would be helpful. The cryptography authors indicated they will not add support for non-v3 certificates. I do think the actual certs issued by Google are v3, at least I'd be surprised if not.


I am watching the cryptography repo. I will update the PR once they release this feature.

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

Successfully merging this pull request may close these issues.

3 participants