287285 - certutil can issue cert with zero-length EKU OID

Status: NEW

(NSS :: Tools, defect, P3)

Description

[Alexei Volkov]

The problem with code is that in case when user choose to exit
the loop right away by picking number that is out of range, then
we will and up with the cert that has SEC_OID_UNKNOWN usage.

Here is discussion log:

From Julien  Pierre:
>AddExtKeyUsage reads user value and created an extension based on added OID value.
>In case, then user pick something above 6, it will create an extension
>with oid SEC_OID_UNKNOWN value.
>
>There are couple places like this. Is it correct thing to add such
>extension to cert(especially if user will pick it as critical) or
>is it something that should be fixed?
>
>
It looks to me like something that should be fixed. rv should probably
be initialized, and the default case should goto loser instead of endloop .
 
I'm cc'ing Neil and Nelson on this. I know there were some fixes going
on in certutil last week that might be related.


From Nelson B Bolyard:
Neil Williams wrote:
>I didn't check all the places there might be code like this but in
>AddExtKeyUsage() it's building a sequence of usages. It loops collecting
>the items of the sequence. It assumes that anything out of range signals
>end of list so it jumps to endloop and creates the extension from its
>sequence of OIDs. So the default case means "end-of-list" not
>"out-of-range". If the default case jumped to loser instead of endloop
>IMO it would never generate an ExtKeyUsage extension. I ran it as is,
>entered 8 at the first prompt and got what looked like a good cert with
>an extension that had a zero length sequence in it. Have to check x.509
>to see if that is legal.

Let's assume that it is not, that EKU extensions must have at least one
valid OID in them.  In that case, sounds like you've found a bug in
certutil's extension generation, and it should be fixed.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

This is a tools bug.  The original summary noted this problem is in 
function AddExtKeyUsage in certutil.c

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.