fix(federation): security hardening — OID verification, atomic activation, audit on failure #501
@@ -35,7 +35,7 @@ import * as crypto from 'node:crypto';
|
|||||||
import * as fs from 'node:fs';
|
import * as fs from 'node:fs';
|
||||||
import * as https from 'node:https';
|
import * as https from 'node:https';
|
||||||
import { SignJWT, importJWK } from 'jose';
|
import { SignJWT, importJWK } from 'jose';
|
||||||
import { Pkcs10CertificateRequest } from '@peculiar/x509';
|
import { Pkcs10CertificateRequest, X509Certificate } from '@peculiar/x509';
|
||||||
import type { IssueCertRequestDto } from './ca.dto.js';
|
import type { IssueCertRequestDto } from './ca.dto.js';
|
||||||
import { IssuedCertDto } from './ca.dto.js';
|
import { IssuedCertDto } from './ca.dto.js';
|
||||||
|
|
||||||
@@ -624,6 +624,51 @@ export class CaService {
|
|||||||
|
|
||||||
const serialNumber = extractSerial(response.crt);
|
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}`);
|
this.logger.log(`Certificate issued — serial=${serialNumber} grantId=${req.grantId}`);
|
||||||
|
|
||||||
const result = new IssuedCertDto();
|
const result = new IssuedCertDto();
|
||||||
|
|||||||
@@ -14,6 +14,7 @@
|
|||||||
|
|
||||||
import {
|
import {
|
||||||
BadRequestException,
|
BadRequestException,
|
||||||
|
ConflictException,
|
||||||
GoneException,
|
GoneException,
|
||||||
Inject,
|
Inject,
|
||||||
Injectable,
|
Injectable,
|
||||||
@@ -66,6 +67,21 @@ export class EnrollmentService {
|
|||||||
*/
|
*/
|
||||||
async createToken(dto: CreateEnrollmentTokenDto): Promise<EnrollmentTokenResult> {
|
async createToken(dto: CreateEnrollmentTokenDto): Promise<EnrollmentTokenResult> {
|
||||||
const ttl = Math.min(dto.ttlSeconds, 900);
|
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 token = crypto.randomBytes(32).toString('hex');
|
||||||
const expiresAt = new Date(Date.now() + ttl * 1000);
|
const expiresAt = new Date(Date.now() + ttl * 1000);
|
||||||
|
|
||||||
@@ -99,16 +115,23 @@ export class EnrollmentService {
|
|||||||
* 8. Return { certPem, certChainPem }
|
* 8. Return { certPem, certChainPem }
|
||||||
*/
|
*/
|
||||||
async redeem(token: string, csrPem: string): Promise<RedeemResult> {
|
async redeem(token: string, csrPem: string): Promise<RedeemResult> {
|
||||||
|
// 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;
|
||||||
|
|
||||||
|
try {
|
||||||
// 1. Fetch token row
|
// 1. Fetch token row
|
||||||
const [row] = await this.db
|
const [fetchedRow] = await this.db
|
||||||
.select()
|
.select()
|
||||||
.from(federationEnrollmentTokens)
|
.from(federationEnrollmentTokens)
|
||||||
.where(eq(federationEnrollmentTokens.token, token))
|
.where(eq(federationEnrollmentTokens.token, token))
|
||||||
.limit(1);
|
.limit(1);
|
||||||
|
|
||||||
if (!row) {
|
if (!fetchedRow) {
|
||||||
throw new NotFoundException('Enrollment token not found');
|
throw new NotFoundException('Enrollment token not found');
|
||||||
}
|
}
|
||||||
|
row = fetchedRow;
|
||||||
|
|
||||||
// 2. Already used?
|
// 2. Already used?
|
||||||
if (row.usedAt !== null) {
|
if (row.usedAt !== null) {
|
||||||
@@ -144,7 +167,10 @@ export class EnrollmentService {
|
|||||||
.update(federationEnrollmentTokens)
|
.update(federationEnrollmentTokens)
|
||||||
.set({ usedAt: sql`NOW()` })
|
.set({ usedAt: sql`NOW()` })
|
||||||
.where(
|
.where(
|
||||||
and(eq(federationEnrollmentTokens.token, token), isNull(federationEnrollmentTokens.usedAt)),
|
and(
|
||||||
|
eq(federationEnrollmentTokens.token, token),
|
||||||
|
isNull(federationEnrollmentTokens.usedAt),
|
||||||
|
),
|
||||||
)
|
)
|
||||||
.returning({ token: federationEnrollmentTokens.token });
|
.returning({ token: federationEnrollmentTokens.token });
|
||||||
|
|
||||||
@@ -164,8 +190,9 @@ export class EnrollmentService {
|
|||||||
ttlSeconds: 300,
|
ttlSeconds: 300,
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
|
// HIGH-4: Log only the first 8 hex chars of the token for correlation — never log the full token.
|
||||||
this.logger.error(
|
this.logger.error(
|
||||||
`issueCert failed after token ${token} was claimed — grant ${row.grantId} is stranded pending`,
|
`issueCert failed after token ${token.slice(0, 8)}... was claimed — grant ${row.grantId} is stranded pending`,
|
||||||
err instanceof Error ? err.stack : String(err),
|
err instanceof Error ? err.stack : String(err),
|
||||||
);
|
);
|
||||||
if (err instanceof FederationScopeError) {
|
if (err instanceof FederationScopeError) {
|
||||||
@@ -177,11 +204,19 @@ export class EnrollmentService {
|
|||||||
// 7. Atomically activate grant, update peer record, and write audit log.
|
// 7. Atomically activate grant, update peer record, and write audit log.
|
||||||
const certNotAfter = this.extractCertNotAfter(issued.certPem);
|
const certNotAfter = this.extractCertNotAfter(issued.certPem);
|
||||||
await this.db.transaction(async (tx) => {
|
await this.db.transaction(async (tx) => {
|
||||||
await tx
|
// CRIT-2: Guard activation with WHERE status='pending' to prevent double-activation.
|
||||||
|
const [activated] = await tx
|
||||||
.update(federationGrants)
|
.update(federationGrants)
|
||||||
.set({ status: 'active' })
|
.set({ status: 'active' })
|
||||||
.where(eq(federationGrants.id, row.grantId));
|
.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
|
await tx
|
||||||
.update(federationPeers)
|
.update(federationPeers)
|
||||||
.set({
|
.set({
|
||||||
@@ -190,12 +225,12 @@ export class EnrollmentService {
|
|||||||
certNotAfter,
|
certNotAfter,
|
||||||
state: 'active',
|
state: 'active',
|
||||||
})
|
})
|
||||||
.where(eq(federationPeers.id, row.peerId));
|
.where(and(eq(federationPeers.id, row!.peerId), eq(federationPeers.state, 'pending')));
|
||||||
|
|
||||||
await tx.insert(federationAuditLog).values({
|
await tx.insert(federationAuditLog).values({
|
||||||
requestId: crypto.randomUUID(),
|
requestId: crypto.randomUUID(),
|
||||||
peerId: row.peerId,
|
peerId: row!.peerId,
|
||||||
grantId: row.grantId,
|
grantId: row!.grantId,
|
||||||
verb: 'enrollment',
|
verb: 'enrollment',
|
||||||
resource: 'federation_grant',
|
resource: 'federation_grant',
|
||||||
statusCode: 200,
|
statusCode: 200,
|
||||||
@@ -207,24 +242,40 @@ export class EnrollmentService {
|
|||||||
`Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
|
`Enrollment complete — peerId=${row.peerId} grantId=${row.grantId} serial=${issued.serialNumber}`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
outcome = 'allowed';
|
||||||
|
|
||||||
// 8. Return cert material
|
// 8. Return cert material
|
||||||
return {
|
return {
|
||||||
certPem: issued.certPem,
|
certPem: issued.certPem,
|
||||||
certChainPem: issued.certChainPem,
|
certChainPem: issued.certChainPem,
|
||||||
};
|
};
|
||||||
|
} catch (err) {
|
||||||
|
// 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;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Extract the notAfter date from a PEM certificate.
|
* 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 {
|
private extractCertNotAfter(certPem: string): Date {
|
||||||
try {
|
|
||||||
const cert = new X509Certificate(certPem);
|
const cert = new X509Certificate(certPem);
|
||||||
return new Date(cert.validTo);
|
return new Date(cert.validTo);
|
||||||
} catch {
|
|
||||||
// Fallback: 90 days from now
|
|
||||||
return new Date(Date.now() + 90 * 24 * 60 * 60 * 1000);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user