diff --git a/packages/cli/src/serve/run-qwen-serve.test.ts b/packages/cli/src/serve/run-qwen-serve.test.ts index 906891dea3f..0992743d568 100644 --- a/packages/cli/src/serve/run-qwen-serve.test.ts +++ b/packages/cli/src/serve/run-qwen-serve.test.ts @@ -2338,6 +2338,73 @@ DBgH1Rq+scl7kyyxeucobVMtUWK6eJRwZ5gfsrpeMH0nqCJLj7s80WMNL9BLojPm rv3eIL/tayAWGZDmOpAiQdZn5pxv2jCLmv06Ae4= -----END CERTIFICATE----- `; +// A renewed root — two self-signed certificates sharing a +// subject AND a key, so both verify the leaf — with the SHORT-lived copy +// first in the bundle. The greedy first-match walk picked the expired copy +// and claimed every handshake fails CERT_HAS_EXPIRED while the merged bundle +// authorizes through the renewed one. +const TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT = `-----BEGIN CERTIFICATE----- +MIIDNDCCAhygAwIBAgIUO1dPcT+RFrKO9eAvR7RprjWBRZswDQYJKoZIhvcNAQEL +BQAwJDEiMCAGA1UEAwwZcXdlbiByZW5ld2VkIHRlc3Qgcm9vdCBDQTAgFw0yNjA4 +MTkxMjEyMjBaGA8yMTI2MDcyNjEyMTIyMFowFDESMBAGA1UEAwwJbG9jYWxob3N0 +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvXioBNIbk+BxfkQYG51E +4VpH2dEvI7CSVtDb9hCr6ejdrsOmIMuHgQoFNrj33WGVovXwqoUsy74j666mXwIU +E4uXM2kObg5rgmdT010CtqVk10NQ2zVP8HfocwvhfsGc+pXxIG3llLsEfEE4Sz6H +opapxI0J0KVngEiJtQ1pQ7ETmowE1ox9pHXg+uWNiWOfdSOffoEBdlbckl/LpPHq +JMzB7bDvKKl7Oj1/0mQjO/vVkPeFsyO0wUSzbo7RrEYfCq9ehZIHHkhTCDLZyy5x +DtPrvyq31LqQYdfFpM2r9hjIrAn4y9SjbrudLY7/mLAQ90gIO8/++CYfEUR7OOTY +gQIDAQABo2wwajAaBgNVHREEEzARhwR/AAABgglsb2NhbGhvc3QwDAYDVR0TAQH/ +BAIwADAdBgNVHQ4EFgQUkteOCy+vH/p/7mLACkjh+sYr+YswHwYDVR0jBBgwFoAU +dNjaHx6CZDrwtW6UMTPqfrILgpAwDQYJKoZIhvcNAQELBQADggEBAGPSIBGUmoNm +ofVSh+2KNJhc5RzePoGUCBJa9wKa8RIpNGhIlKyD5UqvDXuQIK5GxpQ2B/B2T1zq +oyQFT1cvAedW4FbgvdF45IUAsuQmtArTduL7vawgAOW3NAZl8Ib6D8UM/bMZuMZl +78Nu8Amzuti1e4hIYceAFfs8HxgHPOhjA9O4TMr/A6hwHKsnR8CQTGE06lDkRa6p +R3EjMxkYWwzZm+3jaJlpkmxc4u2ouW5Ve7BAyCRb5afl9HD7TA/gLnPk6Z+EKbS9 +avt2q89TDSohaR3fNGbSxWmyq6+gl/8c12kQZABY6FvvUAgfxMURCIoNhgTqJCNk +SJzaW9y9p4M= +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDOTCCAiGgAwIBAgIUFK4nnxLsa266COSbMQBio3y2to0wDQYJKoZIhvcNAQEL +BQAwJDEiMCAGA1UEAwwZcXdlbiByZW5ld2VkIHRlc3Qgcm9vdCBDQTAeFw0yNjA4 +MTkxMjEyMjBaFw0yNjA4MjIxMjEyMjBaMCQxIjAgBgNVBAMMGXF3ZW4gcmVuZXdl +ZCB0ZXN0IHJvb3QgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC1 +WejFH2aVeBrJU/U9sMpdNxYDKB267SM7NckZwvukM/yw8uNTBnFTFmK0MMb18TVA +uxp12+gN1EYxgSHLQQbTHy6sGk8g8XOhbJmL5CM0OSSQ3o7lVI14hbMSLa4IoTpw +snTU/xH6FbT4F7MZc3rtKLLheymaZGaNvGGwZ6hQYSdjQxge1RLtUHqwsKR9utwF +FNxm7EmRrDRHuoxJofQGZORyUHwLZ4X0gLjO1BHNp3qcLfMXh3NM5zmgwwi0rO/u +7M3dv5MPz+EokG1ZdMywdOjYeMZMHKO8id75mmH7sKtePbsMyKbyNK+WtYqdsCSe +mhmwKLUwLPsXprwngzRTAgMBAAGjYzBhMB0GA1UdDgQWBBR02NofHoJkOvC1bpQx +M+p+sguCkDAfBgNVHSMEGDAWgBR02NofHoJkOvC1bpQxM+p+sguCkDAPBgNVHRMB +Af8EBTADAQH/MA4GA1UdDwEB/wQEAwIBBjANBgkqhkiG9w0BAQsFAAOCAQEAryag +ROvm/WygCrsuMoeZvzGM+hIwYpq1nuOGput0zQUAqFQhyUistLCNBQ3zGxdRFEV9 +FWIjBO3XdJArzc/34OWH28OOrf5OD24aXg3aK97NHk8RWCkx9AEU6mP3IAjgGnGy +Qnj3CXnEAXRkDG7iCgM0jrW6gjgXvt4Ytb5WqxuqkUUOjboa5Ib01mI1QUobSqA5 +q3YiuRtwWCjJN5AyTHQnczRQU9GMcCaBd6d1Hs5DHx1dqYj5Vd6SDoUJojkydPiB +gux1cYPewfQh1p2SX6YLKRHheooVp1mgXju1x81nPl80cXsErrlFYLo7rVhjabNR +uhzf6T5xCyhBjj2XWA== +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDOzCCAiOgAwIBAgIURuDBy7kKe3JAEkySJTgKilHa8ecwDQYJKoZIhvcNAQEL +BQAwJDEiMCAGA1UEAwwZcXdlbiByZW5ld2VkIHRlc3Qgcm9vdCBDQTAgFw0yNjA4 +MTkxMjEyMjBaGA8yMTI2MDcyNjEyMTIyMFowJDEiMCAGA1UEAwwZcXdlbiByZW5l +d2VkIHRlc3Qgcm9vdCBDQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB +ALVZ6MUfZpV4GslT9T2wyl03FgMoHbrtIzs1yRnC+6Qz/LDy41MGcVMWYrQwxvXx +NUC7GnXb6A3URjGBIctBBtMfLqwaTyDxc6FsmYvkIzQ5JJDejuVUjXiFsxItrgih +OnCydNT/EfoVtPgXsxlzeu0osuF7KZpkZo28YbBnqFBhJ2NDGB7VEu1QerCwpH26 +3AUU3GbsSZGsNEe6jEmh9AZk5HJQfAtnhfSAuM7UEc2nepwt8xeHc0znOaDDCLSs +7+7szd2/kw/P4SiQbVl0zLB06Nh4xkwco7yJ3vmaYfuwq149uwzIpvI0r5a1ip2w +JJ6aGbAotTAs+xemvCeDNFMCAwEAAaNjMGEwHQYDVR0OBBYEFHTY2h8egmQ68LVu +lDEz6n6yC4KQMB8GA1UdIwQYMBaAFHTY2h8egmQ68LVulDEz6n6yC4KQMA8GA1Ud +EwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgEGMA0GCSqGSIb3DQEBCwUAA4IBAQCj +1C2V2wPvByLc9hmmx247jZs+kKiNmVW6HIEieTbRAgEr2X+sE3DGDdvxs9N7cBKW +hrrxW0DKfi86M3cqM3Z4+hpOyPukRpGF6MSuK+Z09gN/Mr8fHcpGqY37GdcREEbt +eSR8whZQy7mxoNBgPgDUa5dusMsemhkvQtT0kJNBo3mrVdxMrbT/iMqPHDnfUB5P +JuqlSTd6kmII8MLMkOPnlrBLE/IiMnpeq5bPbNqWJ3jSOWB0Y5urevXlhLvhhkZi +mfaYUhXXxhzIHbSGiqWkLbZH0m1OB7jZXhlee02W/9i/vmdFNBIAxTAfshB0X9d+ +CLo3gjYAPFzEeGqHNP3E +-----END CERTIFICATE----- +`; + const CROSS_ROOT_1 = `-----BEGIN CERTIFICATE----- MIIBkTCCATagAwIBAgIUeT/d3GHNmHzQitzyLZ3Yru0BnuIwCgYIKoZIzj0EAwIw DTELMAkGA1UEAwwCUjEwHhcNMjYwODIwMTQwNjQ0WhcNMzYwODE3MTQwNjQ0WjAN @@ -2628,6 +2695,23 @@ describe.skipIf(!loaderOracleAvailable)('describeWorkerTlsTrustGaps', () => { return `${blocks!.at(-1)}\n`; }; + /** The renewed-root bundle minus its long-lived twin. */ + const leafPlusShortTwinOnlyPem = (): string => { + const blocks = TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT.match( + /-----BEGIN CERTIFICATE-----[^-]*-----END CERTIFICATE-----/g, + ); + // Index 1 must hold the short-lived twin the boundary clock sits on; + // a reordered fixture would silently un-pin the notAfter edge below. + const shortTwin = new X509Certificate(blocks![1]); + if ( + new Date(shortTwin.validTo).getTime() !== + new Date('2026-08-22T12:12:20Z').getTime() + ) { + throw new Error('renewed-root fixture order changed'); + } + return `${blocks![0]}\n${blocks![1]}\n`; + }; + it('reports nothing for a self-signed cert covering the dialled host', () => { expect( describeWorkerTlsTrustGaps({ @@ -2785,6 +2869,124 @@ describe.skipIf(!loaderOracleAvailable)('describeWorkerTlsTrustGaps', () => { ); }); + // The false-alarm direction: the walk took whichever copy + // of a renewed CA came first and reported the expired one as the path the + // handshake relies on. OpenSSL may use either; preferring the usable one + // keeps the diagnostic from sending operators to renew a CA they already + // renewed. The clock is moved past the short-lived copy only. + it('prefers a usable issuer over an expired same-subject twin', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2030-01-01T00:00:00Z')); + try { + expect( + describeWorkerTlsTrustGaps({ + cert: Buffer.from(TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT), + certPath: '/certs/fullchain.pem', + daemonUrl, + }), + ).toEqual([]); + } finally { + vi.useRealTimers(); + } + }); + + // One-clock witnesses: the walk's issuer preference and the per-member + // validity flags must judge the SAME instant the report samples. The + // straddle test hands that one sample an instant inside the short-lived + // twin's window and poisons any second sample with an instant past it — + // two clocks would anchor through the short root AND tell the operator + // to renew it, the false CERT_HAS_EXPIRED this PR removes. Sample order + // routes the clocks, not a stack-name match on the module-private walk, + // so a rename cannot silently no-op the guard. The boundary tests freeze + // the clock on each edge of that window, where the expired/not-yet-valid + // flags flip, so a one-sided edit to either predicate fails instead of + // passing silently years away from any boundary. + it('judges the walk and the report at one sampled instant', () => { + const insideWindow = new Date('2026-08-20T00:00:00Z').getTime(); + const pastWindow = new Date('2026-08-25T00:00:00Z').getTime(); + // Production passes the serving file as certSourcePath; mirror that. + // The temp-file fallback spends a Date.now sample of its own + // (graceful-fs retry timing), which would shift the poison below. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'qwen-one-clock-')); + const certSourcePath = path.join(dir, 'fullchain.pem'); + fs.writeFileSync(certSourcePath, TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT, { + mode: 0o600, + }); + let samples = 0; + const clock = vi.spyOn(Date, 'now').mockImplementation(() => { + samples += 1; + return samples === 1 ? insideWindow : pastWindow; + }); + try { + expect( + describeWorkerTlsTrustGaps({ + cert: Buffer.from(TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT), + certPath: '/certs/fullchain.pem', + certSourcePath, + daemonUrl, + }), + ).toEqual([]); + // A second sample IS the regression: the walk and the report judging + // different instants. Fail on it even when the gap happens not to. + expect(samples).toBe(1); + } finally { + clock.mockRestore(); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it('agrees on the exact instant the short-lived twin expires', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-08-22T12:12:20Z')); + try { + expect( + describeWorkerTlsTrustGaps({ + cert: Buffer.from(TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT), + certPath: '/certs/fullchain.pem', + daemonUrl, + }), + ).toEqual([]); + } finally { + vi.useRealTimers(); + } + }); + + // The notAfter edge is pinned only when the expiring twin is the walk's + // ONLY issuer: with the long-lived twin in the bundle, a strict `>` in + // certValidAt re-anchors through it and still reports no gaps one + // instant early — the same false CERT_HAS_EXPIRED class this PR removes. + it('still anchors at the exact notAfter when the short twin is the only issuer', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-08-22T12:12:20Z')); + try { + expect( + describeWorkerTlsTrustGaps({ + cert: Buffer.from(leafPlusShortTwinOnlyPem()), + certPath: '/certs/fullchain.pem', + daemonUrl, + }), + ).toEqual([]); + } finally { + vi.useRealTimers(); + } + }); + + it('agrees on the exact instant the short-lived twin becomes valid', () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date('2026-08-19T12:12:20Z')); + try { + expect( + describeWorkerTlsTrustGaps({ + cert: Buffer.from(TEST_TLS_CERT_FULLCHAIN_RENEWED_ROOT), + certPath: '/certs/fullchain.pem', + daemonUrl, + }), + ).toEqual([]); + } finally { + vi.useRealTimers(); + } + }); + it('names an expired chain member the signature-only walk accepts', () => { // R2-3(b): `x509.verify` checks signatures and never consults dates, so an // expired root anchors a fullchain "fine" here while every worker diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index ac96b3d5257..8b4af9b916d 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -1037,9 +1037,14 @@ export function describeWorkerTlsTrustGaps(opts: { // leaf (what the `mkcert` flow this project documents produces) never // terminates the chain — unless something else in the worker's bundle // carries the issuer that does. + // One clock for the whole report: the walk's issuer preference and the + // per-member validity flags below must judge the same sampled instant, or + // the walk can anchor through a certificate the report then contradicts. + const now = Date.now(); const anchorPath = walkWorkerAnchorPath( x509, workerTrustStore, + now, // The `servingBlocks === undefined` fallback prepends the leaf too, but // that file already reports its own gap below and the workers receive it // verbatim; only the partial-read case needs the distinction. @@ -1230,7 +1235,6 @@ export function describeWorkerTlsTrustGaps(opts: { // `X509Certificate.verify` checks signatures only and never consults dates, // so an expired root or intermediate anchors "fine" here while every worker // handshake fails CERT_HAS_EXPIRED. Boot validation covers the leaf alone. - const now = Date.now(); for (const member of anchorPath.path) { if (member.fingerprint256 === x509.fingerprint256) continue; const subject = member.subject.replace(/\r?\n/g, ', '); @@ -1251,20 +1255,22 @@ export function describeWorkerTlsTrustGaps(opts: { ); continue; } - if (new Date(member.validTo).getTime() < now) { - gaps.push( - `--tls-cert "${opts.certPath}" chains through "${subject}", which ` + - `expired on ${member.validTo} — every worker handshake to the ` + - `daemon will fail CERT_HAS_EXPIRED. Renew that chain member and ` + - `restart.`, - ); - } else if (new Date(member.validFrom).getTime() > now) { - gaps.push( - `--tls-cert "${opts.certPath}" chains through "${subject}", which is ` + - `not yet valid (validFrom: ${member.validFrom}) — every worker ` + - `handshake to the daemon will fail CERT_NOT_YET_VALID. Check that ` + - `chain member's notBefore date or the system clock.`, - ); + if (!certValidAt(member, now)) { + if (new Date(member.validTo).getTime() < now) { + gaps.push( + `--tls-cert "${opts.certPath}" chains through "${subject}", which ` + + `expired on ${member.validTo} — every worker handshake to the ` + + `daemon will fail CERT_HAS_EXPIRED. Renew that chain member and ` + + `restart.`, + ); + } else { + gaps.push( + `--tls-cert "${opts.certPath}" chains through "${subject}", which is ` + + `not yet valid (validFrom: ${member.validFrom}) — every worker ` + + `handshake to the daemon will fail CERT_NOT_YET_VALID. Check that ` + + `chain member's notBefore date or the system clock.`, + ); + } } } const host = workerDialHost(opts.daemonUrl); @@ -1452,6 +1458,14 @@ function cannotIssueCertificates(cert: X509Certificate): boolean { return !isV1Certificate(cert) && keyUsage === undefined; } +/** Whether `cert`'s validity window contains `now`. */ +function certValidAt(cert: X509Certificate, now: number): boolean { + return ( + new Date(cert.validFrom).getTime() <= now && + new Date(cert.validTo).getTime() >= now + ); +} + function isSelfSignedCert(x509: X509Certificate): boolean { if (x509.subject !== x509.issuer) return false; try { @@ -1580,6 +1594,12 @@ function pathLengthConstraint(cert: X509Certificate): number | undefined { function walkWorkerAnchorPath( leaf: X509Certificate, chain: readonly X509Certificate[], + /** + * The instant the report samples trust at. The issuer preference must judge + * the same clock the caller's per-member validity flags do, or the walk can + * anchor through a certificate the report then contradicts. + */ + now: number, /** * Whether `leaf` is a certificate the workers' loader actually hands them. * It is not when the caller had to PREPEND the boot-parsed leaf because the @@ -1650,11 +1670,19 @@ function walkWorkerAnchorPath( : { anchored: true, path }; } walked.add(current.fingerprint256); - const issuer: X509Certificate | undefined = chain.find( + const issuers = chain.filter( (candidate) => !walked.has(candidate.fingerprint256) && certIssuedBy(current, candidate), ); + // A renewed CA leaves two certificates in the bundle that share a subject + // AND a key, so both verify what they issued. Taking whichever one came + // first reported the expired copy as the path the handshake depends on and + // told the operator to renew a CA they had already renewed, while the + // merged bundle authorizes through the renewed copy. OpenSSL may use + // either, so prefer the copy that is usable now. + const issuer: X509Certificate | undefined = + issuers.find((candidate) => certValidAt(candidate, now)) ?? issuers[0]; // `certIssuedBy` asks only "did this sign that": name match plus signature. // OpenSSL asks a second question of every certificate it uses AS an issuer, // and answering only the first is how a chain that walks THROUGH an