174193 - certutil doesn't verify signature on cert request before issuing cert

Status: RESOLVED FIXED

(NSS :: Tools, defect, P1)

Description

[Nelson Bolyard (seldom reads bugmail)]

A certificate request is a signed document.  The signature proves that
the requestor holds the private key that corresponds to the public key
in the cert request.  Any CA program, including certutil, should verify
that signature before honoring it.  

It appears to me that certutil decodes the signed data, but doesn't actually
verify it.  So, any requester can get a cert issued.  
This is very bad if someone is going to use certutil to try to be a real CA.

I think the solution is to do something very similar to CERT_VerifySignedData
except that the public key is taken from the request, not from a certificate.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: new func to check signature on cert request, call from certutil

I created a new function 
SECStatus
CERT_VerifySignedDataWithPubKeyInfo(CERTSignedData *sd,
				    CERTSubjectPublicKeyInfo *pubKeyInfo,
				   void *wincx)

that checks the signature on a certrequest (or any signedData) using 
the public key info taken from a PublicKeyInfo, such as the one found in
the a cert request.  This new function will be exported from NSS.

I added a call to this function in certutil's function that imports
and parses a cert request.  

Please review.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Bob, please review the patch in this bug.  Thanks.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Taking bug, since I have a patch for it already.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Marking assigned.  Since I already have a patch for it.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Patch checked in.  Marking fixed.  

Comment 6

[Robert Relyea]

Comment on attachment 103261 [details] [diff] [review]
new func to check signature on cert request, call from certutil

The patch looks fine, thought I would have preferred the interface to take the
public key and let the application do the SECKEY_ExtractPublicKey().

bob

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Add function CERT_VerifySignedDataWithPublicKey

In light of Bob's comment above, I wrote a new function
CERT_VerifySignedDataWithPublicKey, and changed the existing functions
CERT_VerifySignedDataWithPubKeyInfo and CERT_VerifySignedData to use
this new function, since it contained logic previously common to both.

Bob, please review this patch.