Skip to content

fix: Additional cert validation and sanity checks #187

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.

Sign up for GitHub

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

Merged
awilliams1-cb merged 3 commits into from
Jul 14, 2026
Merged

fix: Additional cert validation and sanity checks #187

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d4b20c
fix: validate external and internal cert linking
awilliams1-cb Jul 13, 2026
02a0a8a
Formatting
awilliams1-cb Jul 13, 2026
f73689f
PR changes
awilliams1-cb Jul 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 189 additions & 0 deletions __tests__/task-origin-validate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { fileURLToPath } from 'url';
import os from 'os';
import fs from 'fs/promises';
import crypto from 'crypto';
import forge from 'node-forge';
import * as tar from 'tar';
import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals';
import {
Expand Down Expand Up @@ -249,4 +250,192 @@ describe('buildAndValidateSignature', () => {
).rejects.toThrow();
});
});

describe('self-minted certificate chain', () => {
let tempDir: string;

beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'forged-sig-'));
});

afterEach(async () => {
await fs.rm(tempDir, { recursive: true });
});
Comment thread
awilliams1-cb marked this conversation as resolved.
Outdated

// Mints a completely self-signed chain (attacker's own root -> intermediate ->
// leaf) that never touches the real Coinbase hierarchy. The subject/issuer names
// mirror the genuine chain so that, absent the pins, it would build a valid path.
function mintSelfSignedChain(sanURI: string): {
leaf: string;
intermediate: string;
root: string;
} {
const notBefore = new Date('2020-01-01T00:00:00Z');
const notAfter = new Date('2050-01-01T00:00:00Z');

const derBase64 = (cert: forge.pki.Certificate): string =>
forge.util.encode64(forge.asn1.toDer(forge.pki.certificateToAsn1(cert)).getBytes());

const makeCert = (
subjectCN: string,
issuerCN: string,
subjectKey: forge.pki.rsa.PublicKey,
signingKey: forge.pki.rsa.PrivateKey,
serial: string,
// eslint-disable-next-line @typescript-eslint/no-explicit-any
extensions: any[]
): forge.pki.Certificate => {
const cert = forge.pki.createCertificate();
cert.publicKey = subjectKey;
cert.serialNumber = serial;
cert.validity.notBefore = notBefore;
cert.validity.notAfter = notAfter;
cert.setSubject([{ name: 'commonName', value: subjectCN }]);
cert.setIssuer([{ name: 'commonName', value: issuerCN }]);
cert.setExtensions(extensions);
cert.sign(signingKey, forge.md.sha256.create());
return cert;
};

const rootKeys = forge.pki.rsa.generateKeyPair(2048);
const intKeys = forge.pki.rsa.generateKeyPair(2048);
const leafKeys = forge.pki.rsa.generateKeyPair(2048);

// Self-signed root impersonating the real CB-ROOT-CORE.
const root = makeCert(
'CB-ROOT-CORE',
'CB-ROOT-CORE',
rootKeys.publicKey,
rootKeys.privateKey,
'01',
[
{ name: 'basicConstraints', cA: true, critical: true },
{ name: 'keyUsage', keyCertSign: true, cRLSign: true, critical: true },
]
);
// Intermediate impersonating the runtime intermediate, signed by the fake root.
const intermediate = makeCert(
'corporate.device.cbhq.net',
'CB-ROOT-CORE',
intKeys.publicKey,
rootKeys.privateKey,
'02',
[
{ name: 'basicConstraints', cA: true, critical: true },
{ name: 'keyUsage', keyCertSign: true, cRLSign: true, critical: true },
]
);
// Leaf carrying a forged facilitator identity, signed by the fake intermediate.
const leaf = makeCert(
'forged',
'corporate.device.cbhq.net',
leafKeys.publicKey,
intKeys.privateKey,
'03',
[
{ name: 'basicConstraints', cA: false },
{ name: 'keyUsage', digitalSignature: true, critical: true },
{ name: 'subjectAltName', altNames: [{ type: 6, value: sanURI }] },
]
);

return {
leaf: derBase64(leaf),
intermediate: derBase64(intermediate),
root: derBase64(root),
};
}

