Skip to content

Commit

Permalink
Merge pull request #650 from MasterKale/fix/647-attestation-trust-anc…
Browse files Browse the repository at this point in the history 
…hor-handling

fix/647-attestation-trust-anchor-handling
  • Loading branch information
@MasterKale
MasterKale authored Dec 6, 2024
2 parents beb784f + 7b27b80 commit 4463107
Show file tree
Hide file tree
Showing 5 changed files with 240 additions and 96 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ciChecks.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
strategy:
matrix:
node-version: [20, 22]
deno-version: ['v2.0.x']
deno-version: ['v2.1.x']

steps:
- uses: actions/checkout@v4
Expand Down
170 changes: 99 additions & 71 deletions packages/server/src/helpers/validateCertificatePath.ts
View file
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
import { AsnSerializer } from '@peculiar/asn1-schema';
import type { Certificate } from '@peculiar/asn1-x509';

import { isCertRevoked } from './isCertRevoked.ts';
import { verifySignature } from './verifySignature.ts';
import { mapX509SignatureAlgToCOSEAlg } from './mapX509SignatureAlgToCOSEAlg.ts';
import { getCertificateInfo } from './getCertificateInfo.ts';
import { type CertificateInfo, getCertificateInfo } from './getCertificateInfo.ts';
import { convertPEMToBytes } from './convertPEMToBytes.ts';

