The in-tree versions of pyasn1 and pyasn1-modules are a year or two old. The newest versions have many improvements.

Comment on attachment 8926632 [details] bug 1413336 - (1/7) update pyasn1 to 0.3.7 https://reviewboard.mozilla.org/r/197858/#review203538

Comment on attachment 8926633 [details] bug 1413336 - (2/7) update pyasn1-modules to 0.1.5 https://reviewboard.mozilla.org/r/197860/#review203540

Comment on attachment 8926634 [details] bug 1413336 - (3/7) fix pycert.py and pykey.py with respect to pyasn1/pyasn1-modules updates https://reviewboard.mozilla.org/r/197862/#review204502 r+ with issues addressed and my question answered. ::: security/manager/ssl/tests/unit/pycert.py:84 (Diff revision 1) > feature value (see rfc7633 for more information). > > If a serial number is not explicitly specified, it is automatically > generated based on the contents of the certificate. > """ > We should probably update security/manager/ssl/tests/unit/requirements.txt as well in this commit or another. ::: security/manager/ssl/tests/unit/pycert.py:222 (Diff revision 1) > > > def getASN1Tag(asn1Type): > """Helper function for returning the base tag value of a given > type from the pyasn1 package""" > - return asn1Type.baseTagSet.getBaseTag().asTuple()[2] > + return asn1Type.tagSet.getBaseTag().tagId Optional: Consider switching from the old `getBaseTag()` to the new `baseTag` property while you're here. ::: security/manager/ssl/tests/unit/pycert.py:516 (Diff revision 1) > # the OID for id_kp_serverAuth is incorrect in the > # pyasn1-modules implementation > return univ.ObjectIdentifier('1.3.6.1.5.5.7.3.1') It looks like pyasn1-modules 0.0.6 fixed the OID, so we can remove this workaround. ::: security/manager/ssl/tests/unit/pykey.py:539 (Diff revision 1) > - algorithmIdentifier.setComponentByName('parameters', univ.Null()) > + nullEncapsulated = univ.Any(encoder.encode(univ.Null())) > + algorithmIdentifier.setComponentByName('parameters', nullEncapsulated) Hmm, I don't quite understand this change. Could you enlighten me?

Comment on attachment 8926635 [details] bug 1413336 - (4/7) make certificate serial number generation not depend on pyasn1 object string representation https://reviewboard.mozilla.org/r/197864/#review204506

Comment on attachment 8926636 [details] bug 1413336 - (5/7) ensure text files generated by pycert et. al. have trailing newlines https://reviewboard.mozilla.org/r/197866/#review204508 Heh, the lack of a trailing newline has always bothered me, but didn't seem worth fixing by itself. Thanks.

Comment on attachment 8926637 [details] bug 1413336 - (7/7) regenerate all the certificates! https://reviewboard.mozilla.org/r/197868/#review204510 rs=me.

Comment on attachment 8926634 [details] bug 1413336 - (3/7) fix pycert.py and pykey.py with respect to pyasn1/pyasn1-modules updates https://reviewboard.mozilla.org/r/197862/#review204584 ::: security/manager/ssl/tests/unit/pycert.py:84 (Diff revision 1) > feature value (see rfc7633 for more information). > > If a serial number is not explicitly specified, it is automatically > generated based on the contents of the certificate. > """ > Good idea. ::: security/manager/ssl/tests/unit/pycert.py:222 (Diff revision 1) > > > def getASN1Tag(asn1Type): > """Helper function for returning the base tag value of a given > type from the pyasn1 package""" > - return asn1Type.baseTagSet.getBaseTag().asTuple()[2] > + return asn1Type.tagSet.getBaseTag().tagId Yeah, good call. ::: security/manager/ssl/tests/unit/pycert.py:516 (Diff revision 1) > # the OID for id_kp_serverAuth is incorrect in the > # pyasn1-modules implementation > return univ.ObjectIdentifier('1.3.6.1.5.5.7.3.1') Good catch. ::: security/manager/ssl/tests/unit/pykey.py:539 (Diff revision 1) > - algorithmIdentifier.setComponentByName('parameters', univ.Null()) > + nullEncapsulated = univ.Any(encoder.encode(univ.Null())) > + algorithmIdentifier.setComponentByName('parameters', nullEncapsulated) There's something wrong with the way univ.Any interacts with univ.Null right now. I think this should be possible without the encapsulation, but at the moment that just creates an AlgorithmIdentifier that's missing the Null tag. This essentially explicitly sets the value of the Any tag to 05 00 (i.e. NULL). (And it's a good thing you mentioned this because I hadn't noticed we've got the same bug in stringToAlgorithmIdentifiers.)

