284191 - pp incorrectly prints cert request attributes

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

When the pp program attempts to print a PKCS10 certificate request that
contains "attributes", it does not print them correctly.  The problem is
that SECU_PrintCertificateRequest attempts to print the attributes as if
they were decoded as a single "SEC_ASN1_ANY", which they are not.

A related problem is an error in the declaration of struct
CERTCertificateRequestStr.   The last member of that struct, declared as

     SECItem **attributes;

doesn't match the template.  It should be a CERTAttribute **, which is 
quite different from a SECItem **.  

I plan to attach to this bug a cert request that exhibits the problem,
(which Neil created today) and a patch that fixes pp to print it.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: PKCS10 cert request with attribute

This cert request demonstrates the problem.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

We currently believe that ability for certutil/pp to create and display 
PKCS10 cert requests with multiple subjectAltNames is vital for servers.
However, if we find that another means is preferered to get SSL server
certs with multiple host names, this bug may drop to P3 or below.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1 - see comment for prerequisites

This patch has two prerequisite patches:
1. The new OID tag SEC_OID_PKCS9_EXTENSION_REQUEST, which is added by 
   a patch for bug 263779 (recently checked in on trunk).
2. CERT_SequenceOfCertExtensionTemplate must be exported.  This is done by
   a patch for bug 284200 (not yet checked in). 
Neil and Wan-Teh, please review.

Comment 4

[Neil Williams]

Comment on attachment 176002 [details] [diff] [review]
patch v1 - see comment for prerequisites

The only quibble I see is that SECU_PrintSetOfAny() sets rv and returns it but
never assigns anything to it.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 176002 [details] [diff] [review]
patch v1 - see comment for prerequisites

secutil.c  new revision: 1.67; previous revision: 1.66
secutil.h  new revision: 1.17; previous revision: 1.16
Thanks for the review.	Checked in for Beta 1 build.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

marking fixed