Closed
Bug 1264761
Opened 8 years ago
Closed 8 years ago
Certificates ASN.1 version number set to v4 stops certificate viewer from showing PKI chain
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: geekmasher, Assigned: keeler)
Details
(Whiteboard: [psm-assigned])
Attachments
(2 files)
Hey Guys, I created a test case where I set the ASN.1 version number variable of a X509v3 leaf certificate to version 4. The certificate was accepted (signed by my CA), which doesn't cause major issues, but when I tried to view the certificate chain in the viewer, it was broken and doesn't display anything at all. Files Attachments: Certificates: - certs/cert.pub.pem - certs/cert.key.pem - certs/ca.pem Screenshots: - screenshots/viewer.png - screenshots/general.png I'm not 100% sure if both this bug should be in "Security" and if this could lead to any major vulnerabilities but I will still submit it as hidden. I've been developing a tool called SquarePeg (https://github.com/cigital/SquarePeg), which is still not published at the time I submit this bug report I built is to test for TLS/SSL vulnerabilities in applications.
Updated•8 years ago
|
Group: core-security → crypto-core-security
Assignee | ||
Comment 1•8 years ago
|
||
Thanks for filing this bug. It's more or less a display issue (the backend code is handling the unexpected value by returning an error and the frontend essentially bails as a result).
Assignee: nobody → dkeeler
Group: crypto-core-security
Status: UNCONFIRMED → ASSIGNED
Component: Security → Security: PSM
Ever confirmed: true
Whiteboard: [psm-assigned]
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/47223/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/47223/
Attachment #8742481 -
Flags: review?(cykesiopka.bmo)
Reporter | ||
Comment 3•8 years ago
|
||
Excellent. Glad I can help. GeekMasher.
Comment 4•8 years ago
|
||
I skimmed over the changes for now, and in general they look good. I just wanted to confirm something before looking closer... It looks like since Bug 1047177, mozilla::pkix treats v4 certs as v3 internally. Do we want to surface this fact in the UI?
Flags: needinfo?(dkeeler)
Reporter | ||
Comment 5•8 years ago
|
||
Either, Firefox should treat the v4 certificate like any other unknown versions (negatives and 5+) with a DER encoding error or it should display in the UI the fact that it is a v4 certificate. I don't know if or when there will be a X509v4 certificate but support for it should be provided. Whether now or when the standard is published.
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to :Cykesiopka from comment #4) > It looks like since Bug 1047177, mozilla::pkix treats v4 certs as v3 > internally. > Do we want to surface this fact in the UI? In the future I'm imagining some kind of built-in linter that would flag this with a warning like "you probably meant x509v3, in which case you should set the value to 2 here", but I think that's out of scope for this bug. My goal here was to just show whatever was in the certificate, even if it claimed to be an x509 version that doesn't exist yet.
Flags: needinfo?(dkeeler)
Comment 7•8 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #6) > In the future I'm imagining some kind of built-in linter that would flag > this with a warning like "you probably meant x509v3, in which case you > should set the value to 2 here", but I think that's out of scope for this > bug. My goal here was to just show whatever was in the certificate, even if > it claimed to be an x509 version that doesn't exist yet. OK, thanks for clarifying.
Comment 8•8 years ago
|
||
Comment on attachment 8742481 [details] MozReview Request: bug 1264761 - improve handling of x509 versions in certificate manager r?Cykesiopka https://reviewboard.mozilla.org/r/47223/#review44235 Looks good. ::: security/manager/locales/en-US/chrome/pipnss/pipnss.properties:56 (Diff revision 1) > # LOCALIZATION NOTE (nick_template): $1s is the common name from a cert (e.g. "Mozilla"), $2s is the CA name (e.g. VeriSign) > nick_template=%1$s's %2$s ID > #These are the strings set for the ASN1 objects in a certificate. > CertDumpCertificate=Certificate > CertDumpVersion=Version > -CertDumpVersion1=Version 1 > +CertDumpVersionValue=Version %S Nit: An l10n note to describe the %S part would probably help here. ::: security/manager/ssl/nsNSSCertHelper.cpp:105 (Diff revision 1) > // If there is no version present in the cert, then rfc2459 > // says we default to v1 (0) Optional: Update this to say RFC5280. ::: security/manager/ssl/tests/unit/test_cert_version.js:58 (Diff revision 1) > + Assert.equal(version.displayValue, expectedVersionString, > + "Actual and expected version strings should match"); Nit: Aline the message, and maybe drop the |Assert.| part.
Attachment #8742481 -
Flags: review?(cykesiopka.bmo) → review+
Assignee | ||
Comment 9•8 years ago
|
||
https://reviewboard.mozilla.org/r/47223/#review44235 Thanks! > Nit: An l10n note to describe the %S part would probably help here. Good idea. > Optional: Update this to say RFC5280. Sounds good. > Nit: Aline the message, and maybe drop the |Assert.| part. Fixed.
Assignee | ||
Comment 10•8 years ago
|
||
Comment on attachment 8742481 [details] MozReview Request: bug 1264761 - improve handling of x509 versions in certificate manager r?Cykesiopka Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47223/diff/1-2/
I had to back this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5c27d9625278 for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=26158852&repo=mozilla-inbound
Flags: needinfo?(dkeeler)
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8742481 [details] MozReview Request: bug 1264761 - improve handling of x509 versions in certificate manager r?Cykesiopka Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47223/diff/2-3/
Assignee | ||
Comment 14•8 years ago
|
||
Turned out to be a 32-bit/64-bit issue, basically.
Flags: needinfo?(dkeeler)
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f7caa27d445d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•