go-webauthn/webauthn

x509: unhandled critical extension error when using TPM attestation format

Closed this issue · 12 comments

on-keyday commented

Version

0.11.0

Description

Thank you for developing this library

My Test Environment:
OS: Windows 11 Home
Version: 23H2
OS Build: 22631.3880
Browser: Brave (Brave 1.68.137 Chromium: 127.0.6533.100 (Official Build) （64 bit)

Actual library version I used is 0.11.1 but Version selection of issue template is not shown so selected 0.11.0.

I encountered below error when using this library at registration logic with Windows Hello.

Unable to validate attestation signature statement during attestation validation: invalid certificate chain from MDS: x509: unhandled critical extension

When I used version 0.10.2 of this library, this error never happened.
I tried using metadata.Provider feature so I guessed it is the reason.
I read some code and found the reason.

The error happens here.

webauthn/protocol/attestation.go

Lines 257 to 259 in cf1758a

if _, err = x5c.Verify(entry.MetadataStatement.Verifier()); err != nil {
return ErrInvalidAttestation.WithDetails(fmt.Sprintf("Unable to validate attestation signature statement during attestation validation: invalid certificate chain from MDS: %v", err))
}

and when I was debugging the code in x509.ParseCertificate in standard library, I reached here unhandled = true
https://github.com/golang/go/blob/72735094660a475a69050b7368c56b25346f5406/src/crypto/x509/parser.go#L691

out.DNSNames, out.EmailAddresses, out.IPAddresses, out.URIs, err = parseSANExtension(e.Value)
if err != nil {
    return err
}

if len(out.DNSNames) == 0 && len(out.EmailAddresses) == 0 && len(out.IPAddresses) == 0 && len(out.URIs) == 0 {
    // If we didn't parse anything then we do the critical check, below.
    unhandled = true
}

after that, reaches here
https://github.com/golang/go/blob/72735094660a475a69050b7368c56b25346f5406/src/crypto/x509/parser.go#L819-L821

if e.Critical && unhandled {
      out.UnhandledCriticalExtensions = append(out.UnhandledCriticalExtensions, e.Id)
}

and then in x5c.Verify finally reaches here
https://github.com/golang/go/blob/72735094660a475a69050b7368c56b25346f5406/src/crypto/x509/verify.go#L564-L566

if len(c.UnhandledCriticalExtensions) > 0 {
    return UnhandledCriticalExtension{}
}

parseSANExtension in standard library is here.
https://github.com/golang/go/blob/72735094660a475a69050b7368c56b25346f5406/src/crypto/x509/parser.go#L374-L417

func parseSANExtension(der cryptobyte.String) (dnsNames, emailAddresses []string, ipAddresses []net.IP, uris []*url.URL, err error) {
	err = forEachSAN(der, func(tag int, data []byte) error {
		switch tag {
		case nameTypeEmail:
			email := string(data)
			if err := isIA5String(email); err != nil {
				return errors.New("x509: SAN rfc822Name is malformed")
			}
			emailAddresses = append(emailAddresses, email)
		case nameTypeDNS:
			name := string(data)
			if err := isIA5String(name); err != nil {
				return errors.New("x509: SAN dNSName is malformed")
			}
			dnsNames = append(dnsNames, string(name))
		case nameTypeURI:
			uriStr := string(data)
			if err := isIA5String(uriStr); err != nil {
				return errors.New("x509: SAN uniformResourceIdentifier is malformed")
			}
			uri, err := url.Parse(uriStr)
			if err != nil {
				return fmt.Errorf("x509: cannot parse URI %q: %s", uriStr, err)
			}
			if len(uri.Host) > 0 {
				if _, ok := domainToReverseLabels(uri.Host); !ok {
					return fmt.Errorf("x509: cannot parse URI %q: invalid domain", uriStr)
				}
			}
			uris = append(uris, uri)
		case nameTypeIP:
			switch len(data) {
			case net.IPv4len, net.IPv6len:
				ipAddresses = append(ipAddresses, data)
			default:
				return errors.New("x509: cannot parse IP address of length " + strconv.Itoa(len(data)))
			}
		}

		return nil
	})

	return
}

By the way, at attestation_tpm.go, custom certificate extension validation logic exists like below.

webauthn/protocol/attestation_tpm.go

Lines 175 to 182 in 9ca2fae

for _, ext := range aikCert.Extensions {
if ext.Id.Equal([]int{2, 5, 29, 17}) {
manufacturer, model, version, err = parseSANExtension(ext.Value)
if err != nil {
return "", nil, err
}
}
}

