391434 - avoid multiple encoding/decoding of PKIX_PL_OID to and from ascii string

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[Alexei Volkov]

libpkix keeps oids as an array of unsigned integers. But this is just a part of the problem. The rest of the problem is that there are multiple places in the code where pkix oid structure is created from ascii string that represents an OID.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Alexei, libpkix keeps OIDs in lots of different forms in different places.
So, WHERE does it keep OIDs as an array of unsigned integers?

Comment 2

[Alexei Volkov]

struct PKIX_PL_OIDStruct {
        PKIX_UInt32 *components;
        PKIX_UInt32 length;
};

"components" is a pointer the array of PKIX_UInt32 with length "length".

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Ah, yes, that's YET ANOTHER form in which libPKIX stores OIDs.
It's an array of integers, so that an oid like 2.6.13.313976.1.2
becomes an array like
   2
   6
   13
   313976
   1
   2
with a length of 7.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Unsetting target milestone in unresolved bugs whose targets have passed.

Comment 5

[Alexei Volkov]

Attachment: Patch v1 - first draft of the req changes

Comment 6

[Alexei Volkov]

Comment on attachment 372344 [details] [diff] [review]
Patch v1 - first draft of the req changes

Found miner things that need to be fixed in the next patch.

Comment 7

[Alexei Volkov]

Attachment: Patch v2 - review changes

Comment 8

[Alexei Volkov]

Comment on attachment 372364 [details] [diff] [review]
Patch v2 - review changes 

Made a mistake in the patch. Removing the request.

Comment 9

[Alexei Volkov]

Attachment: Patch v3 - address review comments

Comment 10

[Alexei Volkov]

Comment on attachment 372429 [details] [diff] [review]
Patch v3 - address review comments

The patch has extra file in the diff. Impossible to do diff of the diffs. :( Will reattach again.

Comment 11

[Alexei Volkov]

Attachment: Patch v4 - patches files included in patch v1(committed)

Comment 12

[Alexei Volkov]

Attachment: Patch v4 - changes to new files(committed)

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 372431 [details] [diff] [review]
Patch v4 - patches files included in patch v1(committed)

r=nelson,
I suggest one minor string change

>+    OD( x509CertificatePoliciesAnyPolicy, SEC_OID_X509_ANY_POLICY,
>+ 	"Certificate Policies AnyPolicy",

I suggest"
 	"Certificate Policy AnyPolicy",
or perhaps
 	"Certificate Policy \"AnyPolicy\"",

>+        CKM_INVALID_MECHANISM, UNSUPPORTED_CERT_EXTENSION ),
> };

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 372432 [details] [diff] [review]
Patch v4 - changes to new files(committed)

r=nelson, after one required string change:


>+ER3(SEC_ERROR_BAD_INFO_ACCESS_METHOD,      		(SEC_ERROR_BASE + 170),
>+"Unknown info access method.")

Change that to

  "Unknown information access method in certificate extension."