Thanks for the reviews! I decided to fix the not including NULL parameters for RSA algorithms - would you mind having a look at that changeset as well?

Er... actually that changeset is more about using the [] syntax over setComponentByName, but I also did fix the NULL issue.

Comment on attachment 8928323 [details] bug 1413336 - (6/7) replace setComponentByName with direct property setters https://reviewboard.mozilla.org/r/199528/#review205482 Nice!

Comment on attachment 8926634 [details] bug 1413336 - (3/7) fix pycert.py and pykey.py with respect to pyasn1/pyasn1-modules updates https://reviewboard.mozilla.org/r/197862/#review205484 Looks good, but unless I misunderstand, it looks like pycms.py needs updating as well. r=me with that.

(In reply to :Cykesiopka from comment #24) > Comment on attachment 8926634 [details] > bug 1413336 - (3/7) fix pycert.py and pykey.py with respect to > pyasn1/pyasn1-modules updates > > https://reviewboard.mozilla.org/r/197862/#review205484 > > Looks good, but unless I misunderstand, it looks like pycms.py needs > updating as well. r=me with that. Indeed! Good catch. Thanks for the reviews!

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=455e89a11733 Tree's closed right now, but I'll queue this up in autoland.

Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/552ac666904c (1/7) update pyasn1 to 0.3.7 r=ted https://hg.mozilla.org/integration/autoland/rev/79d2717b12ea (2/7) update pyasn1-modules to 0.1.5 r=ted https://hg.mozilla.org/integration/autoland/rev/62a772c93461 (3/7) fix pycert.py and pykey.py with respect to pyasn1/pyasn1-modules updates r=Cykesiopka https://hg.mozilla.org/integration/autoland/rev/37dd644dc984 (4/7) make certificate serial number generation not depend on pyasn1 object string representation r=Cykesiopka https://hg.mozilla.org/integration/autoland/rev/d6c06eed38fe (5/7) ensure text files generated by pycert et. al. have trailing newlines r=Cykesiopka https://hg.mozilla.org/integration/autoland/rev/f9180211e874 (6/7) replace setComponentByName with direct property setters r=Cykesiopka https://hg.mozilla.org/integration/autoland/rev/e972f1bffe69 (7/7) regenerate all the certificates! r=Cykesiopka

https://hg.mozilla.org/mozilla-central/rev/552ac666904c https://hg.mozilla.org/mozilla-central/rev/79d2717b12ea https://hg.mozilla.org/mozilla-central/rev/62a772c93461 https://hg.mozilla.org/mozilla-central/rev/37dd644dc984 https://hg.mozilla.org/mozilla-central/rev/d6c06eed38fe https://hg.mozilla.org/mozilla-central/rev/f9180211e874 https://hg.mozilla.org/mozilla-central/rev/e972f1bffe69

https://hg.mozilla.org/releases/mozilla-release/rev/5264677c6881 https://hg.mozilla.org/releases/mozilla-release/rev/c197126fd051 https://hg.mozilla.org/releases/mozilla-release/rev/7065deadb774 https://hg.mozilla.org/releases/mozilla-release/rev/1f054fd58214 https://hg.mozilla.org/releases/mozilla-release/rev/28fc6ddf2aa2 https://hg.mozilla.org/releases/mozilla-release/rev/0356904016f4 https://hg.mozilla.org/releases/mozilla-release/rev/e14c47f4ceff