and custom parseSANExtension which is not compatible with standard library

webauthn/protocol/attestation_tpm.go

Lines 301 to 336 in 9ca2fae

func parseSANExtension(value []byte) (manufacturer string, model string, version string, err error) {
err = forEachSAN(value, func(tag int, data []byte) error {
switch tag {
case nameTypeDN:
tpmDeviceAttributes := pkix.RDNSequence{}
_, err := asn1.Unmarshal(data, &tpmDeviceAttributes)
if err != nil {
return err
}
for _, rdn := range tpmDeviceAttributes {
if len(rdn) == 0 {
continue
}
for _, atv := range rdn {
value, ok := atv.Value.(string)
if !ok {
continue
}
if atv.Type.Equal(tcgAtTpmManufacturer) {
manufacturer = strings.TrimPrefix(value, "id:")
}
if atv.Type.Equal(tcgAtTpmModel) {
model = value
}
if atv.Type.Equal(tcgAtTpmVersion) {
version = strings.TrimPrefix(value, "id:")
}
}
}
}
return nil
})
return
}

that's why this error happens.

Reproduction

static/register.html

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Bug Test</title>
</head>
<body>
    <script type="module">
        
        const base64ToUint8Array = (base64) =>{
            const base64Characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
            //const base64URLCharacters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_';

            let cleanedBase64 = String(base64).replace(/-/g, '+').replace(/_/g, '/');
            const padding = (4 - (cleanedBase64.length % 4)) % 4;
            cleanedBase64 += '='.repeat(padding);

            const rawLength = cleanedBase64.length;
            const decodedLength = (rawLength * 3) / 4 - padding;

            const uint8Array = new Uint8Array(decodedLength);

            let byteIndex = 0;
            for (let i = 0; i < rawLength; i += 4) {
                const encoded1 = base64Characters.indexOf(cleanedBase64[i]);
                const encoded2 = base64Characters.indexOf(cleanedBase64[i + 1]);
                const encoded3 = base64Characters.indexOf(cleanedBase64[i + 2]);
                const encoded4 = base64Characters.indexOf(cleanedBase64[i + 3]);

                const decoded1 = (encoded1 << 2) | (encoded2 >> 4);
                const decoded2 = ((encoded2 & 15) << 4) | (encoded3 >> 2);
                const decoded3 = ((encoded3 & 3) << 6) | encoded4;

                uint8Array[byteIndex++] = decoded1;
                if (encoded3 !== 64) uint8Array[byteIndex++] = decoded2;
                if (encoded4 !== 64) uint8Array[byteIndex++] = decoded3;
            }

            return uint8Array;
        }

        const Uint8ArrayToBase64 = (uint8Array) =>{
            //const base64Characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/';
            const base64URLCharacters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_';
            const base64Characters = base64URLCharacters;

            let base64 = '';
            const { length } = uint8Array;

            for (let i = 0; i < length; i += 3) {
                const byte1 = uint8Array[i];
                const byte2 = uint8Array[i + 1];
                const byte3 = uint8Array[i + 2];

                const encoded1 = byte1 >> 2;
                const encoded2 = ((byte1 & 3) << 4) | (byte2 >> 4);
                const encoded3 = ((byte2 & 15) << 2) | (byte3 >> 6);
                const encoded4 = byte3 & 63;

                base64 += base64Characters[encoded1] + base64Characters[encoded2];
                base64 += byte2 !== undefined ? base64Characters[encoded3] : '=';
                base64 += byte3 !== undefined ? base64Characters[encoded4] : '=';
            }

            return base64;
        }

        const j = await fetch("/register").then(r => r.json());
        j.publicKey.challenge = base64ToUint8Array(j.publicKey.challenge).buffer;
        j.publicKey.user.id = base64ToUint8Array(j.publicKey.user.id).buffer;
        console.log(j);
        const r = await navigator.credentials.create({
            publicKey: {
                ...j.publicKey,
               attestation: "direct" 
            }
        });
        console.log(r);
        //r.rawId = Uint8ArrayToBase64(new Uint8Array(r.rawId))
        const js =JSON.stringify({
            id: r.id,
            rawId: Uint8ArrayToBase64(new Uint8Array(r.rawId)),
            response: {
                clientDataJSON: Uint8ArrayToBase64(new Uint8Array(r.response.clientDataJSON)),
                attestationObject: Uint8ArrayToBase64(new Uint8Array(r.response.attestationObject))
            },
            type: r.type
        })
        console.log(js);
        await fetch("/verify", {
            method: "POST",
            headers: {
                "Content-Type": "application/json"
            },
            body: js
        });


    </script>
