passwordless-lib / fido2-net-lib

FIDO2 .NET library for FIDO2 / WebAuthn Attestation and Assertion using .NET
https://fido2-net-lib.passwordless.dev/
MIT License
1.17k stars 167 forks source link

Inconsistent chain validation on Windows / Linux which results to 2 failed unit tests on Linux #293

Open dbalikhin opened 2 years ago

dbalikhin commented 2 years ago

It works on Windows, but on Linux there is a different behavior that seems actually normal, see below.

Failed tests:

Message:  Fido2NetLib.Fido2VerificationException : Invalid certificate chain

From: https://github.com/passwordless-lib/fido2-net-lib/blob/db931bdba20aadc0743184fc9b0383a88b56ce02/Src/Fido2/CryptoUtils.cs#L107-L118

What happens in ValidateTrustChain:

  1. // try to build a chain with what we've got if (chain.Build(trustPath[0])) We disable revocation flag and allow unknown CA

On Windows we will get success + Status Untrusted Root (A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider.) which is expected. On Linux we will get success + Status Partial Chain (Unable to find local issuer)

  1. // now, verify chain again with all checks turned on if (chain.Build(trustPath[0])) This one will succeed on Windows but fail on Linux. The required item won't be added to CustomTrustStore on Linux, because "chain.ChainElements[^1].Certificate" doesn't contain it.

So we will have something like this:

        if (chain.Build(trustPath[0])) // first chain build
        {
            if (chain.ChainStatus[0].Status == X509ChainStatusFlags.UntrustedRoot)
            {
                // if that validated, we should have a root for this chain now, add it to the custom trust store
                chain.ChainPolicy.CustomTrustStore.Clear();
                chain.ChainPolicy.CustomTrustStore.Add(chain.ChainElements[^1].Certificate);

            }
            else if (chain.ChainStatus[0].Status == X509ChainStatusFlags.PartialChain)
            {
                ???  
                Chain is validated but we won't have a root of this chain. 
            }

.. }

Any ideas on how to handle PartialChain on Linux then?

PartialChain result returned by OpenSSL (which is used under the hood on Linux).

Related discussions: https://github.com/dotnet/runtime/issues/49615#issuecomment-817947393

https://github.com/dotnet/runtime/issues/49615 https://github.com/dotnet/dotnet-api-docs/pull/6660 https://github.com/dotnet/runtime/issues/29164 https://github.com/dotnet/runtime/issues/28314

dbalikhin commented 2 years ago

