Skip to content

Commit ffd0d4f

Browse files
committed
fix: return clear error for unsupported ECDSA/EC provisioning certificates
* Detect when node-forge cannot parse EC certificates from a PFX and return a descriptive error instead of a generic decrypt failure. * Add null guards for empty cert arrays and missing CN fields in domainSuffixChecker. * Route cert errors through a dedicated middleware to bypass express-validator field-level formatting. Addresses #2505
1 parent 7b84bd7 commit ffd0d4f

File tree

6 files changed

+112
-8
lines changed

6 files changed

+112
-8
lines changed

src/certManager.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
**********************************************************************/
55

6-
import { CertManager } from './certManager.js'
6+
import { CertManager, UnsupportedCertificateError } from './certManager.js'
77
import { NodeForge } from './NodeForge.js'
88
import { type AMTKeyUsage, type CertAttributes, type CertificateObject } from './models/index.js'
99
import Logger from './Logger.js'
@@ -63,6 +63,29 @@ describe('certManager tests', () => {
6363
expect(pfxobj.certs).toHaveLength(2)
6464
expect(pfxobj.keys).toHaveLength(1)
6565
})
66+
67+
test('throws UnsupportedCertificateError when cert bags contain no parseable certs (e.g. ECDSA)', () => {
68+
const nodeForge = new NodeForge()
69+
const certManager = new CertManager(new Logger('CertManager'), nodeForge)
70+
const certBagOid = nodeForge.pkiOidsCertBag
71+
const keyBagOid = nodeForge.pkcs8ShroudedKeyBag
72+
73+
spyOn(nodeForge, 'pkcs12FromAsn1').mockReturnValue({
74+
getBags: (opts: any) => {
75+
if (opts.bagType === certBagOid) {
76+
return { [certBagOid]: [{ cert: null }, { cert: null }] }
77+
}
78+
79+
return { [keyBagOid]: [] }
80+
}
81+
} as any)
82+
83+
expect(() => certManager.convertPfxToObject(TEST_PFXCERT, 'P@ssw0rd')).toThrow(UnsupportedCertificateError)
84+
85+
expect(() => certManager.convertPfxToObject(TEST_PFXCERT, 'P@ssw0rd')).toThrow(
86+
/ECDSA\/EC certificates are not supported/
87+
)
88+
})
6689
})
6790