it('rejects a bundle whose certificate chain is fully self-signed', async () => {
const chain = mintSelfSignedChain('ldap:///base-facilitators');

// Start from a genuine bundle and keep its real signature + TSA timestamp so
// verification reaches the certificate-chain stage; only the cert chain is
// swapped for the self-minted one, laid out as [leaf, runtime intermediate,
// static intermediate, root] to match the injection indices.
const bundle = JSON.parse(
await fs.readFile(
path.join(VALID_SIGNATURES_DIR, 'base-facilitator-signature.json'),
'utf8'
)
);
bundle.verificationMaterial.x509CertificateChain.certificates = [
{ rawBytes: chain.leaf },
{ rawBytes: chain.intermediate },
{ rawBytes: chain.intermediate },
{ rawBytes: chain.root },
];

const forgedSignatureFile = path.join(tempDir, 'base-facilitator-signature.json');
await fs.writeFile(forgedSignatureFile, JSON.stringify(bundle));

// The pins refuse to inject the self-minted root/intermediate as trust anchors,
// so no trusted certificate path can be built for the forged leaf.
await expect(
buildAndValidateSignature({
taskFolderPath: VALID_TASK_FOLDER,
signatureFile: forgedSignatureFile,
commonName: 'base-facilitators',
role: 'baseFacilitator' as TaskOriginRole,
})
).rejects.toThrow(/certificate chain/i);
}, 60000);
Comment thread
awilliams1-cb marked this conversation as resolved.
Outdated
});

describe('malformed certificate chain', () => {
let tempDir: string;

beforeEach(async () => {
tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'malformed-sig-'));
});

afterEach(async () => {
await fs.rm(tempDir, { recursive: true });
});

// The pinning logic assumes the bundle carries the full chain
// [leaf, runtime intermediate, static intermediate, root]. A bundle missing the
// runtime intermediate ([1]) or root ([3]) must be rejected outright rather than
// silently falling through to a base-only chain.
async function writeBundleWithChain(certificateCount: number): Promise<string> {
const bundle = JSON.parse(
await fs.readFile(
path.join(VALID_SIGNATURES_DIR, 'base-facilitator-signature.json'),
'utf8'
)
);
bundle.verificationMaterial.x509CertificateChain.certificates =
bundle.verificationMaterial.x509CertificateChain.certificates.slice(0, certificateCount);

const signatureFile = path.join(tempDir, 'base-facilitator-signature.json');
await fs.writeFile(signatureFile, JSON.stringify(bundle));
return signatureFile;
}

it('rejects a bundle missing the root certificate', async () => {
// Keep [leaf, runtime, static] but drop the root ([3]).
const signatureFile = await writeBundleWithChain(3);
await expect(
buildAndValidateSignature({
taskFolderPath: VALID_TASK_FOLDER,
signatureFile,
commonName: 'base-facilitators',
role: 'baseFacilitator' as TaskOriginRole,
})
).rejects.toThrow('certificate chain must contain a runtime intermediate and root');
});

