From 2e124ab50c06fae30d11eaaf6fc9e234ae0c752e Mon Sep 17 00:00:00 2001 From: master <> Date: Fri, 6 Mar 2026 16:10:09 +0200 Subject: [PATCH] fix: DPoP DER signature parsing race condition causing intermittent auth failures Root cause: derToJoseSignature() used bytes[0]===0x30 to detect DER format, but raw P1363 ECDSA signatures have a ~1/256 chance of their first byte being 0x30, causing spurious DER parse attempts and "expected INTEGER for r" errors. This broke DPoP proof generation intermittently, failing console context loads on random pages. Fix: detect raw P1363 by checking byte length matches expected curve size (64 for ES256, 96 for ES384) before inspecting content bytes. Only attempt DER parsing after full structural validation confirms SEQUENCE+INTEGER structure. Also pass componentSize from the algorithm so ES384 signatures are handled correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/app/core/auth/dpop/dpop.service.ts | 3 +- .../src/app/core/auth/dpop/jose-utilities.ts | 117 +++++++++++++++--- 2 files changed, 101 insertions(+), 19 deletions(-) diff --git a/src/Web/StellaOps.Web/src/app/core/auth/dpop/dpop.service.ts b/src/Web/StellaOps.Web/src/app/core/auth/dpop/dpop.service.ts index d6d4432c0..3604e716e 100644 --- a/src/Web/StellaOps.Web/src/app/core/auth/dpop/dpop.service.ts +++ b/src/Web/StellaOps.Web/src/app/core/auth/dpop/dpop.service.ts @@ -80,7 +80,8 @@ export class DpopService { new TextEncoder().encode(signingInput) ); - const joseSignature = base64UrlEncode(derToJoseSignature(signature)); + const componentSize = keyPair.algorithm === 'ES384' ? 48 : 32; + const joseSignature = base64UrlEncode(derToJoseSignature(signature, componentSize)); return `${signingInput}.${joseSignature}`; } diff --git a/src/Web/StellaOps.Web/src/app/core/auth/dpop/jose-utilities.ts b/src/Web/StellaOps.Web/src/app/core/auth/dpop/jose-utilities.ts index a6722fda0..dec8daa9a 100644 --- a/src/Web/StellaOps.Web/src/app/core/auth/dpop/jose-utilities.ts +++ b/src/Web/StellaOps.Web/src/app/core/auth/dpop/jose-utilities.ts @@ -66,42 +66,123 @@ function canonicalizeJwk(jwk: JsonWebKey): string { throw new Error(`Unsupported JWK key type: ${jwk.kty}`); } -export function derToJoseSignature(der: ArrayBuffer): Uint8Array { +/** + * Convert an ECDSA signature from DER encoding to the fixed-length JOSE/P1363 + * format (r || s, each zero-padded to componentSize bytes). + * + * Web Crypto's `crypto.subtle.sign('ECDSA', ...)` returns DER on some + * platforms/browsers and raw P1363 on others. This function handles both. + * + * The previous implementation checked only `bytes[0] === 0x30` to decide + * between formats, but raw P1363 signatures have a ~1/256 chance of their + * first byte being 0x30, causing a spurious DER parse attempt and a throw. + * + * Fix: detect raw P1363 by checking whether the byte length matches an + * expected raw size (64 for P-256, 96 for P-384) BEFORE inspecting content + * bytes. Only attempt DER parsing when the length doesn't match any known + * raw size, or when the length matches AND full DER structural validation + * confirms it really is DER. + */ +export function derToJoseSignature(der: ArrayBuffer, componentSize = 32): Uint8Array { const bytes = new Uint8Array(der); - if (bytes[0] !== 0x30) { - // Some implementations already return raw (r || s) signature bytes. - if (bytes.length === 64) { - return bytes; + const rawLength = componentSize * 2; // 64 for ES256, 96 for ES384 + + // --- Fast path: raw IEEE P1363 (r || s) --- + // If the byte count exactly equals the expected raw size AND the bytes do + // NOT form a valid DER SEQUENCE, treat them as raw P1363. + if (bytes.length === rawLength) { + // Only re-interpret as DER if full structural validation passes. + if (bytes[0] === 0x30 && isDerSignature(bytes)) { + return parseDer(bytes, componentSize); } - throw new Error('Invalid DER signature: expected sequence.'); + return bytes; } - let offset = 2; // skip SEQUENCE header and length (assume short form) - if (bytes[1] & 0x80) { - const lengthBytes = bytes[1] & 0x7f; - offset = 2 + lengthBytes; + // --- DER path --- + if (bytes[0] === 0x30) { + return parseDer(bytes, componentSize); } + throw new Error( + `Unexpected ECDSA signature format: length=${bytes.length}, first=0x${bytes[0].toString(16)}` + ); +} + +/** + * Validate whether `bytes` is a structurally valid DER-encoded ECDSA + * signature (SEQUENCE of two INTEGERs) without throwing. + */ +function isDerSignature(bytes: Uint8Array): boolean { + if (bytes.length < 8 || bytes[0] !== 0x30) return false; + + // Parse SEQUENCE length + let offset = 1; + let seqLen: number; + if (bytes[offset] & 0x80) { + const numLenBytes = bytes[offset] & 0x7f; + if (numLenBytes === 0 || numLenBytes > 2 || offset + 1 + numLenBytes > bytes.length) return false; + seqLen = 0; + for (let i = 0; i < numLenBytes; i++) { + seqLen = (seqLen << 8) | bytes[offset + 1 + i]; + } + offset += 1 + numLenBytes; + } else { + seqLen = bytes[offset]; + offset += 1; + } + + // SEQUENCE length must account for exactly the remaining bytes + if (offset + seqLen !== bytes.length) return false; + + // First INTEGER (r) + if (offset >= bytes.length || bytes[offset] !== 0x02) return false; + offset += 1; + if (offset >= bytes.length) return false; + const rLen = bytes[offset]; + offset += 1 + rLen; + + // Second INTEGER (s) + if (offset >= bytes.length || bytes[offset] !== 0x02) return false; + offset += 1; + if (offset >= bytes.length) return false; + const sLen = bytes[offset]; + offset += 1 + sLen; + + // Must consume all bytes + return offset === bytes.length; +} + +function parseDer(bytes: Uint8Array, componentSize: number): Uint8Array { + // Skip SEQUENCE tag (0x30) and length + let offset = 1; + if (bytes[offset] & 0x80) { + const numLenBytes = bytes[offset] & 0x7f; + offset += 1 + numLenBytes; + } else { + offset += 1; + } + + // Read r INTEGER if (bytes[offset] !== 0x02) { throw new Error('Invalid DER signature: expected INTEGER for r.'); } const rLength = bytes[offset + 1]; - let r: Uint8Array = bytes.slice(offset + 2, offset + 2 + rLength); + const rRaw = bytes.slice(offset + 2, offset + 2 + rLength); offset = offset + 2 + rLength; + // Read s INTEGER if (bytes[offset] !== 0x02) { throw new Error('Invalid DER signature: expected INTEGER for s.'); } const sLength = bytes[offset + 1]; - let s: Uint8Array = bytes.slice(offset + 2, offset + 2 + sLength); + const sRaw = bytes.slice(offset + 2, offset + 2 + sLength); - r = trimLeadingZeros(r); - s = trimLeadingZeros(s); + const r = trimLeadingZeros(rRaw); + const s = trimLeadingZeros(sRaw); - const targetLength = 32; - const signature = new Uint8Array(targetLength * 2); - signature.set(padStart(r, targetLength), 0); - signature.set(padStart(s, targetLength), targetLength); + const signature = new Uint8Array(componentSize * 2); + signature.set(padStart(r, componentSize), 0); + signature.set(padStart(s, componentSize), componentSize); return signature; }