If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Create utility for converting OIDs to C++ source code

RESOLVED FIXED in mozilla32

Status

()

Core
Security: PSM
--
enhancement
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: briansmith, Assigned: briansmith)

Tracking

Trunk
mozilla32
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Created attachment 8432648 [details] [diff] [review]
Add DottedOIDToCode.py

Example:

$ python DottedOIDToCode.py ecdsa-with-SHA512 1.2.840.10045.4.3.4
  // python DottedOIDToCode.py ecdsa-with-SHA512 1.2.840.10045.4.3.4
  static const uint8_t ecdsa_with_SHA512[] = {
    0x2a, 0x86,0x48, 0xce,0x3d, 0x04, 0x03, 0x04
  };

Note that there are other possible output formats, such as outputing "40*x+y" for the first two nodes and/or outputing "128+x" or "0x80|x" for the leading bytes of other nodes. I chose the pure-hex style because it should be easier to compare the values with other tools like asn1.js and various cert veiwers which output hex.
Attachment #8432648 - Flags: review?(dkeeler)
Comment on attachment 8432648 [details] [diff] [review]
Add DottedOIDToCode.py

Review of attachment 8432648 [details] [diff] [review]:
-----------------------------------------------------------------

Cool! Just a couple of nits.

::: security/pkix/tools/DottedOIDToCode.py
@@ +21,5 @@
> +
> +from __future__ import print_function
> +import sys
> +
> +def base128Stringified(value):

nit: add a Docstring for each function?

@@ +22,5 @@
> +from __future__ import print_function
> +import sys
> +
> +def base128Stringified(value):
> +  if value < 0:

nit: I think the style guidelines for python say to use 4 spaces :/
(I actually prefer two spaces, though, so I think we're fine as long as we're consistent)

@@ +33,5 @@
> +  value /= 0x80
> +
> +  while value != 0:
> +    # other bytes have highest bit set
> +    result = (format % (0x80 | (value % 0x80))) + "," + result

nit: I'm on the fence about not including a space after the comma here (I realize it groups the hex value representing one OID part, but since you're going for a more pure-hex style, I don't think it adds much and doesn't look well-formatted).

@@ +34,5 @@
> +
> +  while value != 0:
> +    # other bytes have highest bit set
> +    result = (format % (0x80 | (value % 0x80))) + "," + result
> +    value /= 128

nit: let's be consistent about 0x80 vs 128 (looks like 0x80 is more common)

@@ +62,5 @@
> +                       dottedOIDToCEncoding(dottedOID))
> +
> +if __name__ == "__main__":
> +  if len(sys.argv) != 3:
> +    print("usage:   python %s <name> <dotted-oid>" % sys.argv[0],

nit: three spaces after "usage:"?
Attachment #8432648 - Flags: review?(dkeeler) → review+
Created attachment 8432895 [details] [diff] [review]
add-DottedOIDToCode.patch [v2]

I added some comments. I tried to make them follow the basics of the docstring conventions, but I didn't try to be perfect there.

The three spaces after "usage:" are so that the "usage:" and "example:" lines are aligned.
Attachment #8432648 - Attachment is obsolete: true
Attachment #8432895 - Flags: review?(dkeeler)
Comment on attachment 8432895 [details] [diff] [review]
add-DottedOIDToCode.patch [v2]

Review of attachment 8432895 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Just some nits.

::: security/pkix/tools/DottedOIDToCode.py
@@ +35,5 @@
> +    '0xce, 0x3d'
> +    """
> +    if value < 0:
> +        raise ValueError("An OID must have only positive-value nodes.")
> +    

nit: trailing whitespace

@@ +37,5 @@
> +    if value < 0:
> +        raise ValueError("An OID must have only positive-value nodes.")
> +    
> +    format = "0x%.2x"
> +    

ws

@@ +41,5 @@
> +    
> +    # least significant byte has highest bit unset
> +    result = format % (value % 0x80)
> +    value /= 0x80
> +    

ws

@@ +64,5 @@
> +        raise ValueError("The first node of an OID must be 0, 1, or 2.")
> +    if not (0 <= nodes[1] <= 39):
> +        # XXX: Does this restriction apply when the first part is 2?
> +        raise ValueError("The second node of an OID must be 0-39.")
> +    firstByte = (40*nodes[0]) + nodes[1]

nit: spaces around operators: 40 * nodes[0]

@@ +85,5 @@
> +    Given an ASN.1 name and a string containing the dotted representation of an
> +    OID, returns a string that contains a C++ declaration for a named constant
> +    that contains that OID value. Note that the ASN.1 tag and length are *not*
> +    included in the result.
> +    

ws

@@ +93,5 @@
> +      static const uint8_t ecdsa_with_SHA512[] = {
> +        0x2a, 0x86,0x48, 0xce,0x3d, 0x04, 0x03, 0x04
> +      };
> +    """
> +    return ("  // python DottedOIDToCode.py %s %s\n" +

hmmm - if we rename this file, "DottedOIDToCode.py" will be out of date. Do we care? We could pass in sys.argv[0] here.
Attachment #8432895 - Flags: review?(dkeeler) → review+
(In reply to David Keeler (:keeler) [use needinfo?] from comment #3)
> nit: trailing whitespace

I fixed all of these. (It turns out that this file also had Windows-style line endings, which caused my checks for trailing whitespace to fail. I fixed the Windows-style newlines too.)

> nit: spaces around operators: 40 * nodes[0]

Done.

> hmmm - if we rename this file, "DottedOIDToCode.py" will be out of date. Do
> we care? We could pass in sys.argv[0] here.

I switched this to use sys.argv[0].

Thanks for the review!

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b366010fbbc
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/7b366010fbbc
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.