Closed Bug 1483626 Opened Last year Closed 8 months ago

Variable may be blank on the new certificate error strings

Categories

(Firefox :: Security, defect, P2, minor)

61 Branch
defect

Tracking

()

RESOLVED FIXED
Firefox 67
Tracking Status
firefox-esr60 --- unaffected
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- fixed

People

(Reporter: yfdyh000, Assigned: carolina.jimenez.g, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

Attached file siteCert.crt
Steps to reproduce:
1. Visit the https://www.txrjy.com/.
2. Click the Advanced or Continue button.


Actual results:
www.txrjy.com uses an invalid security certificate. The certificate is not trusted because the issuer certificate is unknown. The server might not be sending the appropriate intermediate certificates. An additional root certificate may need to be imported. The certificate is only valid for . Error code: SEC_ERROR_UNKNOWN_ISSUER

(certErrorMismatchSinglePrefix)


Expected results:
...
"The certificate is only valid for www.notexist.com."


Regression range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=52c14d32d9812951c12d92b2594056fc15c5de80&tochange=617a25dfb30dcf0d64014ca92c987aa5bd3f6a4c
Blocks: 1463693
Component: Security: PSM → Security
Product: Core → Firefox
Assignee: nobody → guptatrisha97
Status: NEW → ASSIGNED
Hey, I was working on this and noticed that input.data.certSubjectAltNames and subjectAltNames[0] are blank (shouldn't be), which is why this error is occurring. Does this mean something's wrong with the native code not providing correct data?
Flags: needinfo?(dkeeler)
nsIX509Cert.subjectAltNames will be an empty string if the server's certificate doesn't have the subject alternative name extension present. It also doesn't do much validation of the entries in the certificate's subject alternative name extension if it is present. For example, if there were a dNSName entry that was just spaces or maybe even the empty string, that would show up as spaces or an empty string. I would treat that field as essentially untrusted.

Here's the backing code if it's helpful:

https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/security/manager/ssl/nsNSSCertificate.cpp#646 (this is what runs when the front-end accesses .subjectAltNames)

https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/security/manager/ssl/nsNSSCertificate.cpp#572 (this is called when an nsIX509Cert is initialized)
Flags: needinfo?(dkeeler)
Soo there is a case where the subjectAltName is just blank, that makes sense. Sounds to me like we should have a new string that leaves out the "The certificate is only valid for ..." part, like we had before bug 1415279. That should probably trigger if the alt name is null or an empty string after calling .trim().

Trisha, does that help you?
Flags: needinfo?(guptatrisha97)
Thanks a lot for your information. That really helps!
Flags: needinfo?(guptatrisha97)
Updating tracking flags as we get closer to the 64 release.

Hi Trisha, are you still looking into this bug or can I unassign you? :)

Thanks!

Flags: needinfo?(guptatrisha97)
Duplicate of this bug: 1520973
Assignee: guptatrisha97 → nobody
Blocks: better-cert-errors
No longer blocks: 1463693
Status: ASSIGNED → NEW
Flags: needinfo?(guptatrisha97)
Assignee: nobody → carolina.jimenez.g
Mentor: jhofmann
Status: NEW → ASSIGNED

This is the output of doing a try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=a2d9beaa7129ec770a96138295deeb671033cdec

Flags: needinfo?(jhofmann)

Hey, that's probably due to removing the mochitest cert. See Phabricator :)

Flags: needinfo?(jhofmann)

This is the output again... https://treeherder.mozilla.org/#/jobs?repo=try&revision=309cd8d487dabc2c0ee46531d234f1ce8de69a25

Flags: needinfo?(jhofmann)

That looks good to me! You will notice that some tests failed (the orange fields), but all of them seem to be intermittent (they have existing bugs filed for them and don't seem to be related to our test).

You should update the patch on phabricator and set checkin-needed.

Flags: needinfo?(jhofmann)
Keywords: checkin-needed

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

Flags: needinfo?(carolina.jimenez.g)
Keywords: checkin-needed

You can do that with hg pull central --rebase :)

Flags: needinfo?(carolina.jimenez.g)
Keywords: checkin-needed

Pushed by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a09b3ceb8f72
Checks if subjectAltNames has elements that are not empty string, and if it has them, they will be remove, preventing incomplete r=johannh

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
You need to log in before you can comment on or make changes to this bug.