/**
* Traverse an array of PEM certificates and ensure they form a proper chain
* @param certificates Typically the result of `x5c.map(convertASN1toPEM)`
* @param rootCertificates Possible root certificates to complete the path
* @param x5cCertsPEM Typically the result of `x5c.map(convertASN1toPEM)`
* @param trustAnchorsPEM PEM-formatted certs that an attestation statement x5c may chain back to
*/
export async function validateCertificatePath(
certificates: string[],
rootCertificates: string[] = [],
x5cCertsPEM: string[],
trustAnchorsPEM: string[] = [],
): Promise<boolean> {
if (rootCertificates.length === 0) {
// We have no root certs with which to create a full path, so skip path validation
// TODO: Is this going to be acceptable default behavior??
if (trustAnchorsPEM.length === 0) {
// We have no trust anchors to chain back to, so skip path validation
return true;
}

let invalidSubjectAndIssuerError = false;
let certificateNotYetValidOrExpiredErrorMessage = undefined;
for (const rootCert of rootCertificates) {
for (const anchorPEM of trustAnchorsPEM) {
try {
const certsWithRoot = certificates.concat([rootCert]);
await _validatePath(certsWithRoot);
const certsWithTrustAnchor = x5cCertsPEM.concat([anchorPEM]);
await _validatePath(certsWithTrustAnchor);
// If we successfully validated a path then there's no need to continue. Reset any existing
// errors that were thrown by earlier root certificates
// errors that were thrown by earlier trust anchors
invalidSubjectAndIssuerError = false;
certificateNotYetValidOrExpiredErrorMessage = undefined;
break;
Expand All @@ -43,7 +43,7 @@ export async function validateCertificatePath(
}
}

// We tried multiple root certs and none of them worked
// We tried multiple trust anchors and none of them worked
if (invalidSubjectAndIssuerError) {
throw new InvalidSubjectAndIssuer();
} else if (certificateNotYetValidOrExpiredErrorMessage) {
Expand All @@ -55,84 +55,104 @@ export async function validateCertificatePath(
return true;
}

async function _validatePath(certificates: string[]): Promise<boolean> {
if (new Set(certificates).size !== certificates.length) {
/**
* @param x5cCerts X.509 `x5c` certs in PEM string format
* @param anchorCert X.509 trust anchor cert in PEM string format
*/
async function _validatePath(x5cCertsWithTrustAnchorPEM: string[]): Promise<boolean> {
if (new Set(x5cCertsWithTrustAnchorPEM).size !== x5cCertsWithTrustAnchorPEM.length) {
throw new Error('Invalid certificate path: found duplicate certificates');
}

// From leaf to root, make sure each cert is issued by the next certificate in the chain
for (let i = 0; i < certificates.length; i += 1) {
const subjectPem = certificates[i];

const isLeafCert = i === 0;
const isRootCert = i + 1 >= certificates.length;
// Make sure no certs are revoked, and all are within their time validity window
for (const certificatePEM of x5cCertsWithTrustAnchorPEM) {
const certInfo = getCertificateInfo(convertPEMToBytes(certificatePEM));
await assertCertNotRevoked(certInfo.parsedCertificate);
assertCertIsWithinValidTimeWindow(certInfo, certificatePEM);
}

let issuerPem = '';
if (isRootCert) {
issuerPem = subjectPem;
} else {
issuerPem = certificates[i + 1];
}
// Make sure each x5c cert is issued by the next certificate in the chain
for (let i = 0; i < (x5cCertsWithTrustAnchorPEM.length - 1); i += 1) {
const subjectPem = x5cCertsWithTrustAnchorPEM[i];
const issuerPem = x5cCertsWithTrustAnchorPEM[i + 1];

const subjectInfo = getCertificateInfo(convertPEMToBytes(subjectPem));
const issuerInfo = getCertificateInfo(convertPEMToBytes(issuerPem));

const x509Subject = subjectInfo.parsedCertificate;

// Check for certificate revocation
const subjectCertRevoked = await isCertRevoked(x509Subject);

if (subjectCertRevoked) {
throw new Error(`Found revoked certificate in certificate path`);
}

// Check that intermediate certificate is within its valid time window
const { notBefore, notAfter } = issuerInfo;

const now = new Date(Date.now());
if (notBefore > now || notAfter < now) {
if (isLeafCert) {
throw new CertificateNotYetValidOrExpired(
`Leaf certificate is not yet valid or expired: ${issuerPem}`,
);
} else if (isRootCert) {
throw new CertificateNotYetValidOrExpired(
`Root certificate is not yet valid or expired: ${issuerPem}`,
);
} else {
throw new CertificateNotYetValidOrExpired(
`Intermediate certificate is not yet valid or expired: ${issuerPem}`,
);
}
}

// Make sure subject issuer is issuer subject
if (subjectInfo.issuer.combined !== issuerInfo.subject.combined) {
throw new InvalidSubjectAndIssuer();
}

// Verify the subject certificate's signature with the issuer cert's public key
const data = AsnSerializer.serialize(x509Subject.tbsCertificate);
const signature = x509Subject.signatureValue;
const signatureAlgorithm = mapX509SignatureAlgToCOSEAlg(
x509Subject.signatureAlgorithm.algorithm,
);
const issuerCertBytes = convertPEMToBytes(issuerPem);
const issuerCertIsRootCert = issuerInfo.issuer.combined === issuerInfo.subject.combined;

const verified = await verifySignature({
data: new Uint8Array(data),
signature: new Uint8Array(signature),
x509Certificate: issuerCertBytes,
hashAlgorithm: signatureAlgorithm,
});
await assertSubjectIsSignedByIssuer(subjectInfo.parsedCertificate, issuerPem);

if (!verified) {
throw new Error('Invalid certificate path: invalid signature');
// Perform one final check if the issuer cert is also a root certificate
if (issuerCertIsRootCert) {
await assertSubjectIsSignedByIssuer(issuerInfo.parsedCertificate, issuerPem);
}
}

return true;
}

/**
* Check if the certificate is revoked or not. If it is, raise an error
*/
async function assertCertNotRevoked(certificate: Certificate): Promise<void> {
// Check for certificate revocation
const subjectCertRevoked = await isCertRevoked(certificate);

if (subjectCertRevoked) {
throw new Error(`Found revoked certificate in certificate path`);
}
}

/**
* Require the cert to be within its notBefore and notAfter time window
*
* @param certInfo Parsed cert information
* @param certPEM PEM-formatted certificate, for error reporting
*/
function assertCertIsWithinValidTimeWindow(certInfo: CertificateInfo, certPEM: string): void {
const { notBefore, notAfter } = certInfo;

const now = new Date(Date.now());
if (notBefore > now || notAfter < now) {
throw new CertificateNotYetValidOrExpired(
`Certificate is not yet valid or expired: ${certPEM}`,
);
}
}

/**
* Ensure that the subject cert has been signed by the next cert in the chain
*/
async function assertSubjectIsSignedByIssuer(
subjectCert: Certificate,
issuerPEM: string,
): Promise<void> {
// Verify the subject certificate's signature with the issuer cert's public key
const data = AsnSerializer.serialize(subjectCert.tbsCertificate);
const signature = subjectCert.signatureValue;
const signatureAlgorithm = mapX509SignatureAlgToCOSEAlg(
subjectCert.signatureAlgorithm.algorithm,
);
const issuerCertBytes = convertPEMToBytes(issuerPEM);

const verified = await verifySignature({
data: new Uint8Array(data),
signature: new Uint8Array(signature),
x509Certificate: issuerCertBytes,
hashAlgorithm: signatureAlgorithm,
});

if (!verified) {
throw new InvalidSubjectSignatureForIssuer();
}
}

// Custom errors to help pass on certain errors
class InvalidSubjectAndIssuer extends Error {
constructor() {
Expand All @@ -142,6 +162,14 @@ class InvalidSubjectAndIssuer extends Error {
}
}

class InvalidSubjectSignatureForIssuer extends Error {
constructor() {
const message = 'Subject signature was invalid for issuer';
super(message);
this.name = 'InvalidSubjectSignatureForIssuer';
}
}

class CertificateNotYetValidOrExpired extends Error {
constructor(message: string) {
super(message);
Expand Down
77 changes: 76 additions & 1 deletion packages/server/src/metadata/verifyAttestationWithMetadata.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,30 @@
import { assertEquals } from '@std/assert';
import { FakeTime } from '@std/testing/time';

import { verifyAttestationWithMetadata } from './verifyAttestationWithMetadata.ts';
import { MetadataStatement } from '../metadata/mdsTypes.ts';
import type { MetadataStatement } from '../metadata/mdsTypes.ts';
import { isoBase64URL } from '../helpers/iso/index.ts';

Deno.test('should verify attestation with metadata (android-safetynet)', async () => {
// Faking time to something that'll satisfy all of these ranges:
// {
// notBefore: 2022-01-25T10:00:34.000Z,
// notAfter: 2022-04-25T10:00:33.000Z
// }
// {
// notBefore: 2020-08-13T00:00:42.000Z,
// notAfter: 2027-09-30T00:00:42.000Z
// }
// {
// notBefore: 2020-06-19T00:00:42.000Z,
// notAfter: 2028-01-28T00:00:42.000Z
// }
// {
// notBefore: 1998-09-01T12:00:00.000Z,
// notAfter: 2028-01-28T12:00:00.000Z
// }
const fakedNow = new FakeTime(new Date('2022-02-01T00:00:00.000Z'));

const metadataStatementJSONSafetyNet: MetadataStatement = {
legalHeader: 'https://fidoalliance.org/metadata/metadata-statement-legal-header/',
aaguid: 'b93fd961-f2e6-462f-b122-82002247de78',
Expand Down Expand Up @@ -56,6 +76,8 @@ Deno.test('should verify attestation with metadata (android-safetynet)', async (
});

assertEquals(verified, true);

fakedNow.restore();
});

Deno.test('should verify attestation with rsa_emsa_pkcs1_sha256_raw authenticator algorithm in metadata', async () => {
Expand Down Expand Up @@ -242,3 +264,56 @@ Deno.test('should verify idmelon attestation with updated root certificate', asy

assertEquals(verified, true);
});

Deno.test('should verify when trust anchor is an intermediate certificate', async () => {
/**
* See https://github.com/MasterKale/SimpleWebAuthn/issues/647 for more context, basically
* HID sells authenticators that are "unique" for their use of intermediate certificates as trust
* anchors. This has been a problem for many other WebAuthn libraries, and now it's my turn to
* deal with an errant assumption from early in this project's life.
*
* The metadata statement below is a slimmed down version of it as pulled from MDS on Dec 2024
*/
const metadataStatement: MetadataStatement = {
legalHeader:
'Submission of this statement and retrieval and use of this statement indicates acceptance of the appropriate agreement located at https://fidoalliance.org/metadata/metadata-legal-terms/.',
aaguid: '692db549-7ae5-44d5-a1e5-dd20a493b723',
description: 'HID Crescendo Key',
authenticatorVersion: 10,
protocolFamily: 'fido2',
schema: 3,
upv: [{ major: 1, minor: 0 }],
authenticationAlgorithms: ['secp256r1_ecdsa_sha256_raw'],
publicKeyAlgAndEncodings: ['cose'],
attestationTypes: ['basic_full'],
userVerificationDetails: [
[{ userVerificationMethod: 'passcode_external' }],
[{ userVerificationMethod: 'none' }],
[{ userVerificationMethod: 'presence_internal' }],
[
{ userVerificationMethod: 'passcode_external' },
{ userVerificationMethod: 'presence_internal' },
],
],
keyProtection: ['hardware', 'secure_element'],
matcherProtection: ['on_chip'],
tcDisplay: [],
attestationRootCertificates: [
'MIIDCDCCAq+gAwIBAgIQQAFqUNTHZ8kBN8u/bCk+xDAKBggqhkjOPQQDAjBrMQswCQYDVQQGEwJVUzETMBEGA1UEChMKSElEIEdsb2JhbDEiMCAGA1UECxMZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEjMCEGA1UEAxMaRklETyBBdHRlc3RhdGlvbiBSb290IENBIDEwHhcNMTkwNDI0MTkzMTIzWhcNNDQwNDI3MTkzMTIzWjBmMQswCQYDVQQGEwJVUzETMBEGA1UEChMKSElEIEdsb2JhbDEiMCAGA1UECxMZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEeMBwGA1UEAxMVRklETyBBdHRlc3RhdGlvbiBDQSAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE4nK9ctzk6GEGFNQBcrnBBmWU+dCnuHQAARrB2Eyc8MbsljkSFhZtfz/Rw6SuVIDk5VakDzrKBAOJ9v0Rvg/406OCATgwggE0MBIGA1UdEwEB/wQIMAYBAf8CAQAwDgYDVR0PAQH/BAQDAgGGMIGEBggrBgEFBQcBAQR4MHYwLgYIKwYBBQUHMAGGImh0dHA6Ly9oaWQuZmlkby5vY3NwLmlkZW50cnVzdC5jb20wRAYIKwYBBQUHMAKGOGh0dHA6Ly92YWxpZGF0aW9uLmlkZW50cnVzdC5jb20vcm9vdHMvSElERklET1Jvb3RjYTEucDdjMB8GA1UdIwQYMBaAFB2m3iwWSYHvWTHbJiHAyKDp+CSjMEcGA1UdHwRAMD4wPKA6oDiGNmh0dHA6Ly92YWxpZGF0aW9uLmlkZW50cnVzdC5jb20vY3JsL0hJREZJRE9Sb290Y2ExLmNybDAdBgNVHQ4EFgQUDLCbuLslcclrOZIz57Fu0imSMQ8wCgYIKoZIzj0EAwIDRwAwRAIgDCW5IrbjEI/y35lPjx9a+/sF4lPSoZdBHgFgTWC+8VICIEqs2SPzUHgHVh65Ajl1oIUmhh0C2lyR/Zdk7O3u1TIK',
],
};

const x5c = [
'MIIDLjCCAtSgAwIBAgIQQAFs2JXwQcL5Eh4rnp2ASjAKBggqhkjOPQQDAjBmMQswCQYDVQQGEwJVUzETMBEGA1UEChMKSElEIEdsb2JhbDEiMCAGA1UECxMZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEeMBwGA1UEAxMVRklETyBBdHRlc3RhdGlvbiBDQSAyMB4XDTE5MDgyODE0MTY0MFoXDTM5MDgyMzE0MTY0MFowaTELMAkGA1UEBhMCVVMxHzAdBgNVBAoTFkhJRCBHbG9iYWwgQ29ycG9yYXRpb24xIjAgBgNVBAsTGUF1dGhlbnRpY2F0b3IgQXR0ZXN0YXRpb24xFTATBgNVBAMTDENyZXNjZW5kb0tleTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAGouI654w6qbGonSTStO2cESYTo8Ezr8OJiPkMl02d6K6i44wXCKV2i+w+bpR6vgYQZ/cKQxMS4uGytqPRNPIejggFfMIIBWzAOBgNVHQ8BAf8EBAMCB4AwgYAGCCsGAQUFBwEBBHQwcjAuBggrBgEFBQcwAYYiaHR0cDovL2hpZC5maWRvLm9jc3AuaWRlbnRydXN0LmNvbTBABggrBgEFBQcwAoY0aHR0cDovL3ZhbGlkYXRpb24uaWRlbnRydXN0LmNvbS9jZXJ0cy9oaWRmaWRvY2EyLnA3YzAfBgNVHSMEGDAWgBQMsJu4uyVxyWs5kjPnsW7SKZIxDzAJBgNVHRMEAjAAMEMGA1UdHwQ8MDowOKA2oDSGMmh0dHA6Ly92YWxpZGF0aW9uLmlkZW50cnVzdC5jb20vY3JsL2hpZGZpZG9jYTIuY3JsMBMGCysGAQQBguUcAgEBBAQDAgQwMB0GA1UdDgQWBBR9h/lCWeTiMUhRS1tj31hBXaOurzAhBgsrBgEEAYLlHAEBBAQSBBBpLbVJeuVE1aHl3SCkk7cjMAoGCCqGSM49BAMCA0gAMEUCIQDpDa1ZbAfCTlBMiDUuB5XH8hnhZUF1JCuCmc+ShI4ZTwIga/ApAudL5R8HxOOHgk8AA/JpgCkMmYDQLVq0QF6oxrU=',
];
const credentialPublicKey =
'pQECAyYgASFYIL80Thvv0K7ftQDCmrwLFELMZxm7s1I1VmPRMYMMkleHIlggtk7VFv4ABu7en1D7I74t66bn2ghLS9-qdcWPDujOvJQ';

const verified = await verifyAttestationWithMetadata({
statement: metadataStatement,
credentialPublicKey: isoBase64URL.toBuffer(credentialPublicKey),
x5c,
});

assertEquals(verified, true);
});
38 changes: 32 additions & 6 deletions packages/server/src/registration/verifications/verifyAttestationAndroidSafetyNet.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,20 @@ Deno.test('should verify Android SafetyNet attestation', async () => {
rpIdHash,
} = await getResponseValues(attestationAndroidSafetyNet);

// notBefore: 2017-06-15T00:00:42.000Z
// notAfter: 2021-12-15T00:00:42.000Z
const mockDate = new FakeTime(new Date('2021-11-15T00:00:42.000Z'));
// Faking time to something that'll satisfy all of these ranges:
// {
// notBefore: 2018-10-10T07:19:45.000Z,
// notAfter: 2019-10-09T07:19:45.000Z
// }
// {
// notBefore: 2017-06-15T00:00:42.000Z,
// notAfter: 2021-12-15T00:00:42.000Z
// }
// {
// notBefore: 1998-09-01T12:00:00.000Z,
// notAfter: 2028-01-28T12:00:00.000Z
// }
const mockDate = new FakeTime(new Date('2019-10-01T00:00:42.000Z'));

const verified = await verifyAttestationAndroidSafetyNet({
attStmt,
Expand Down Expand Up @@ -121,9 +132,24 @@ Deno.test('should validate response with cert path completed with GlobalSign R1
rpIdHash,
} = await getResponseValues(safetyNetUsingGSR1RootCert);

// notBefore: 2006-12-15T08:00:00.000Z
// notAfter: 2021-12-15T08:00:00.000Z
const mockDate = new FakeTime(new Date('2021-11-15T00:00:42.000Z'));
// Faking time to something that'll satisfy all of these ranges:
// {
// notBefore: 2021-07-19T13:13:42.000Z,
// notAfter: 2021-10-17T13:13:41.000Z
// }
// {
// notBefore: 2020-08-13T00:00:42.000Z,
// notAfter: 2027-09-30T00:00:42.000Z
// }
// {
// notBefore: 2020-06-19T00:00:42.000Z,
// notAfter: 2028-01-28T00:00:42.000Z
// }
// {
// notBefore: 1998-09-01T12:00:00.000Z,
// notAfter: 2028-01-28T12:00:00.000Z
// }
const mockDate = new FakeTime(new Date('2021-10-15T00:00:42.000Z'));

const verified = await verifyAttestationAndroidSafetyNet({
attStmt,
Expand Down
Loading

0 comments on commit 4463107

Please sign in to comment.