#12 org.mozilla.jss.pkix.primitive.AlgorithmIdentifier decode/encode process alters original data
Closed: fixed 3 years ago Opened 3 years ago by cfu.

This is a clone of the Mozilla bug https://bugzilla.mozilla.org/show_bug.cgi?id=830781
(Note: Mozilla jss bug repository is now defunct)

Description is copied as is (the rest of the info on that bug can be found on the Mozilla bug)

====

David Stutzman (Reporter)
Description • 6 years ago

If you have an AlgorithmIdentifier in encoded form such as:
SEQUENCE {
OBJECT IDENTIFIER '1.2.3.4'
}

decode it into an org.mozilla.jss.pkix.primitive.AlgorithmIdentifier object then encode that back into bytes you will end up with:
SEQUENCE {
OBJECT IDENTIFIER '1.2.3.4'
NULL
}

This is due to the decoding template in AlgorithmIdentifier class which takes the optional second element of the decoded sequence (which would be a normal Java null when presented with encoded form of my first example) and calls the 2 parameter AlgorithmIdentifier constructor (http://mxr.mozilla.org/security/source/security/jss/org/mozilla/jss/pkix/primitive/AlgorithmIdentifier.java#103). Doing this will turn the java null into a JSS PKIX NULL object (http://mxr.mozilla.org/security/source/security/jss/org/mozilla/jss/pkix/primitive/AlgorithmIdentifier.java#43) and add it to the sequence to be encoded should that method then be called.

Apparently this code has been like this since first checked in many years ago and as such would affect all versions of Mozilla's JSS. If you are working with DER encoded data such as this you end up altering it slightly if you decode/re-encode pieces of it and could end up invalidating signatures on signed data since it has been modified.

I'm not on development machine to generate a valid patch but a fix would be as simple as checking the actual value of seq.elementAt(1) in the decode method (http://mxr.mozilla.org/security/source/security/jss/org/mozilla/jss/pkix/primitive/AlgorithmIdentifier.java#105) and if the value is a Java null, meaning nothing at all was in the encoded form in the first place, then call the 1 arg constructor, else call the 2 arg constructor.

A simple test case is to construct an AlgorithmIdentifier with the 1 arg constructor and encode it to a byte[]. Decode the byte[] back into AlgorithmIdentifier object and encode to a second byte[]. Compare the resulting byte[] to the original. You will see the second byte array has an additional ASN.1 encoded NULL (hex bytes: 05 00) in it.


Metadata Update from @cfu:
- Custom field cc adjusted to david.k.stutzman2.ctr@mail.mil
- Custom field component adjusted to None
- Custom field feature adjusted to None
- Custom field origin adjusted to Community
- Custom field proposedmilestone adjusted to None
- Custom field proposedpriority adjusted to None
- Custom field reviewer adjusted to None
- Custom field rhbz adjusted to https://bugzilla.redhat.com/show_bug.cgi?id=1534772
- Custom field type adjusted to None
- Custom field version adjusted to None

3 years ago

Metadata Update from @cfu:
- Issue assigned to cfu

3 years ago

commit 8fc2c37b3a7af8df7da0365c23f0553febb36905 (HEAD -> master, Ticket-12-AlgorithmIdEncodeDecode)
Author: Christina Fu cfu@redhat.com
Date: Tue Jun 26 17:59:28 2018 -0700

Ticket 12 AlgorithmIdentifier decode/encode process alters original data

This patch provides fix to ensure that the encoding and decoding of an AlgorithmIdentifier
structure would not alter the data.

credit: original fix suggestion provided by david.k.stutzman2.ctr@mail.mil

fixes https://pagure.io/jss/issue/12

Metadata Update from @cfu:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

3 years ago

Login to comment on this ticket.

Metadata