fix: resolve certificate/key mismatch in P12/JKS downloads

Fixes GitHub issue #1: Private key in P12/JKS downloads didn't match
the certificate's public key because the KMS certify operation wasn't
using the pre-created key pair.

Root cause:
- When issuing certificates, a key pair was created first via createKeyPair()
- But the certify() call didn't reference this key pair via UniqueIdentifier
- KMS would generate a NEW key pair for the certificate
- The stored kmsKeyId pointed to the original key pair
- Downloads fetched the original private key, which didn't match the
  certificate's actual public key

Fix:
- Add UniqueIdentifier to certify() request to reference existing public key
- Add PublicKeyLink attribute as backup
- Disable key reuse for renewals (was already broken due to same issue)
- Add test to verify certificate/private key modulus match in P12

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

Author: oriolrius

backend/src/kms/client.ts

@@ -410,6 +410,17 @@ export class KMSClient {
   }): Promise<CertificateInfo> {
     const requestValue: KMIPElement[] = [];
 
+    // CRITICAL: Add UniqueIdentifier for existing public key
+    // Per KMIP spec, when certifying an existing public key, the UniqueIdentifier
+    // at the request level identifies which public key to certify
+    if (options.publicKeyId) {
+      requestValue.push({
+        tag: "UniqueIdentifier",
+        type: "TextString",
+        value: options.publicKeyId,
+      });
+    }
+
     // Add CSR if provided
     if (options.csr) {
       requestValue.push({
@@ -471,6 +482,27 @@ export class KMSClient {
       });
     }
 
+    // Add Link to public key (to use existing key pair instead of generating new one)
+    // This is CRITICAL: without this link, KMS generates a new key pair for the certificate
+    // which causes a mismatch between the stored private key and the certificate's public key
+    if (options.publicKeyId) {
+      attributes.push({
+        tag: "Link",
+        value: [
+          {
+            tag: "LinkType",
+            type: "Enumeration",
+            value: "PublicKeyLink",
+          },
+          {
+            tag: "LinkedObjectIdentifier",
+            type: "TextString",
+            value: options.publicKeyId,
+          },
+        ],
+      });
+    }
+
     // Add subject name using CertificateAttributes structure with individual fields
     if (options.subjectName) {
       // Parse the DN string into individual components

backend/src/rest/routes/certificate-download.test.ts

@@ -349,7 +349,7 @@ describe('Certificate Download - KMS Integration', () => {
   });
 
   describe('PKCS#12 Bundle Downloads', () => {
-    it('should download PKCS#12 bundle (p12)', async (ctx) => {
+    it('should download PKCS#12 bundle (p12) with matching certificate and key', async (ctx) => {
       if (!kmsAvailable) {
         ctx.skip();
         return;
@@ -380,7 +380,19 @@ describe('Certificate Download - KMS Integration', () => {
       expect(keyBags[forge.pki.oids.pkcs8ShroudedKeyBag]).toBeDefined();
       expect(keyBags[forge.pki.oids.pkcs8ShroudedKeyBag]!.length).toBeGreaterThan(0);
 
-      console.log('✅ p12 format download works');
+      // CRITICAL: Verify that certificate and private key modulus match
+      // This validates the fix for GitHub issue #1: key mismatch in P12/JKS downloads
+      const cert = certBags[forge.pki.oids.certBag]![0].cert!;
+      const privateKey = keyBags[forge.pki.oids.pkcs8ShroudedKeyBag]![0].key as forge.pki.rsa.PrivateKey;
+
+      // Get modulus from certificate's public key and from private key
+      const certModulus = (cert.publicKey as forge.pki.rsa.PublicKey).n.toString(16);
+      const keyModulus = privateKey.n.toString(16);
+
+      // The modulus must match - this is the core validation that the key pair is correct
+      expect(certModulus).toBe(keyModulus);
+
+      console.log('✅ p12 format download works with matching certificate and key modulus');
     });
 
     it('should download PKCS#12 bundle (pfx alias)', async (ctx) => {

backend/src/services/certificate.service.ts

@@ -850,31 +850,25 @@ export class CertificateService {
       const validityDays = params.validityDays ||
         Math.ceil((originalCert.notAfter.getTime() - originalCert.notBefore.getTime()) / (1000 * 60 * 60 * 24));
 
-      let kmsKeyId: string;
-      let publicKeyId: string;
-
-      if (params.generateNewKey) {
-        logger.info({ newCertId, originalCertId: params.id }, 'Creating new key pair for certificate renewal');
-
-        const keyPair = await kmsService.createKeyPair({
-          sizeInBits: 2048,
-          tags: [],
-          purpose: 'certificate',
-          entityId: newCertId,
-        });
+      // Always generate a new key pair for renewal
+      // Key reuse is not supported because we don't store the public key ID separately,
+      // and deriving it from the private key ID is unreliable
+      if (!params.generateNewKey) {
+        logger.warn({ newCertId, originalCertId: params.id },
+          'Key reuse requested but not supported - generating new key pair instead');
+      }
 
-        kmsKeyId = keyPair.privateKeyId;
-        publicKeyId = keyPair.publicKeyId;
-      } else {
-        logger.info({ newCertId, originalCertId: params.id }, 'Reusing existing key pair for certificate renewal');
+      logger.info({ newCertId, originalCertId: params.id }, 'Creating new key pair for certificate renewal');
 
-        if (!originalCert.kmsKeyId) {
-          throw new CertificateNoKeyError(params.id);
-        }
+      const keyPair = await kmsService.createKeyPair({
+        sizeInBits: 2048,
+        tags: [],
+        purpose: 'certificate',
+        entityId: newCertId,
+      });
 
-        kmsKeyId = originalCert.kmsKeyId;
-        publicKeyId = kmsKeyId.replace('-private', '-public');
-      }
+      const kmsKeyId = keyPair.privateKeyId;
+      const publicKeyId = keyPair.publicKeyId;
 
       const subjectName = formatDN(subjectDN);
       logger.info({ newCertId, subjectName, caId: originalCert.caId }, 'Signing renewed certificate via KMS');
@@ -907,7 +901,7 @@ export class CertificateService {
         notBefore: certMetadata.validity.notBefore,
         notAfter: certMetadata.validity.notAfter,
         kmsCertificateId: certInfo.certificateId,
-        kmsKeyId: params.generateNewKey ? kmsKeyId : null,
+        kmsKeyId: kmsKeyId, // Always store since we always generate new keys
         status: 'active',
         sanDns: sanDns ? JSON.stringify(sanDns) : null,
         sanIp: sanIp ? JSON.stringify(sanIp) : null,

backend/src/trpc/procedures/certificate-renew.test.ts

@@ -156,7 +156,7 @@ describe("Certificate Renew - KMS Integration", () => {
     console.log(`✅ Certificate renewed with new key: ${result.id}`);
   });
 
-  it("should renew a certificate with key reuse (young cert)", async (ctx) => {
+  it("should renew a certificate (generateNewKey=false still generates new key)", async (ctx) => {
     if (!kmsAvailable) {
       ctx.skip();
       return;
@@ -186,10 +186,11 @@ describe("Certificate Renew - KMS Integration", () => {
 
     createdCertIds.push(freshCert.id);
 
-    // Now renew with key reuse
+    // Now renew - note: key reuse is not supported, a new key will be generated
+    // even when generateNewKey: false is specified
     const result = await caller.certificate.renew({
       id: freshCert.id,
-      generateNewKey: false, // Reuse existing key
+      generateNewKey: false, // Ignored - new key is always generated
       validityDays: 365,
     });
 
@@ -202,12 +203,12 @@ describe("Certificate Renew - KMS Integration", () => {
     // Store for cleanup
     createdCertIds.push(result.id);
 
-    // Verify that kmsKeyId is null (reused key, not stored again)
+    // Verify that kmsKeyId is set (new key is always generated due to key reuse not being supported)
     const dbCert = await db.select().from(certificates).where(eq(certificates.id, result.id));
     expect(dbCert).toHaveLength(1);
-    expect(dbCert[0].kmsKeyId).toBeNull(); // Key was reused, not regenerated
+    expect(dbCert[0].kmsKeyId).not.toBeNull(); // New key is always generated
 
-    console.log(`✅ Certificate renewed with key reuse: ${result.id}`);
+    console.log(`✅ Certificate renewed (new key generated): ${result.id}`);
   });
 
   it("should renew a certificate with updated subject info", async (ctx) => {

backend/src/trpc/procedures/certificate.ts

@@ -1001,42 +1001,27 @@ export const certificateRouter = router({
         const validityDays = input.validityDays ||
           Math.ceil((originalCert.notAfter.getTime() - originalCert.notBefore.getTime()) / (1000 * 60 * 60 * 24));
 
-        let kmsKeyId: string;
-        let publicKeyId: string;
-
-        if (input.generateNewKey) {
-          // Generate new key pair in KMS
-          logger.info({ newCertId, originalCertId: input.id }, 'Creating new key pair for certificate renewal');
-
-          // Determine key size from original certificate
-          // For simplicity, defaulting to RSA-2048. In production, you'd extract this from the original cert
-          const keyPair = await kmsService.createKeyPair({
-            sizeInBits: 2048,
-            tags: [],
-            purpose: 'certificate',
-            entityId: newCertId,
-          });
-
-          kmsKeyId = keyPair.privateKeyId;
-          publicKeyId = keyPair.publicKeyId;
-        } else {
-          // Reuse existing key pair
-          logger.info({ newCertId, originalCertId: input.id }, 'Reusing existing key pair for certificate renewal');
+        // Always generate a new key pair for renewal
+        // Key reuse is not supported because we don't store the public key ID separately,
+        // and deriving it from the private key ID is unreliable
+        if (!input.generateNewKey) {
+          logger.warn({ newCertId, originalCertId: input.id },
+            'Key reuse requested but not supported - generating new key pair instead');
+        }
 
-          if (!originalCert.kmsKeyId) {
-            throw new TRPCError({
-              code: 'BAD_REQUEST',
-              message: 'Original certificate has no associated KMS key to reuse',
-            });
-          }
+        logger.info({ newCertId, originalCertId: input.id }, 'Creating new key pair for certificate renewal');
 
-          kmsKeyId = originalCert.kmsKeyId;
+        // Determine key size from original certificate
+        // For simplicity, defaulting to RSA-2048. In production, you'd extract this from the original cert
+        const keyPair = await kmsService.createKeyPair({
+          sizeInBits: 2048,
+          tags: [],
+          purpose: 'certificate',
+          entityId: newCertId,
+        });
 
-          // Get the public key ID from KMS
-          // Note: This assumes the KMS service has a method to get public key from private key
-          // In production, you might need to store the public key ID alongside the private key ID
-          publicKeyId = kmsKeyId.replace('-private', '-public'); // Simplified assumption
-        }
+        const kmsKeyId = keyPair.privateKeyId;
+        const publicKeyId = keyPair.publicKeyId;
 
         const subjectName = formatDN(subjectDN);
         logger.info({ newCertId, subjectName, caId: originalCert.caId }, 'Signing renewed certificate via KMS');
@@ -1069,7 +1054,7 @@ export const certificateRouter = router({
           notBefore: certMetadata.validity.notBefore,
           notAfter: certMetadata.validity.notAfter,
           kmsCertificateId: certInfo.certificateId,
-          kmsKeyId: input.generateNewKey ? kmsKeyId : null,
+          kmsKeyId: kmsKeyId, // Always store since we always generate new keys
           status: 'active',
           sanDns: sanDns ? JSON.stringify(sanDns) : null,
           sanIp: sanIp ? JSON.stringify(sanIp) : null,
@@ -2594,7 +2579,7 @@ export const certificateRouter = router({
             notBefore: certMetadata.validity.notBefore,
             notAfter: certMetadata.validity.notAfter,
             kmsCertificateId: certInfo.certificateId,
-            kmsKeyId: input.generateNewKey ? kmsKeyId : null,
+            kmsKeyId: kmsKeyId, // Always store since we always generate new keys
             status: 'active',
             sanDns: sanDns ? JSON.stringify(sanDns) : null,
             sanIp: sanIp ? JSON.stringify(sanIp) : null,