If it helps, I used WSL2 for debugging on Linux under Win (https://docs.microsoft.com/en-us/visualstudio/test/remote-testing?view=vs-2022#:~:text=Remote%20testing%20enables%20developers%20to,Windows%20or%20Linux%20operating%20systems.)

The pure Linux runner fails the tests as well.

aseigler commented 2 years ago

Yep, you're 100% correct, I can confirm locally. Will figure it out and fix.

aseigler commented 2 years ago

Based on https://github.com/dotnet/runtime/pull/55423, this definitely is known/expected behavior. With the X509ChainTrustMode set to System in the first call to chain.Build() on Windows, the platform uses the authority information access extension in the issuing CA cert to chase down the root and add it to the chain.ChainElements. That doesn't happen on Linux. I'll have to rework this part of the validation.

aseigler commented 2 years ago

Under the covers, this is almost exactly like #284. I think it makes sense to change the terminology from root to trust anchor and have this work from that perspective.

aseigler commented 2 years ago

The above only applies to the TestValidateTrustChainSubAnchor test. The issue with the TestInvalidU2FAttestationASync test is actually a completely different issue entirely, it looks quite a bit like https://github.com/dotnet/runtime/issues/31569 in fact.

Authenticator cert:

-----BEGIN CERTIFICATE-----
MIICAzCCAaigAwIBAgIEAcAAADAKBggqhkjOPQQDAjBiMQswCQYDVQQGEwJTRTES
MBAGA1UECgwJQVRLZXlDQTAwMSIwIAYDVQQLDBlBdXRoZW50aWNhdG9yIEF0dGVz
dGF0aW9uMRswGQYDVQQDExJBdXRoZW50cmVuZCBDQSAwMDAwIBcNMjAwNTEyMTMz
MzIwWhgPMjA2MDA1MDIxMzMzMjBaMGcxCzAJBgNVBAYTAlRXMRQwEgYDVQQKDAtB
dXRoZW5UcmVuZDEiMCAGA1UECwwZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEe
MBwGA1UEAwwVQVRLZXkuUHJvIFNOIDAxQzAwMDAwMFkwEwYHKoZIzj0CAQYIKoZI
zj0DAQcDQgAE02eZbXcVoWahCO1/zHaW8Qi6+QSAw4k3jUKvr8Fwxb75Pgc8Pluj
mUCfc1BfFY7Bj7Fw12kU7GViKbZT1omTIqNFMEMwEwYLKwYBBAGC5RwCAQEEBAQC
BSAwIQYLKwYBBAGC5RwBAQQEEgQQ4alhg1AWTyS1W+OuI2FMxjAJBgNVHRMEAjAA
MAoGCCqGSM49BAMCA0kAMEYCIQCJe3fgns8VFtgIN+pYASCq7ZYklqPjRol83P8o
c5zqRgIhAK+yCOrHj6VCpKcTqdKQ7ZDjLctNg64SjPW093Uhnflo
-----END CERTIFICATE-----

Trust anchor:

-----BEGIN CERTIFICATE-----
MIIBzDCCAXGgAwIBAgIBATAKBggqhkjOPQQDAjBiMQswCQYDVQQGEwJTRTESMBAG
A1UECgwJQVRLZXlDQTAwMSIwIAYDVQQLDBlBdXRoZW50aWNhdG9yIEF0dGVzdGF0
aW9uMRswGQYDVQQDExJBdXRoZW50cmVuZCBDQSAwMDAwIBcNMTYwMjI2MDgxMTA2
WhgPMjA1MDAyMjUwODExMDZaMGIxCzAJBgNVBAYTAlNFMRIwEAYDVQQKDAlBVEtl
eUNBMDAxIjAgBgNVBAsMGUF1dGhlbnRpY2F0b3IgQXR0ZXN0YXRpb24xGzAZBgNV
BAMTEkF1dGhlbnRyZW5kIENBIDAwMDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IA
BAJcWqeCxga9KJbFO2TZdjcgrtZAgfi8TXKu+v5lcR5ceb5GJYxyoCjhueESL3dd
mMIkpGyhsEEtfFUyBwsyFVCjFjAUMBIGA1UdEwEB/wQIMAYBAQECAQAwCgYIKoZI
zj0EAwIDSQAwRgIhAMqIc/Qtr+jZbnrJB7g7W8r/DHmDe+IyFvzwpzdSrxEXAiEA
tXcixZznhcDzlnIqFqkIJRGmvL9Yr6lVoP1ZkDeqmjk=
-----END CERTIFICATE-----

Both certutil and openssl are happy with this chain:

certutil -verify .\authenticator.cer .\trustanchor.cer
CertIssuer:
    CN=Authentrend CA 000
    OU=Authenticator Attestation
    O=ATKeyCA00
    C=SE
  Name Hash(sha1): c7f9a6a1b24376a38ad9b64685e6ab2aace0ae4a
  Name Hash(md5): 379c249ddefa685e689f2e3d9528664d
CertSubject:
    CN=ATKey.Pro SN 01C00000
    OU=Authenticator Attestation
    O=AuthenTrend
    C=TW
  Name Hash(sha1): ab87f67074831a7852f147b8f39f4c86994cd1b5
  Name Hash(md5): b5fe3ad60cd67bf0ef7597de31115fae
Issuing CA CertIssuer:
    CN=Authentrend CA 000
    OU=Authenticator Attestation
    O=ATKeyCA00
    C=SE
  Name Hash(sha1): c7f9a6a1b24376a38ad9b64685e6ab2aace0ae4a
  Name Hash(md5): 379c249ddefa685e689f2e3d9528664d
Issuing CA CertSubject:
    CN=Authentrend CA 000
    OU=Authenticator Attestation
    O=ATKeyCA00
    C=SE
  Name Hash(sha1): c7f9a6a1b24376a38ad9b64685e6ab2aace0ae4a
  Name Hash(md5): 379c249ddefa685e689f2e3d9528664d
Issuing CA Subject name matches Cert Issuer

ERROR: Cert Expires after issuing CA Cert Expires

Cert is an End Entity certificate
Certificate signature is valid
Certificate is current
Certificate has no revocation-check extension

.\authenticator.cer verifies as issued by .\trustanchor.cer -- Revocation check skipped.
CertUtil: -verify command completed successfully.
openssl verify -verbose -CAfile trustanchor.cer -CApath . authenticator.cer 
authenticator.cer: OK
dbalikhin commented 2 years ago

I'm relatively new to this topic, but what is the actual impact of failed ValidateTrustChainSubAnchor on Linux? My understanding is it's required for attestation. But all input data is provided by a huge metadata file from Fido. For each AAGUID we will have some certs in this file. Is it a common case when you have to trust SubAnchor or it's more like edge case that has to be supported because of the specification?

aseigler commented 2 years ago

The way the spec works during registration with attestation, the server looks at a local cache of trusted metadata containing information about various aspects of authenticators, including things like if they are FIDO certified, are there known vulnerabilities in the product, trust anchor certificates and lots of other stuff. That local cache can be populated from the FIDO MDS repository, or managed manually by the server operator. If an authenticator shows up with a known AAGUID and that AAGUID has metadata that indicates that an attestation should look a certain way, a conformant server should only accept registrations that have properties that match what is expected. In this case, it's ensuring that the authenticator attestation certificate was signed by an entity that ties back to one of the trust anchors. It could be that an authenticator shows up with BLE transport, but the metadata shows it is only capable of USB, for example. Enterprise may care about things like this, but general consumers will not, which is what @abergs was getting at in https://github.com/passwordless-lib/fido2-net-lib/issues/291#issuecomment-1119318579.

iamcarbon commented 1 year ago

@aseigler Should we skip these two tests for now so we can have a clean CI build again?

abergs commented 1 year ago

@aseigler Should we skip these two tests for now so we can have a clean CI build again?

I love that we have tests for linux, but I too think we need to disable them (or fix them, but that's not straight forward) to get back to a green CI build. I'd welcome a PR for that @iamcarbon. I don't think @aseigler disagrees :)

aseigler commented 1 year ago

Yeah, I agree we should skip those tests for now until I/we figure out a way to make them work.