it('rejects a bundle with only a leaf certificate', async () => {
// Only [leaf] present, so both the runtime intermediate ([1]) and root ([3]) are missing.
const signatureFile = await writeBundleWithChain(1);
await expect(
buildAndValidateSignature({
taskFolderPath: VALID_TASK_FOLDER,
signatureFile,
commonName: 'base-facilitators',
role: 'baseFacilitator' as TaskOriginRole,
})
).rejects.toThrow('certificate chain must contain a runtime intermediate and root');
});
});
});
34 changes: 31 additions & 3 deletions src/lib/task-origin-validate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as tar from 'tar';
import path from 'path';
import fs from 'fs/promises';
import { X509Certificate } from 'crypto';
import { Verifier, toTrustMaterial, toSignedEntity } from '@sigstore/verify';
import { bundleFromJSON } from '@sigstore/bundle';
import trustedRoot from './config/trusted-root.json';
Expand Down Expand Up @@ -135,6 +136,16 @@ export async function buildAndValidateSignature(options: TaskOriginVerifyOptions
const runtimeIntermediate = bundleCerts[1] ? { rawBytes: bundleCerts[1].rawBytes } : null;
const rootCert = bundleCerts[3] ? { rawBytes: bundleCerts[3].rawBytes } : null;

// The pinning below assumes the bundle carries the full chain
// [leaf, runtime intermediate, static intermediate, root]. If the runtime
// intermediate or root is missing, that assumption is broken and we cannot build a
// chain that anchors to a configured CA, so reject rather than silently continuing.
if (!runtimeIntermediate || !rootCert) {
throw new Error(
'Invalid signature bundle: certificate chain must contain a runtime intermediate and root'
);
}

// Prepare trust material with:
// 1. Date strings converted to Date objects (required for filtering)
// 2. Runtime intermediate CA injected into certificate chain
Comment thread
awilliams1-cb marked this conversation as resolved.
Outdated
Expand All @@ -150,10 +161,27 @@ export async function buildAndValidateSignature(options: TaskOriginVerifyOptions
rawBytes: Buffer.from(cert.rawBytes, 'base64'),
}));

// Build the certificate chain: base certs + runtime intermediate + root
// Build the certificate chain: base certs + runtime intermediate + root.
// The runtime intermediate and root come from an external source, so they are
// only injected after being pinned to the statically-configured intermediate.
const staticInter = new X509Certificate(baseCerts[0].rawBytes);
const certChain = [...baseCerts];
if (runtimeIntermediate) certChain.push(runtimeIntermediate);
if (rootCert) certChain.push(rootCert);

// Pin: the root must actually have signed our pinned static intermediate, so
// only the genuine root is trusted. A cert that is not part of the chain (e.g.
// a self-minted root) cannot have produced that signature and is rejected.
// A failed pin falls through to a base-only chain so other configured CAs can
// still be tried; verification rejects only if no CA yields a trusted path.
const root = new X509Certificate(new Uint8Array(rootCert.rawBytes));
if (root.ca && staticInter.verify(root.publicKey)) {
// Pin: the runtime intermediate must be issued by our pinned static
// intermediate; this also rejects a self-signed intermediate injected here.
const runtime = new X509Certificate(new Uint8Array(runtimeIntermediate.rawBytes));
if (runtime.ca && runtime.verify(staticInter.publicKey)) {

@leopoldjoy leopoldjoy Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we fail closed if this runtime pin does not verify? Otherwise the root may still be trusted and verification can fall back to shorter static/root-issued paths.

@awilliams1-cb awilliams1-cb Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our trusted-root.json has two certificates to try and build the cert chain against. If we fail early then we would short-circuit before both certificates have been checked. Implicitly a shorter path wouldn't be able to validate against the signature unless the shortened path actually did sign. In that case, the shorter path would only be the cb static intermediate which would fail even before the signature because the leaf wouldn't connect back up to it.

certChain.push(runtimeIntermediate);
}
certChain.push(rootCert);

@leopoldjoy leopoldjoy Jul 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we confirm the intended trust model here? Since Sigstore treats each cert in certChain as a trust anchor, adding the external root may allow paths that skip the runtime/static hierarchy.

@awilliams1-cb awilliams1-cb Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The certs being added are being validated against the trusted intermediate, so if the root cert doesn't link to the trusted intermediate, or static intermediate isn't linked to the trusted intermediate, then they aren't added as a trust anchor. So there is no circumvention of the trusted intermediate with the addition of the hierarchy verification.

Now if the verified root or any verified intermediate (including the ones that are already in this repo) were to directly issue the leaf, then yes that would count. Our verification only ensures that a given signature originates from the cert chain that we build using the trusted roots stored in this repo.

}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const normalized: any = {
Expand Down
Loading