</body>
</html>

main.go

package main

import (
	_ "embed"
	"encoding/base64"
	"encoding/json"
	"log"
	"net/http"

	"github.com/go-webauthn/webauthn/metadata/providers/cached"
	"github.com/go-webauthn/webauthn/protocol"
	"github.com/go-webauthn/webauthn/webauthn"
)

type user struct{}

func (u *user) WebAuthnID() []byte {
	return []byte("test")
}

func (u *user) WebAuthnName() string {
	return "test"
}

func (u *user) WebAuthnDisplayName() string {
	return "test"
}

func (u *user) WebAuthnIcon() string {
	return ""
}

func (u *user) WebAuthnCredentials() []webauthn.Credential {
	return nil
}

var _ webauthn.User = &user{}

func marshal(v interface{}) string {
	b, err := json.Marshal(v)
	if err != nil {
		panic(err)
	}
	return string(b)
}

func marshalAndBase64(v interface{}) string {
	return base64.StdEncoding.EncodeToString([]byte(marshal(v)))
}

func unmarshal(s string, v interface{}) error {
	return json.Unmarshal([]byte(s), v)
}

func unmarshalFromBase64(s string, v interface{}) error {
	b, err := base64.StdEncoding.DecodeString(s)
	if err != nil {
		return err
	}
	return unmarshal(string(b), v)
}

//go:embed static/register.html
var registerHTML string

func main() {
	v, err := cached.New(cached.WithPath("metadata.json"))
	if err != nil {
		log.Fatal("metadata.json error:", err)
	}
	a, err := webauthn.New(&webauthn.Config{
		RPID: "localhost",
		RPOrigins: []string{
			"http://localhost:8081",
		},
		RPDisplayName: "localhost",
		MDS:           v,
	})
	if err != nil {
		log.Fatal("webauthn error:", err)
	}
	http.HandleFunc("/register", func(w http.ResponseWriter, r *http.Request) {
		creation, session, err := a.BeginRegistration(&user{}, webauthn.WithAttestationFormats([]protocol.AttestationFormat{
			protocol.AttestationFormatTPM,
		}))
		if err != nil {
			log.Println(err)
			return
		}
		http.SetCookie(w, &http.Cookie{
			Name:     "session",
			Value:    marshalAndBase64(session),
			SameSite: http.SameSiteStrictMode,
		})
		w.Header().Set("Content-Type", "application/json")
		w.Write([]byte(marshal(creation)))
	})

	http.HandleFunc("/verify", func(w http.ResponseWriter, r *http.Request) {
		session := &webauthn.SessionData{}
		cookie, err := r.Cookie("session")
		if err != nil {
			log.Println(err)
			return
		}
		err = unmarshalFromBase64(cookie.Value, session)
		if err != nil {
			log.Println(err)
			return
		}
		attestation, err := a.FinishRegistration(&user{}, *session, r)
		if err != nil {
			log.Println(err) // error will be printed from here
			return
		}
		w.Header().Set("Content-Type", "application/json")
		w.Write([]byte(marshal(attestation)))
	})

	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Type", "text/html")
		w.Write([]byte(registerHTML))
	})
        
        log.Println("listening on localhost:8081")
	log.Fatal(http.ListenAndServe("localhost:8081", nil))
}

(sorry, the code may be bit dirty)

  1. paste above code into main.go and static/register.html
  2. go mod init if at the first time
  3. go run .
  4. then access to localhost:8081 and do WebAuthn registration
  5. the error will be printed on the terminal

Expectations

Currently, I would skip this check by using metadata/providers/memory.WithValidateTrustAnchor(false) option.
But I think this additional validation feature is maybe unnecessary because custom trust anchor verification logics looks already implemented for each attestation type. Otherwise I think verification features should be merged in different way.
(note that this is only my opinion from my narrow (not understanding all of features of this library and not perfectly understanding certificate verification) perspective)

Documentation

No response

james-d-elliott commented

Thanks for reporting, and the very detailed report. This is unlikely to be a bug here but to be an issue with that particular MDS entry. I'll take a closer look soon to make sure. There are narrower expectations of an MDS3 entry than a standard X.509 certificate.

The reason this happens now and not prior is no trust anchor validation was done previously. This was the intent of the major change in 0.11. It's likely disabling that option would work around it.

  • 👍1