6891
describe('dumpPfx tests', () => {

src/certManager.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ interface Attribute {
2323
value: string
2424
}
2525

26+
export class UnsupportedCertificateError extends Error {
27+
public readonly code = 'UNSUPPORTED_CERTIFICATE'
28+
29+
constructor(message: string) {
30+
super(message)
31+
this.name = 'UnsupportedCertificateError'
32+
}
33+
}
34+
2635
export class CertManager {
2736
private readonly nodeForge: NodeForge
2837
private readonly logger: ILogger
@@ -165,6 +174,12 @@ export class CertManager {
165174
throw new Error('No certificate bags found')
166175
}
167176

177+
if (pfxOut.certs.length === 0) {
178+
throw new UnsupportedCertificateError(
179+
'No certificates could be parsed from the provisioning certificate. ECDSA/EC certificates are not supported. Please use an RSA certificate. To generate an RSA key: openssl genrsa -out key.pem 2048'
180+
)
181+
}
182+
168183
// Process key bags
169184
const keyBags = pfx.getBags({ bagType: this.nodeForge.pkcs8ShroudedKeyBag })
170185
const keyBagArray = keyBags[this.nodeForge.pkcs8ShroudedKeyBag]

src/custom.d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55

66
import { type IDB } from './interfaces/database/IDb.js'
77
import { type ISecretManagerService } from './interfaces/ISecretManagerService.js'
8+
import { type CertsAndKeys } from './models/index.js'
89

910
declare module 'express' {
1011
export interface Request {
1112
secretsManager: ISecretManagerService
1213
tenantId?: string
1314
db: IDB
15+
certError?: string
16+
pfxobj?: CertsAndKeys | null
1417
}
1518
}

src/routes/admin/domains/domain.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,23 @@ describe('Domain Profile Validation', () => {
184184
}).toThrowError(new Error('FQDN not associated with provisioning certificate'))
185185
})
186186

187+
test('domainSuffixChecker - empty certs array', () => {
188+
const emptyCertsPfx = { certs: [], keys: [] }
189+
expect(() => {
190+
domainSuffixChecker(emptyCertsPfx as any, 'vprodemo.com')
191+
}).toThrowError(new Error('No certificates found in the provisioning certificate'))
192+
})
193+
194+
test('domainSuffixChecker - cert without CN', () => {
195+
const noCnPfx = {
196+
certs: [{ subject: { getField: () => null } }],
197+
keys: []
198+
}
199+
expect(() => {
200+
domainSuffixChecker(noCnPfx as any, 'vprodemo.com')
201+
}).toThrowError(new Error('Provisioning certificate does not contain a Common Name (CN) in the subject'))
202+
})
203+
187204
test('expirationChecker - success', () => {
188205
expect(() => {
189206
expirationChecker(pfxobj)

src/routes/admin/domains/domain.ts

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,15 @@
33
* SPDX-License-Identifier: Apache-2.0
44
**********************************************************************/
55

6-
import { check, type CustomValidator } from 'express-validator'
6+
import { check, validationResult, type CustomValidator } from 'express-validator'
7+
import { type Request, type Response, type NextFunction } from 'express'
78
import { NodeForge } from '../../../NodeForge.js'
8-
import { CertManager } from '../../../certManager.js'
9+
import { CertManager, UnsupportedCertificateError } from '../../../certManager.js'
910
import Logger from '../../../Logger.js'
1011
import { type CertsAndKeys } from '../../../models/index.js'
1112

1213
const nodeForge = new NodeForge()
1314
const certManager = new CertManager(new Logger('CertManager'), nodeForge)
14-
let pfxobj
1515

1616
export const domainInsertValidator = (): any => [
1717
check('profileName')
@@ -60,13 +60,43 @@ export const domainUpdateValidator = (): any => [
6060

6161
function passwordValidator(): CustomValidator {
6262
return (value, { req }) => {
63-
pfxobj = passwordChecker(certManager, req)
63+
try {
64+
req.pfxobj = passwordChecker(certManager, req)
65+
} catch (error) {
66+
req.pfxobj = null
67+
if (error instanceof UnsupportedCertificateError) {
68+
req.certError = error.message
69+
return true
70+
}
71+
throw error
72+
}
6473
return true
6574
}
6675
}
6776

77+
export function certErrorMiddleware(req: Request, res: Response, next: NextFunction): void {
78+
const errors = validationResult(req)
79+
80+
// If there are other validation errors, let the normal validation handling run.
81+
if (!errors.isEmpty()) {
82+
next()
83+
84+
return
85+
}
86+
87+
if (req.certError) {
88+
res.status(400).json({ message: req.certError }).end()
89+
90+
return
91+
}
92+
93+
next()
94+
}
95+
6896
function domainSuffixValidator(): CustomValidator {
6997
return (value, { req }) => {
98+
const pfxobj = req.pfxobj
99+
70100
if (pfxobj != null) {
71101
domainSuffixChecker(pfxobj, value)
72102
}
@@ -76,6 +106,8 @@ function domainSuffixValidator(): CustomValidator {
76106

77107
function expirationValidator(): CustomValidator {
78108
return (value, { req }) => {
109+
const pfxobj = req.pfxobj
110+
79111
if (pfxobj != null) {
80112
expirationChecker(pfxobj)
81113
}
@@ -88,6 +120,10 @@ export function passwordChecker(certManager: CertManager, req: any): CertsAndKey
88120
const pfxobj = certManager.convertPfxToObject(req.body.provisioningCert, req.body.provisioningCertPassword)
89121
return pfxobj
90122
} catch (error) {
123+
if (error instanceof UnsupportedCertificateError) {
124+
throw error
125+
}
126+
91127
throw new Error(
92128
'Unable to decrypt provisioning certificate. Please check that the password is correct, and that the certificate is a valid certificate.',
93129
{ cause: error }
@@ -96,7 +132,17 @@ export function passwordChecker(certManager: CertManager, req: any): CertsAndKey
96132
}
97133

98134
export function domainSuffixChecker(pfxobj: CertsAndKeys, value: any): void {
99-
const certCommonName = pfxobj.certs[0].subject.getField('CN').value
135+
if (!pfxobj.certs || pfxobj.certs.length === 0) {
136+
throw new Error('No certificates found in the provisioning certificate')
137+
}
138+
139+
const cnField = pfxobj.certs[0].subject.getField('CN')
140+
141+
if (!cnField) {
142+
throw new Error('Provisioning certificate does not contain a Common Name (CN) in the subject')
143+
}
144+
145+
const certCommonName = cnField.value
100146
const splittedCertCommonName: string[] = certCommonName.split('.')
101147
const parsedCertCommonName = (
102148
splittedCertCommonName[splittedCertCommonName.length - 2] +

src/routes/admin/domains/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@ import { getDomain } from './get.js'
99
import { DomainCreate } from './create.js'
1010
import { deleteDomain } from './delete.js'
1111
import { editDomain } from './edit.js'
12-
import { domainInsertValidator, domainUpdateValidator } from './domain.js'
12+
import { domainInsertValidator, domainUpdateValidator, certErrorMiddleware } from './domain.js'
1313
import { odataValidator } from '../odataValidator.js'
1414
import validateMiddleware from '../../../middleware/validate.js'
1515
const domainRouter: Router = Router()
1616
const dc = new DomainCreate()
1717
domainRouter.get('/', odataValidator(), validateMiddleware, getAllDomains)
1818
domainRouter.get('/:domainName', getDomain)
19-
domainRouter.post('/', domainInsertValidator(), validateMiddleware, dc.createDomain)
19+
domainRouter.post('/', domainInsertValidator(), certErrorMiddleware, validateMiddleware, dc.createDomain)
2020
domainRouter.patch('/', domainUpdateValidator(), validateMiddleware, editDomain)
2121
domainRouter.delete('/:domainName', deleteDomain)
2222

0 commit comments

Comments
 (0)