Closed Bug 713934 Opened 8 years ago Closed 8 years ago

Update PSM to use SSL_AuthCertificateComplete instead of SSL_RestartHandshakeAfterAuthCertificate

Categories

(Core :: Security: PSM, defect, P1)

Other Branch
defect

Tracking

()

RESOLVED FIXED
mozilla13
Tracking Status
firefox9 --- unaffected
firefox10 --- unaffected
firefox11 + fixed
firefox12 + fixed

People

(Reporter: briansmith, Assigned: briansmith)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

In the latest patches to bug 542832, I changed the signature of SSL_RestartHandshakeAfterAuthCertificate and renamed it to SSL_AuthCertificateComplete. Unless the NSS team disagrees with implementing those patches, SSL_AuthCertificateComplete will replace SSL_RestartHandshakeAfterAuthCertificate, so Gecko must switch to using SSL_AuthCertificateComplete.

Note that currently, we have a private-to-mozilla-central/-aurora patch that implements SSL_RestartHandshakeAfterAuthCertificate. We need to do this to update to the NSS 3.12.2 release, which we *must* do for mozilla-aurora as well as mozilla-central.
Comment on attachment 584869 [details] [diff] [review]
Part 2, version 1: Update SetCertVerificationResult to use SSL_AuthCertificateComplete

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

::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ +982,5 @@
> +          if (errorCode == 0) {
> +            NS_ERROR("SSL_AuthCertificateComplete didn't set error code");
> +            errorCode = PR_INVALID_STATE_ERROR;
> +          }
> +          SetCanceled(errorCode, PlainErrorMessage);

Wrong indention (and if the code where right, see bellow, blank line before SetCanceled when after closing brace, please).

And SetCanceled should be called even errorCode was not 0, right?
Attachment #584869 - Flags: review?(honzab.moz)
Comment on attachment 584868 [details] [diff] [review]
Part 1, version 1: Add new patches to NSS

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

r=honzab of the mozilla-central changes.  Landing this patch is conditioned by reviews of related patches in bug 542832:

ssl-restart-3-alerts-v1.patch = "Rename SSL_RHAAC to SSL_AuthCertificateComplete; send alert when async cert. authentication fails"
ssl-restart-4-tstclnt-alerts-v2.patch = "Update tstclnt to use the new function name and to send alerts when cert authentication fails [v2]"
ssl-restart-5-disable-false-start-v1.patch = "Disable False Start when async cert authentication is used until they work safely together"
Attachment #584868 - Flags: review?(honzab.moz) → review+
Comment on attachment 590543 [details] [diff] [review]
Part 2: Update SetCertVerificationResult to use SSL_AuthCertificateComplete [v2]

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

r=honzab
Attachment #590543 - Flags: review?(honzab.moz) → review+
https://hg.mozilla.org/mozilla-central/rev/8365c736e4f3
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: mozilla11 → mozilla13
Comment on attachment 590543 [details] [diff] [review]
Part 2: Update SetCertVerificationResult to use SSL_AuthCertificateComplete [v2]

[Approval Request Comment]
Regression caused by (bug #): bug 713936
User impact if declined: Firefox stops working on Linux distributions that use system NSS 3.13.2 or later
Testing completed (on m-c, etc.): Landed on mozilla-central yesterday
Risk to taking this patch (and alternatives if risky): Low. This is not a change in logic, it is just rearranging the existing logic to accommodate the change to the NSS function that changed signatures between NSS 3.13.2 beta1 and NSS 3.13.2 RTM.
String changes made by this patch: None
Attachment #590543 - Flags: approval-mozilla-beta?
Attachment #590543 - Flags: approval-mozilla-aurora?
Note that this must land at the same time as bug 713936, because bug 713936 will change the NSS API in a way that breaks Firefox without this patch.
Comment on attachment 590543 [details] [diff] [review]
Part 2: Update SetCertVerificationResult to use SSL_AuthCertificateComplete [v2]

[Triage Comment]
Approved for Aurora 12 and Beta 11 given the need to switch to the final NSS version.
Attachment #590543 - Flags: approval-mozilla-beta?
Attachment #590543 - Flags: approval-mozilla-beta+
Attachment #590543 - Flags: approval-mozilla-aurora?
Attachment #590543 - Flags: approval-mozilla-aurora+
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.