james-d-elliott commented

Yeah okay so I think it's specifically a bug in the new functionality which is optional. Specifically because the AIK certificate is parsed via stdlib the SAN critical extension isn't parsed since the stdlib ignores the ASN.1 tag that's part of the SAN extension for TPM 2.0 AIK certificates.

Because there is now additional logic to validate the attestation against the MDS now the error occurs. I'll put up a PR for you to test. The idea is that if the certificate is parsed and the type is TPM, and the parsed certificate has not had that critical extension parsed we will reparse it internally to determine if that's actually accurate and update the certificate accordingly.

james-d-elliott commented

I'll have a personal look later but I think github.com/go-webauthn/webauthn@6aeb62ba4e1f542c42e54483b75861fe23585bc0 fixes this.

james-d-elliott commented

@on-keyday can you check if github.com/go-webauthn/webauthn@2a0d5434b7f3faee49e4c3820ecf8264328bcb34 fixes this? i.e.

go get github.com/go-webauthn/webauthn@2a0d5434b7f3faee49e4c3820ecf8264328bcb34
on-keyday commented

@james-d-elliott
Sorry for the too late reply and thank you for mention.
I should report that I tried the fix(github.com/go-webauthn/webauthn@6aeb62ba4e1f542c42e54483b75861fe23585bc0) before.
The test case was successful, but in the real environment,another error occurred (I forgot the details).
So I rolled back the version and tried to find out if the cause was this library or my customization (e.g. actually, I use customized format of payload between server and client for my experimental purpose), but I couldn't find it, and forgot to report and fix it until now. Very sorry. Now I'm trying the fix(github.com/go-webauthn/webauthn@2a0d5434b7f3faee49e4c3820ecf8264328bcb34) again and I'll report soon.

on-keyday commented

Still I'm trying the fixes, I found this message from the logs of my server (according to the log, my trial was 2024-08-27 08:28:07 (UTC)).
When I tried the webauthn registration, I encountered below error. But I didn't know if it is because of this library, MDS , my customization, or my environment.

Unable to validate attestation signature statement during attestation validation: invalid certificate chain from MDS: x509: certificate signed by unknown authority
james-d-elliott commented

Looks like the original issue is fixed but there is maybe another one. Is the authenticator in this instance Windows Hello? If so can you set it up with a standard Windows Account or do you need a Microsoft Account?

on-keyday commented

I'm using a Microsoft Account, but I don't think that's the actual cause of the problem (I think Windows Hello is working fine). Today, I realized that I was applying different conditions in my test environment than I did in my production environment, and after I fixed that, I noticed that the same error was occurring in my test environment, so I'll try to debug it.

on-keyday commented

While debugging I realized that Windows Hello might be the actual problem. (The speculation in the previous comment is probably wrong.)

on-keyday commented

I'm sorry for changing my opinion freaquentry but I guess it is because of chain of certificates.

webauthn/protocol/attestation.go

Lines 263 to 265 in ad0f7e2

if _, err = x5c.Verify(entry.MetadataStatement.Verifier()); err != nil {
return ErrInvalidAttestation.WithDetails(fmt.Sprintf("Unable to validate attestation signature statement during attestation validation: invalid certificate chain from MDS: %v", err))
}

https://github.com/golang/go/blob/91d7ab2cefcc653f8b438fbfaa48d504dbfa4f00/src/crypto/x509/x509.go#L761

at here, x5c is a certificate to be attesteted but it's parent was not in crypto/x509.VerifyOpts.Roots from entry.MetadataStatement.Verifier()
but I found the URL to the parent certificate in x5c.IssuingCertificateURL
so to verify this type of certificate, we need to download the parent and add it to crypto/x509.VerifyOpts.Intermediates

james-d-elliott commented

I want to replicate it, and have never used Windows Hello, was trying to ascertain the easiest path for me to do so.

I'm sorry for changing my opinion freaquentry but I guess it is because of chain of certificates.

Not necessarily, we do chain validation, just the way this attestation format is doing it far different to anything else, so we'll just have to validate that process is actually correct.

but I found the URL to the parent certificate in x5c.IssuingCertificateURL so to verify this type of certificate, we need to download the parent and add it to crypto/x509.VerifyOpts.Intermediates

This makes sense. I'll see about adding that fix and get you to test it. Will still be ideal if i can replicate it.

  • 👍1
james-d-elliott commented

This initial issue is fixed in ad0f7e2. A new issue has been created to track the subsequent issue which is linked above.