fix(federation): use null fallback for audit log FK cols when token row missing

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

apps/gateway/src/federation/ca.service.ts

@@ -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();

apps/gateway/src/federation/enrollment.service.ts

@@ -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);
   }
 }