fix(quick-260921-iwr): drei bag.cert-Zusicherungen in cert-manager.service.ts zu echten Waechtern gemacht
node-forge setzt bag.cert bei einem wohlgeformten, aber nicht als X.509
lesbaren Zertifikats-Bag auf null (lib/pkcs12.js Zeile 703-709). Die drei
Zusicherungen in parseCert, mergeCerts und convertCert behaupteten also
etwas Falsches, obwohl die Folge bereits behandelt war: alle vier Pfade
antworteten schon vorher mit 400, nie mit 500 (mit einer selbst gebauten
83-Byte-PFX nachgemessen).
Ersetzt die drei Zusicherungen durch ausdrueckliche Pruefungen mit
praeziser BadRequestException. In mergeCerts wird kein Zertifikat mehr
verschluckt: alle Bags werden auf Vollstaendigkeit geprueft, bevor die
Liste ueber ein Typpraedikat zurueckgegeben wird.
RED-Tests zuerst geschrieben und mit den heutigen Meldungen ("Failed to
extract certificate details" / "Failed to create merged certificate
output" / "Failed to convert certificate to pem: serialization error")
rot bestaetigt, dann die Waechter ergaenzt: GREEN.
Verhaltensaenderung ausdruecklich beabsichtigt (D-02): nur der Text der
Fehlermeldung fuer diese eine Eingabeklasse aendert sich, der Statuscode
bleibt in allen vier Pfaden 400.
NONNULL 11 -> 8, davon 3 in cert-manager.service.ts (Zeilen 133/516/661
bleiben unveraendert — durch Hash-Laenge bzw. vorgelagerte Passwort-
Pruefung garantiert).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPPB4ApQxzSU1rwV2Ffj9J
This commit is contained in:
@@ -526,3 +526,253 @@ describe('convertCert', () => {
|
|||||||
).rejects.toThrow(BadRequestException);
|
).rejects.toThrow(BadRequestException);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// PFX mit unlesbarem Zertifikats-Bag — bag.cert = null (quick-260921-iwr, D-01/D-04)
|
||||||
|
//
|
||||||
|
// node-forge setzt bag.cert bei einem wohlgeformten, aber nicht als X.509
|
||||||
|
// lesbaren Zertifikats-Bag auf null (lib/pkcs12.js Zeile 703-709: der Fehler
|
||||||
|
// aus certificateFromAsn1 wird abgefangen und durch bag.cert = null ersetzt).
|
||||||
|
// Die drei betroffenen Zusicherungen (parseCert Zeile 207, mergeCerts Zeile
|
||||||
|
// 469, convertCert Zeile 602) behaupten also etwas, das node-forge selbst
|
||||||
|
// widerlegt. Alle vier Pfade antworteten schon vorher mit 400 statt 500 — die
|
||||||
|
// hier gepruefte Aenderung ist die Meldung, nicht der Statuscode (D-02).
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('PFX mit unlesbarem Zertifikats-Bag (bag.cert = null)', () => {
|
||||||
|
let malformedPfxBuffer: Buffer;
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
// Handgebaute ~83-Byte-PFX-Datei mit forge.asn1: wohlgeformte DER-Struktur
|
||||||
|
// von aussen nach innen (PFX -> ContentInfo -> AuthenticatedSafe ->
|
||||||
|
// ContentInfo -> SafeContents -> SafeBag -> CertBag), deren
|
||||||
|
// Zertifikats-Bag-Inhalt aber nur "SEQUENCE { INTEGER 1 }" ist — kein
|
||||||
|
// X.509-Zertifikat. Passwort ist die leere Zeichenkette.
|
||||||
|
const fakeCertAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[forge.asn1.create(forge.asn1.Class.UNIVERSAL, forge.asn1.Type.INTEGER, false, String.fromCharCode(1))],
|
||||||
|
);
|
||||||
|
const fakeCertDer = forge.asn1.toDer(fakeCertAsn1).getBytes();
|
||||||
|
|
||||||
|
const certBagAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[
|
||||||
|
forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.OID,
|
||||||
|
false,
|
||||||
|
forge.asn1.oidToDer(forge.pki.oids.x509Certificate).getBytes(),
|
||||||
|
),
|
||||||
|
forge.asn1.create(forge.asn1.Class.CONTEXT_SPECIFIC, 0, true, [
|
||||||
|
forge.asn1.create(forge.asn1.Class.UNIVERSAL, forge.asn1.Type.OCTETSTRING, false, fakeCertDer),
|
||||||
|
]),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
const safeBagAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[
|
||||||
|
forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.OID,
|
||||||
|
false,
|
||||||
|
forge.asn1.oidToDer(forge.pki.oids.certBag).getBytes(),
|
||||||
|
),
|
||||||
|
forge.asn1.create(forge.asn1.Class.CONTEXT_SPECIFIC, 0, true, [certBagAsn1]),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
const safeContentsAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[safeBagAsn1],
|
||||||
|
);
|
||||||
|
const safeContentsDer = forge.asn1.toDer(safeContentsAsn1).getBytes();
|
||||||
|
|
||||||
|
const innerContentInfoAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[
|
||||||
|
forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.OID,
|
||||||
|
false,
|
||||||
|
forge.asn1.oidToDer(forge.pki.oids.data).getBytes(),
|
||||||
|
),
|
||||||
|
forge.asn1.create(forge.asn1.Class.CONTEXT_SPECIFIC, 0, true, [
|
||||||
|
forge.asn1.create(forge.asn1.Class.UNIVERSAL, forge.asn1.Type.OCTETSTRING, false, safeContentsDer),
|
||||||
|
]),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
const authSafeAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[innerContentInfoAsn1],
|
||||||
|
);
|
||||||
|
const authSafeDer = forge.asn1.toDer(authSafeAsn1).getBytes();
|
||||||
|
|
||||||
|
const outerContentInfoAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[
|
||||||
|
forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.OID,
|
||||||
|
false,
|
||||||
|
forge.asn1.oidToDer(forge.pki.oids.data).getBytes(),
|
||||||
|
),
|
||||||
|
forge.asn1.create(forge.asn1.Class.CONTEXT_SPECIFIC, 0, true, [
|
||||||
|
forge.asn1.create(forge.asn1.Class.UNIVERSAL, forge.asn1.Type.OCTETSTRING, false, authSafeDer),
|
||||||
|
]),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
const pfxAsn1 = forge.asn1.create(
|
||||||
|
forge.asn1.Class.UNIVERSAL,
|
||||||
|
forge.asn1.Type.SEQUENCE,
|
||||||
|
true,
|
||||||
|
[
|
||||||
|
forge.asn1.create(forge.asn1.Class.UNIVERSAL, forge.asn1.Type.INTEGER, false, String.fromCharCode(3)),
|
||||||
|
outerContentInfoAsn1,
|
||||||
|
],
|
||||||
|
);
|
||||||
|
|
||||||
|
const pfxDer = forge.asn1.toDer(pfxAsn1).getBytes();
|
||||||
|
malformedPfxBuffer = Buffer.from(pfxDer, 'binary');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('Vorbedingung: forge liefert genau einen Bag mit cert=null fuer diese Datei (belegt, dass der Waechter den gemeinten Fall trifft)', () => {
|
||||||
|
expect(malformedPfxBuffer.length).toBe(83);
|
||||||
|
|
||||||
|
const p12Asn1 = forge.asn1.fromDer(
|
||||||
|
forge.util.createBuffer(malformedPfxBuffer.toString('binary')),
|
||||||
|
);
|
||||||
|
const p12 = forge.pkcs12.pkcs12FromAsn1(p12Asn1, '');
|
||||||
|
const certBags = p12.getBags({ bagType: forge.pki.oids.certBag });
|
||||||
|
const bags = certBags[forge.pki.oids.certBag] ?? [];
|
||||||
|
|
||||||
|
expect(bags).toHaveLength(1);
|
||||||
|
expect(bags[0].cert).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('parseCert: wirft 400 mit einer Meldung, die den Zertifikats-Bag benennt, statt der irrefuehrenden "Failed to extract certificate details"', async () => {
|
||||||
|
const service = new CertManagerService();
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.parseCert({
|
||||||
|
file: { originalname: 'bad.pfx', buffer: malformedPfxBuffer },
|
||||||
|
password: '',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(BadRequestException);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.parseCert({
|
||||||
|
file: { originalname: 'bad.pfx', buffer: malformedPfxBuffer },
|
||||||
|
password: '',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/certificate bag/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('mergeCerts (outputFormat pem): wirft 400 mit derselben praezisen Aussage statt "Failed to create merged certificate output"', async () => {
|
||||||
|
const service = new CertManagerService();
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.mergeCerts({
|
||||||
|
files: [{ originalname: 'bad.pfx', buffer: malformedPfxBuffer }],
|
||||||
|
outputFormat: 'pem',
|
||||||
|
password: '',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(BadRequestException);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.mergeCerts({
|
||||||
|
files: [{ originalname: 'bad.pfx', buffer: malformedPfxBuffer }],
|
||||||
|
outputFormat: 'pem',
|
||||||
|
password: '',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/certificate bag/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('mergeCerts (outputFormat pfx): wirft 400 mit derselben praezisen Aussage statt "Failed to create merged certificate output"', async () => {
|
||||||
|
const service = new CertManagerService();
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.mergeCerts({
|
||||||
|
files: [{ originalname: 'bad.pfx', buffer: malformedPfxBuffer }],
|
||||||
|
outputFormat: 'pfx',
|
||||||
|
password: 'secret',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(BadRequestException);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.mergeCerts({
|
||||||
|
files: [{ originalname: 'bad.pfx', buffer: malformedPfxBuffer }],
|
||||||
|
outputFormat: 'pfx',
|
||||||
|
password: 'secret',
|
||||||
|
}),
|
||||||
|
).rejects.toThrow(/certificate bag/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('convertCert: wirft 400 mit einer Meldung, die den Zertifikats-Bag benennt, statt der irrefuehrenden "Failed to convert certificate to pem: serialization error"', async () => {
|
||||||
|
const service = new CertManagerService();
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
service.convertCert({
|
||||||
|
file: { originalname: 'bad.pfx', buffer: malformedPfxBuffer },
|
||||||
|
password: '',
|
||||||
|
targetFormat: 'pem',
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
} as any),
|
||||||
|
).rejects.toThrow(BadRequestException);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
service.convertCert({
|
||||||
|
file: { originalname: 'bad.pfx', buffer: malformedPfxBuffer },
|
||||||
|
password: '',
|
||||||
|
targetFormat: 'pem',
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
} as any),
|
||||||
|
).rejects.toThrow(/certificate bag/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('eine gueltige PFX-Datei verhaelt sich unveraendert: parseCert liefert weiterhin die CertDetails', async () => {
|
||||||
|
const service = new CertManagerService();
|
||||||
|
|
||||||
|
const keys = forge.pki.rsa.generateKeyPair(1024);
|
||||||
|
const cert = forge.pki.createCertificate();
|
||||||
|
cert.publicKey = keys.publicKey;
|
||||||
|
cert.serialNumber = '01';
|
||||||
|
cert.validity.notBefore = new Date();
|
||||||
|
cert.validity.notAfter = new Date();
|
||||||
|
cert.validity.notAfter.setFullYear(cert.validity.notBefore.getFullYear() + 1);
|
||||||
|
const attrs = [{ name: 'commonName', value: 'valid-pfx.example.com' }];
|
||||||
|
cert.setSubject(attrs);
|
||||||
|
cert.setIssuer(attrs);
|
||||||
|
cert.sign(keys.privateKey, forge.md.sha256.create());
|
||||||
|
|
||||||
|
const p12Asn1 = forge.pkcs12.toPkcs12Asn1(keys.privateKey, [cert], 'secret', {
|
||||||
|
algorithm: '3des',
|
||||||
|
});
|
||||||
|
const validPfxBuffer = Buffer.from(forge.asn1.toDer(p12Asn1).getBytes(), 'binary');
|
||||||
|
|
||||||
|
const result = await (service.parseCert({
|
||||||
|
file: { originalname: 'valid.pfx', buffer: validPfxBuffer },
|
||||||
|
password: 'secret',
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
}) as Promise<any>);
|
||||||
|
|
||||||
|
expect(result.subject.cn).toBe('valid-pfx.example.com');
|
||||||
|
}, 15000);
|
||||||
|
});
|
||||||
|
|||||||
@@ -204,7 +204,17 @@ export class CertManagerService {
|
|||||||
if (bags.length === 0) {
|
if (bags.length === 0) {
|
||||||
throw new Error('No certificate bag found in PFX/PKCS12');
|
throw new Error('No certificate bag found in PFX/PKCS12');
|
||||||
}
|
}
|
||||||
cert = bags[0].cert!;
|
// node-forge sets bag.cert to null when the bag's content parses as
|
||||||
|
// valid DER but is not a readable X.509 certificate (lib/pkcs12.js
|
||||||
|
// certBag decoder). An explicit guard here — not an assertion — so
|
||||||
|
// the 400 names the real cause (quick-260921-iwr, D-01/D-04).
|
||||||
|
const parsedCert = bags[0].cert;
|
||||||
|
if (!parsedCert) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Certificate bag in PFX/PKCS12 does not contain a readable X.509 certificate',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
cert = parsedCert;
|
||||||
} else {
|
} else {
|
||||||
// ── P7B/PKCS7 file — PEM-wrapped or binary DER (Pitfall 4) ────────
|
// ── P7B/PKCS7 file — PEM-wrapped or binary DER (Pitfall 4) ────────
|
||||||
const isPemP7b = (file.buffer as Buffer)
|
const isPemP7b = (file.buffer as Buffer)
|
||||||
@@ -466,7 +476,18 @@ export class CertManagerService {
|
|||||||
const p12 = forge.pkcs12.pkcs12FromAsn1(p12Asn1, password ?? '');
|
const p12 = forge.pkcs12.pkcs12FromAsn1(p12Asn1, password ?? '');
|
||||||
const certBags = p12.getBags({ bagType: forge.pki.oids.certBag });
|
const certBags = p12.getBags({ bagType: forge.pki.oids.certBag });
|
||||||
const bags = certBags[forge.pki.oids.certBag] ?? [];
|
const bags = certBags[forge.pki.oids.certBag] ?? [];
|
||||||
return bags.map((bag) => bag.cert!);
|
// node-forge sets bag.cert to null when a bag's content parses as
|
||||||
|
// valid DER but is not a readable X.509 certificate (lib/pkcs12.js
|
||||||
|
// certBag decoder). No entry may be silently dropped (D-04) — check
|
||||||
|
// every bag and reject the whole file, naming it, before returning.
|
||||||
|
const bagCerts = bags.map((bag) => bag.cert);
|
||||||
|
const missingCertIndex = bagCerts.findIndex((c) => c === null);
|
||||||
|
if (missingCertIndex !== -1) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Certificate bag in "${file.originalname as string}" does not contain a readable X.509 certificate`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return bagCerts.filter((c): c is forge.pki.Certificate => c !== null);
|
||||||
} else {
|
} else {
|
||||||
// P7B/PKCS7 — PEM-wrapped or binary DER (Pitfall 4)
|
// P7B/PKCS7 — PEM-wrapped or binary DER (Pitfall 4)
|
||||||
const isPemP7b = (file.buffer as Buffer)
|
const isPemP7b = (file.buffer as Buffer)
|
||||||
@@ -599,7 +620,17 @@ export class CertManagerService {
|
|||||||
if (bags.length === 0) {
|
if (bags.length === 0) {
|
||||||
throw new Error('No certificate bag found in PFX/PKCS12');
|
throw new Error('No certificate bag found in PFX/PKCS12');
|
||||||
}
|
}
|
||||||
cert = bags[0].cert!;
|
// node-forge sets bag.cert to null when the bag's content parses as
|
||||||
|
// valid DER but is not a readable X.509 certificate (lib/pkcs12.js
|
||||||
|
// certBag decoder). An explicit guard here — not an assertion — so
|
||||||
|
// the 400 names the real cause (quick-260921-iwr, D-01/D-04).
|
||||||
|
const parsedCert = bags[0].cert;
|
||||||
|
if (!parsedCert) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Certificate bag in PFX/PKCS12 does not contain a readable X.509 certificate',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
cert = parsedCert;
|
||||||
} else {
|
} else {
|
||||||
// P7B — extract first cert
|
// P7B — extract first cert
|
||||||
const isPemP7b = (file.buffer as Buffer)
|
const isPemP7b = (file.buffer as Buffer)
|
||||||
|
|||||||
Reference in New Issue
Block a user