fix(federation): security hardening — OID verification, atomic activation, audit on failure #501

Merged
jason.woltje merged 2 commits from fix/federation-m2-security into main 2026-04-22 06:02:53 +00:00
2 changed files with 212 additions and 116 deletions

View File

@@ -35,7 +35,7 @@ import * as crypto from 'node:crypto';
import * as fs from 'node:fs';
import * as https from 'node:https';
import { SignJWT, importJWK } from 'jose';
import { Pkcs10CertificateRequest } from '@peculiar/x509';
import { Pkcs10CertificateRequest, X509Certificate } from '@peculiar/x509';
import type { IssueCertRequestDto } from './ca.dto.js';
import { IssuedCertDto } from './ca.dto.js';
@@ -624,6 +624,51 @@ export class CaService {
const serialNumber = extractSerial(response.crt);
// CRIT-1: Verify the issued certificate contains both Mosaic OID extensions
// with the correct values. Step-CA's federation.tpl encodes each as an ASN.1
// UTF8String TLV: tag 0x0C + 1-byte length + UUID bytes. We skip 2 bytes
// (tag + length) to extract the raw UUID string.
const issuedCert = new X509Certificate(response.crt);
const decoder = new TextDecoder();
const grantIdExt = issuedCert.getExtension('1.3.6.1.4.1.99999.1');
if (!grantIdExt) {
throw new CaServiceError(
'Issued certificate is missing required Mosaic OID: mosaic_grant_id',
'The Step-CA federation.tpl template did not embed OID 1.3.6.1.4.1.99999.1. Check the provisioner template configuration.',
undefined,
'OID_MISSING',
);
}
const grantIdInCert = decoder.decode(grantIdExt.value.slice(2));
if (grantIdInCert !== req.grantId) {
throw new CaServiceError(
`Issued certificate mosaic_grant_id mismatch: expected ${req.grantId}, got ${grantIdInCert}`,
'The Step-CA issued a certificate with a different grant ID than requested. This may indicate a provisioner misconfiguration or a MITM.',
undefined,
'OID_MISMATCH',
);
}
const subjectUserIdExt = issuedCert.getExtension('1.3.6.1.4.1.99999.2');
if (!subjectUserIdExt) {
throw new CaServiceError(
'Issued certificate is missing required Mosaic OID: mosaic_subject_user_id',
'The Step-CA federation.tpl template did not embed OID 1.3.6.1.4.1.99999.2. Check the provisioner template configuration.',
undefined,
'OID_MISSING',
);
}
const subjectUserIdInCert = decoder.decode(subjectUserIdExt.value.slice(2));
if (subjectUserIdInCert !== req.subjectUserId) {
throw new CaServiceError(
`Issued certificate mosaic_subject_user_id mismatch: expected ${req.subjectUserId}, got ${subjectUserIdInCert}`,
'The Step-CA issued a certificate with a different subject user ID than requested. This may indicate a provisioner misconfiguration or a MITM.',
undefined,
'OID_MISMATCH',
);
}
this.logger.log(`Certificate issued — serial=${serialNumber} grantId=${req.grantId}`);
const result = new IssuedCertDto();

View File

@@ -14,6 +14,7 @@
import {
BadRequestException,
ConflictException,
GoneException,
Inject,
Injectable,
@@ -66,6 +67,21 @@ export class EnrollmentService {
*/
async createToken(dto: CreateEnrollmentTokenDto): Promise<EnrollmentTokenResult> {
const ttl = Math.min(dto.ttlSeconds, 900);
// MED-3: Verify the grantId ↔ peerId binding — prevents attacker from
// cross-wiring grants to attacker-controlled peers.
const [grant] = await this.db
.select({ peerId: federationGrants.peerId })
.from(federationGrants)
.where(eq(federationGrants.id, dto.grantId))
.limit(1);
if (!grant) {
throw new NotFoundException(`Grant ${dto.grantId} not found`);
}
if (grant.peerId !== dto.peerId) {
throw new BadRequestException(`peerId does not match the grant's registered peer`);
}
const token = crypto.randomBytes(32).toString('hex');
const expiresAt = new Date(Date.now() + ttl * 1000);
@@ -99,132 +115,167 @@ export class EnrollmentService {
* 8. Return { certPem, certChainPem }
*/
async redeem(token: string, csrPem: string): Promise<RedeemResult> {
// 1. Fetch token row
const [row] = await this.db
.select()
.from(federationEnrollmentTokens)
.where(eq(federationEnrollmentTokens.token, token))
.limit(1);
// HIGH-5: Track outcome so we can write a failure audit row on any error.
let outcome: 'allowed' | 'denied' = 'denied';
// row may be undefined if the token is not found — used defensively in catch.
let row: typeof federationEnrollmentTokens.$inferSelect | undefined;
if (!row) {
throw new NotFoundException('Enrollment token not found');
}
// 2. Already used?
if (row.usedAt !== null) {
throw new GoneException('Enrollment token has already been used');
}
// 3. Expired?
if (row.expiresAt < new Date()) {
throw new GoneException('Enrollment token has expired');
}
// 4. Load grant and verify it is still pending
let grant;
try {
grant = await this.grantsService.getGrant(row.grantId);
// 1. Fetch token row
const [fetchedRow] = await this.db
.select()
.from(federationEnrollmentTokens)
.where(eq(federationEnrollmentTokens.token, token))
.limit(1);
if (!fetchedRow) {
throw new NotFoundException('Enrollment token not found');
}
row = fetchedRow;
// 2. Already used?
if (row.usedAt !== null) {
throw new GoneException('Enrollment token has already been used');
}
// 3. Expired?
if (row.expiresAt < new Date()) {
throw new GoneException('Enrollment token has expired');
}
// 4. Load grant and verify it is still pending
let grant;
try {
grant = await this.grantsService.getGrant(row.grantId);
} catch (err) {
if (err instanceof FederationScopeError) {
throw new BadRequestException(err.message);
}
throw err;
}
if (grant.status !== 'pending') {
throw new GoneException(
`Grant ${row.grantId} is no longer pending (status: ${grant.status})`,
);
}
// 5. Atomically claim the token BEFORE cert issuance to prevent double-minting.
// WHERE used_at IS NULL ensures only one concurrent request wins.
// Using .returning() works on both node-postgres and PGlite without rowCount inspection.
const claimed = await this.db
.update(federationEnrollmentTokens)
.set({ usedAt: sql`NOW()` })
.where(
and(
eq(federationEnrollmentTokens.token, token),
isNull(federationEnrollmentTokens.usedAt),
),
)
.returning({ token: federationEnrollmentTokens.token });
if (claimed.length === 0) {
throw new GoneException('Enrollment token has already been used (concurrent request)');
}
// 6. Issue certificate via CaService (network call — outside any transaction).
// If this throws, the token is already consumed. The grant stays pending.
// Admin must revoke the grant and create a new one.
let issued;
try {
issued = await this.caService.issueCert({
csrPem,
grantId: row.grantId,
subjectUserId: grant.subjectUserId,
ttlSeconds: 300,
});
} catch (err) {
// HIGH-4: Log only the first 8 hex chars of the token for correlation — never log the full token.
this.logger.error(
`issueCert failed after token ${token.slice(0, 8)}... was claimed — grant ${row.grantId} is stranded pending`,
err instanceof Error ? err.stack : String(err),
);
if (err instanceof FederationScopeError) {
throw new BadRequestException((err as Error).message);
}
throw err;
}
// 7. Atomically activate grant, update peer record, and write audit log.
const certNotAfter = this.extractCertNotAfter(issued.certPem);
await this.db.transaction(async (tx) => {
// CRIT-2: Guard activation with WHERE status='pending' to prevent double-activation.
const [activated] = await tx
.update(federationGrants)
.set({ status: 'active' })
.where(and(eq(federationGrants.id, row!.grantId), eq(federationGrants.status, 'pending')))
.returning({ id: federationGrants.id });
if (!activated) {
throw new ConflictException(
`Grant ${row!.grantId} is no longer pending — cannot activate`,
);
}
// CRIT-2: Guard peer update with WHERE state='pending'.
await tx
.update(federationPeers)
.set({
certPem: issued.certPem,
certSerial: issued.serialNumber,
certNotAfter,
state: 'active',
})
.where(and(eq(federationPeers.id, row!.peerId), eq(federationPeers.state, 'pending')));
await tx.insert(federationAuditLog).values({
requestId: crypto.randomUUID(),
peerId: row!.peerId,
grantId: row!.grantId,
verb: 'enrollment',
resource: 'federation_grant',
statusCode: 200,
outcome: 'allowed',
});
});
this.logger.log(
`Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
);
outcome = 'allowed';
// 8. Return cert material
return {
certPem: issued.certPem,
certChainPem: issued.certChainPem,
};
} catch (err) {
if (err instanceof FederationScopeError) {
throw new BadRequestException(err.message);
// HIGH-5: Best-effort audit write on failure — do not let this throw.
if (outcome === 'denied') {
await this.db
.insert(federationAuditLog)
.values({
requestId: crypto.randomUUID(),
peerId: row?.peerId ?? null,
grantId: row?.grantId ?? null,
verb: 'enrollment',
resource: 'federation_grant',
statusCode:
err instanceof GoneException ? 410 : err instanceof NotFoundException ? 404 : 500,
outcome: 'denied',
})
.catch(() => {});
}
throw err;
}
if (grant.status !== 'pending') {
throw new GoneException(
`Grant ${row.grantId} is no longer pending (status: ${grant.status})`,
);
}
// 5. Atomically claim the token BEFORE cert issuance to prevent double-minting.
// WHERE used_at IS NULL ensures only one concurrent request wins.
// Using .returning() works on both node-postgres and PGlite without rowCount inspection.
const claimed = await this.db
.update(federationEnrollmentTokens)
.set({ usedAt: sql`NOW()` })
.where(
and(eq(federationEnrollmentTokens.token, token), isNull(federationEnrollmentTokens.usedAt)),
)
.returning({ token: federationEnrollmentTokens.token });
if (claimed.length === 0) {
throw new GoneException('Enrollment token has already been used (concurrent request)');
}
// 6. Issue certificate via CaService (network call — outside any transaction).
// If this throws, the token is already consumed. The grant stays pending.
// Admin must revoke the grant and create a new one.
let issued;
try {
issued = await this.caService.issueCert({
csrPem,
grantId: row.grantId,
subjectUserId: grant.subjectUserId,
ttlSeconds: 300,
});
} catch (err) {
this.logger.error(
`issueCert failed after token ${token} was claimed — grant ${row.grantId} is stranded pending`,
err instanceof Error ? err.stack : String(err),
);
if (err instanceof FederationScopeError) {
throw new BadRequestException((err as Error).message);
}
throw err;
}
// 7. Atomically activate grant, update peer record, and write audit log.
const certNotAfter = this.extractCertNotAfter(issued.certPem);
await this.db.transaction(async (tx) => {
await tx
.update(federationGrants)
.set({ status: 'active' })
.where(eq(federationGrants.id, row.grantId));
await tx
.update(federationPeers)
.set({
certPem: issued.certPem,
certSerial: issued.serialNumber,
certNotAfter,
state: 'active',
})
.where(eq(federationPeers.id, row.peerId));
await tx.insert(federationAuditLog).values({
requestId: crypto.randomUUID(),
peerId: row.peerId,
grantId: row.grantId,
verb: 'enrollment',
resource: 'federation_grant',
statusCode: 200,
outcome: 'allowed',
});
});
this.logger.log(
`Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
);
// 8. Return cert material
return {
certPem: issued.certPem,
certChainPem: issued.certChainPem,
};
}
/**
* Extract the notAfter date from a PEM certificate.
* Falls back to 90 days from now if parsing fails.
* HIGH-2: No silent fallback — a cert that cannot be parsed should fail loud.
*/
private extractCertNotAfter(certPem: string): Date {
try {
const cert = new X509Certificate(certPem);
return new Date(cert.validTo);
} catch {
// Fallback: 90 days from now
return new Date(Date.now() + 90 * 24 * 60 * 60 * 1000);
}
const cert = new X509Certificate(certPem);
return new Date(cert.validTo);
}
}