Assertion failure: ss->ssl3.hs.hashType == handshake_hash_unknown

RESOLVED FIXED in Firefox 27

Status

()

defect
P1
blocker
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: cbook, Assigned: briansmith)

Tracking

(Blocks 1 bug, {assertion, regression, sec-critical})

Trunk
mozilla28
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox25 wontfix, firefox26 wontfix, firefox27+ fixed, firefox28+ fixed, firefox-esr17 wontfix, firefox-esr2428+ fixed, b2g18? fixed, b2g-v1.1hd fixed, b2g-v1.2 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 unaffected)

Details

(Whiteboard: [adv-main27+][adv-esr24.4+][qa-], )

Attachments

(3 attachments, 1 obsolete attachment)

Loading https://avukat.uyap.gov.tr/portal_baslangic.uyap with a debug build tested on win7 crashes on load with Assertion failure: ss->ssl3.hs.hashType == handshake_hash_unknown 

will try to get a testcase etc for this - also according to bughunter it also crashes nightly and aurora builds
Posted file windows stack
Brian, Wan-Teh, looks like this is a regression caused by NSS 3.15.4 beta.

The crash happens only on the branches where this beta is currently being used.
Flags: needinfo?(brian)
Assignee: nobody → brian
Upgrading to blocker, because we are intending to release NSS within the next few days.
Severity: normal → blocker
Component: Security → Libraries
Product: Core → NSS
Version: Trunk → 3.15.4
Tomcat: 
  Did this happen once? Or multiple times?
  Are you able to reproduce?

Using the latest NSS version (used by mozilla-central+aurora) and command line tools to connect to this server: no problem
I was able to reproduce on Linux/mozilla-inbound.

(gdb) print ss->ssl3.hs.hashType
$1 = handshake_hash_combo

#7  0x00007fabe3cc383f in PR_Assert (s=0x7fabe2c957d8 "ss->ssl3.hs.hashType == handshake_hash_unknown", file=0x7fabe2c93c20 "ssl3con.c", ln=3823) at /pd/moz/inbound/mozilla/nsprpub/pr/src/io/prlog.c:554
#8  0x00007fabe2c5f84b in ssl3_InitHandshakeHashes (ss=0x7fabaf922000) at ssl3con.c:3823
#9  0x00007fabe2c64309 in ssl3_HandleServerHello (ss=0x7fabaf922000, 
    b=0x7fabb1ef3006 "\203N\345D\265\363\226&\031-\353z\266&f\025{\200\036\244ý\370\203\360\241|J\276\315\027\t W)D\017O\253\376\254\335\352\375\226\002\216[k\002E\030\003", length=68) at ssl3con.c:6171
#10 0x00007fabe2c6de66 in ssl3_HandleHandshakeMessage (ss=0x7fabaf922000, 
    b=0x7fabb1ef3004 "\003\001\203N\345D\265\363\226&\031-\353z\266&f\025{\200\036\244ý\370\203\360\241|J\276\315\027\t W)D\017O\253\376\254\335\352\375\226\002\216[k\002E\030\003", length=70) at ssl3con.c:10670
#11 0x00007fabe2c6e43f in ssl3_HandleHandshake (ss=0x7fabaf922000, origBuf=0x7fabaf9223b0) at ssl3con.c:10814
#12 0x00007fabe2c6fac0 in ssl3_HandleRecord (ss=0x7fabaf922000, cText=0x0, databuf=0x7fabaf9223b0) at ssl3con.c:11483
#13 0x00007fabe2c712a2 in ssl3_GatherCompleteHandshake (ss=0x7fabaf922000, flags=0) at ssl3gthr.c:325
#14 0x00007fabe2c744d7 in ssl_GatherRecord1stHandshake (ss=0x7fabaf922000) at sslcon.c:1225
#15 0x00007fabe2c80f6a in ssl_Do1stHandshake (ss=0x7fabaf922000) at sslsecur.c:109
#16 0x00007fabe2c83907 in ssl_SecureRecv (ss=0x7fabaf922000, buf=0x7fabc5bfdb30 "", len=4000, flags=0) at sslsecur.c:1213
#17 0x00007fabe2c8d37c in ssl_Recv (fd=0x7fabc1d6f1c0, buf=0x7fabc5bfdb30, len=4000, flags=0, timeout=4294967295) at sslsock.c:2038
...
...PSM and gecko callers...
The asserting code was introduced in bug 480514 (implement TLS 1.2).
Depends on: 480514
I cannot reproduce in Chromium I'm afraid.
fyi, this has occurred a total of 13 times in the past week counting the occurrences on Linux, OSX, Windows on Aurora and Nightly.
(In reply to Adam Langley from comment #7)
> I cannot reproduce in Chromium I'm afraid.

This is happening with NSS_3_15_4_BETA3, which has false start code.

Could that new code cause old assumptions to be false?
This code sets handshake_hash_combo, while WRITING data:

Breakpoint 8, ssl3_InitHandshakeHashes (ss=0x7fffc7322000) at ssl3con.c:3903
3903		    ss->ssl3.hs.hashType = handshake_hash_combo;
(gdb) print ss->version
$31 = 769
(gdb) bt
#0  ssl3_InitHandshakeHashes (ss=0x7fffc7322000) at ssl3con.c:3903
#1  0x00007ffff5964309 in ssl3_HandleServerHello (ss=0x7fffc7322000, b=0x7fffc289c006 "\203\277\022$\365<\315~\"_\330\302\064\261\371\371\204\253G>\352\"\236", <incomplete sequence \351>, length=68) at ssl3con.c:6171
#2  0x00007ffff596de66 in ssl3_HandleHandshakeMessage (ss=0x7fffc7322000, b=0x7fffc289c004 "\003\001\203\277\022$\365<\315~\"_\330\302\064\261\371\371\204\253G>\352\"\236", <incomplete sequence \351>, length=70) at ssl3con.c:10670
#3  0x00007ffff596e43f in ssl3_HandleHandshake (ss=0x7fffc7322000, origBuf=0x7fffc73223b0) at ssl3con.c:10814
#4  0x00007ffff596fac0 in ssl3_HandleRecord (ss=0x7fffc7322000, cText=0x7fffd88fe7b0, databuf=0x7fffc73223b0) at ssl3con.c:11483
#5  0x00007ffff59714a9 in ssl3_GatherCompleteHandshake (ss=0x7fffc7322000, flags=0) at ssl3gthr.c:372
#6  0x00007ffff59744d7 in ssl_GatherRecord1stHandshake (ss=0x7fffc7322000) at sslcon.c:1225
#7  0x00007ffff5980f6a in ssl_Do1stHandshake (ss=0x7fffc7322000) at sslsecur.c:109
#8  0x00007ffff5983cd3 in ssl_SecureSend (ss=0x7fffc7322000, buf=0x7ffff2dda6df "", len=0, flags=0) at sslsecur.c:1284
#9  0x00007ffff598d474 in ssl_Send (fd=0x7fffd4e67f70, buf=0x7ffff2dda6df, len=0, flags=0, timeout=4294967295) at sslsock.c:2059
#10 0x00007ffff1a14215 in PSMSend (fd=0x7fffc8602130, buf=0x7ffff2dda6df, amount=0, flags=0, timeout=4294967295) at /pd/moz/inbound/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp:1269
#11 0x00007ffff1a142d4 in nsSSLIOLayerWrite (fd=0x7fffc8602130, buf=0x7ffff2dda6df, amount=0) at /pd/moz/inbound/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp:1286
#12 0x00007ffff69bf630 in PR_Write (fd=0x7fffc8602130, buf=0x7ffff2dda6df, amount=0) at /pd/moz/inbound/mozilla/nsprpub/pr/src/io/priometh.c:114


Shortly afterwards, while READING data:

Assertion failure: ss->ssl3.hs.hashType == handshake_hash_unknown, at ssl3con.c:3823

Program received signal SIGABRT, Aborted.
0x00007ffff6e149e9 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff6e149e9 in raise () from /lib64/libc.so.6
#1  0x00007ffff6e160f8 in abort () from /lib64/libc.so.6
#2  0x00007ffff69c383f in PR_Assert (s=0x7ffff59957d8 "ss->ssl3.hs.hashType == handshake_hash_unknown", file=0x7ffff5993c20 "ssl3con.c", ln=3823) at /pd/moz/inbound/mozilla/nsprpub/pr/src/io/prlog.c:554
#3  0x00007ffff595f84b in ssl3_InitHandshakeHashes (ss=0x7fffc7322000) at ssl3con.c:3823
#4  0x00007ffff5964309 in ssl3_HandleServerHello (ss=0x7fffc7322000, b=0x7fffc289c006 "\203\277\022$\365<\315~\"_\330\302\064\261\371\371\204\253G>\352\"\236", <incomplete sequence \351>, length=68) at ssl3con.c:6171
#5  0x00007ffff596de66 in ssl3_HandleHandshakeMessage (ss=0x7fffc7322000, b=0x7fffc289c004 "\003\001\203\277\022$\365<\315~\"_\330\302\064\261\371\371\204\253G>\352\"\236", <incomplete sequence \351>, length=70) at ssl3con.c:10670
#6  0x00007ffff596e43f in ssl3_HandleHandshake (ss=0x7fffc7322000, origBuf=0x7fffc73223b0) at ssl3con.c:10814
#7  0x00007ffff596fac0 in ssl3_HandleRecord (ss=0x7fffc7322000, cText=0x0, databuf=0x7fffc73223b0) at ssl3con.c:11483
#8  0x00007ffff59712a2 in ssl3_GatherCompleteHandshake (ss=0x7fffc7322000, flags=0) at ssl3gthr.c:325
#9  0x00007ffff59744d7 in ssl_GatherRecord1stHandshake (ss=0x7fffc7322000) at sslcon.c:1225
#10 0x00007ffff5980f6a in ssl_Do1stHandshake (ss=0x7fffc7322000) at sslsecur.c:109
#11 0x00007ffff5983907 in ssl_SecureRecv (ss=0x7fffc7322000, buf=0x7fffd88fdb30 "", len=4000, flags=0) at sslsecur.c:1213
#12 0x00007ffff598d37c in ssl_Recv (fd=0x7fffd4e67f70, buf=0x7fffd88fdb30, len=4000, flags=0, timeout=4294967295) at sslsock.c:2038
#13 0x00007ffff1a14123 in PSMRecv (fd=0x7fffc8602130, buf=0x7fffd88fdb30, amount=4000, flags=0, timeout=4294967295) at /pd/moz/inbound/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp:1240
#14 0x00007ffff1a142a1 in nsSSLIOLayerRead (fd=0x7fffc8602130, buf=0x7fffd88fdb30, amount=4000) at /pd/moz/inbound/mozilla/security/manager/ssl/src/nsNSSIOLayer.cpp:1280
#15 0x00007ffff69bf600 in PR_Read (fd=0x7fffc8602130, buf=0x7fffd88fdb30, amount=4000) at /pd/moz/inbound/mozilla/nsprpub/pr/src/io/priometh.c:109
From the above, ssl3_HandleServerHello is happening twice for the same |ss|. This could be a renegotiation, but I don't see an obvious bug in the code for that case: ssl3_SendClientHello calls ssl3_RestartHandshakeHashes which resets the state, including hashType.
The problem seems to have to deal with the handleRecordNow logic in ssl3_GatherCompleteHandshake. I am investigating it now.
Flags: needinfo?(brian)
Brian: I agree. I suspect the same ServerHello message was handled twice.
In the call stack containing the second ssl3_HandleServerHello call,
ssl3_GatherCompleteHandshake passes a NULL cText to ssl3_HandleRecord.
Priority: -- → P1
Target Milestone: --- → 3.15.4
I believe this is due to mis-use of NSS by Gecko.

nsSocketTransport::IsAlive is calling PR_Recv(fd, &c, 1, PR_MSG_PEEK, 0) on the SSL socket. This causes us to drive the handshake forward. This causes us to process the ServerHello, and ssl3_HandleServerHello fails.

Later, we'll try to PR_Read the socket again to actually get application data out of it. This will cause us to try to drive the handshake forward again. ssl3_GatherCompleteHandshake will see the data from the ServerHello in the buffer, and that triggers the handleRecordNow.

The bug is in the application, because once an I/O operation (SSL_ForceHandshake/PR_Write/PR_Recv/etc.) fails, then you are not allowed to attempt any more I/O operations on the socket.

Now I need to figure out why we regressed this.

 	nss3.dll!ssl3_HandleServerHello
 	nss3.dll!ssl3_HandleHandshakeMessage
 	nss3.dll!ssl3_HandleHandshake
 	nss3.dll!ssl3_HandleRecord
 	nss3.dll!ssl3_GatherCompleteHandshake
 	nss3.dll!ssl_GatherRecord1stHandshake
 	nss3.dll!ssl_Do1stHandshake
 	nss3.dll!ssl_SecureRecv
 	nss3.dll!ssl_Recv
 	xul.dll!PSMRecv
 	xul.dll!nsSSLIOLayerRead
 	nss3.dll!PR_Read
>	xul.dll!nsSocketInputStream::Read
 	xul.dll!nsHttpConnection::Close
 	xul.dll!nsHttpConnectionMgr::OnMsgReclaimConnection
 	xul.dll!nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady
 	xul.dll!nsSocketOutputStream::OnSocketReady
 	xul.dll!nsSocketTransport::OnSocketReady
 	xul.dll!nsSocketTransportService::DoPollIteration
Whiteboard: [needs security rating]
I have narrowed down the regression range so far:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=99084b22e38d&tochange=59f6274ce8f1

These ones seem most likely:

78fd5daacc01	Brian Smith — Bug 901718, Part 2: fix comment, r=me
e24e7d00b56d	Brian Smith — Bug 934663 followup: fix unused variable warning, r=me
a8f5e75b8ed8	Brian Smith — Bug 901718: Remove TLS intolerance fallback from TLS 1.0 to SSL 3.0 for connection resets, r=honzab
35c1e12a0772	Brian Smith — Bug 898431: Update to NSS 3.15.4 beta 3 (NSS_3_15_4_BETA3), r=me
cb06029a7f43	Brian Smith — Bug 937984: Allow client.py to pull NSPR and NSS from a user-specified repo, r=kaie
53dffb3da446	Brian Smith — Bug 707275, Part 2: Add telemetry for cipher suites and key sizes, r=keeler
2f3ce72ec2d8	Brian Smith — Bug 707275, Part 1: Add telemetry for TLS intolerance, r=keeler
hg bisected and found that the NSS update is what caused the regression:

> 35c1e12a0772	Brian Smith — Bug 898431: Update to NSS 3.15.4 beta 3
> (NSS_3_15_4_BETA3), r=me

Will bisect the NSS update now.
I traced the regression to:

changeset:   10951:968dc8105c00
user:        Brian Smith <brian@briansmith.org>
date:        Mon Nov 04 18:30:57 2013 -0800
summary:     Bug 936828: Change order of cipher suites offered in client hello ...
With the new cipher suite order, Wireshark shows that the server chooses:

    Cipher Suite: TLS_RSA_WITH_DES_CBC_SHA (0x0009)

With the old cipher suite order, Wireshark shows that the server chooses:

    Cipher Suite: TLS_RSA_WITH_3DES_EDE_CBC_SHA (0x000a)

Note that Firefox doesn't enable TLS_RSA_WITH_DES_CBC_SHA, so ssl3_HandleServerHello fails, since the server never should have selected that one. So, this is an example of a cipher-suite-order-intolerant server.

However, due to some *other* problem (probably in Gecko), we end up processing the ServerHello twice. I suspect that is a regression in one of the other changesets mentioned in comment 15. I will reorder the changesets and find out which one caused the trouble.

tl;dr: There are two bugs: a compatibility issue with the cipher suite order change, and a more severe bug about processing the ServerHello twice when the ssl3_HandleServerHello fails.
Maybe Dan has some idea about a sec rating here.
Flags: needinfo?(dveditz)
In an optimized build I don't see any obvious issues, but I don't know if that's because the behavior is different or if the asserted condition is simply handled safely. The site ends up negotiating SSL_RSA_WITH_3DES_EDE_CBC_SHA in my Nightly build despite the oddness Brian saw from the site in comment 18 so maybe there was a fallback or retry.

 (In reply to Andrew McCreight [:mccr8] from comment #19)
> Maybe Dan has some idea about a sec rating here.

Not really. If we're handling the asserted condition safely (I don't know if we are or if we're just getting lucky) then there may be no security problem at all.
Flags: needinfo?(dveditz)
Component: Libraries → Security: PSM
Product: NSS → Core
Target Milestone: 3.15.4 → mozilla28
Version: 3.15.4 → Trunk
Honza and David, please both review this.

The PSM NSPR I/O layer works by having each method that could drive the handshake call getSocketInfoIfRunning at the beginning of the function. If the nsNSSSocketInfo had previously been canceled due to error, getSocketInfoIfRunning will return NULL after calling PR_SetError() with the previously-set error code. This will cause the caller of getSocketInfoIfRunning to stop early and indicate failure to its caller. Thus, the PSM I/O layer is designed to protect against the type of abuse that Gecko is trying to inflict upon it.

However, in the case of TLS intolerance fallback, we are not calling SetCanceled on the nsNSSSocketInfo object, and consequently we are not saving the error code. Thus, when we abuse the I/O layer by attempting an I/O operation after the failed one, we don't notice. Since the libssl I/O layer underneath the PSM I/O layer is in an inconsistent state (due to the error previously encountered), we have unpredictable and undefined behavior. In this case, the result is trying to handle the same TLS record twice.

This patch avoids that problem by remembering the error code that triggered the TLS intolerance in the nsNSSSocketInfo object.

I filed bug 946147 to track the cipher suite intolerance issue with the site in question.

I filed bug 946157 about the bug with nsSocketTransport::IsAlive being unsafe.
Attachment #8342268 - Flags: review?(honzab.moz)
Attachment #8342268 - Flags: review?(dkeeler)
Comment on attachment 8342268 [details] [diff] [review]
prevent-bad-things-from-happening-for-tls-intolerance.patch

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

As far as I can tell, this looks correct.

::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ +1118,5 @@
> +    // Remember that we encountered an error so that getSocketInfoIfRunning
> +    // will correctly cause us to fail if another part of Gecko
> +    // (erroneously) calls an I/O function (PR_Send/PR_Recv/etc.) again on
> +    // this socket. Note that we use the original error because if we use
> +    // PR_CONNECT_RESET_ERROR, bad things will happen.

"bad things will happen" -> is this deliberately vague? It would be nice to explain exactly which bad things will happen.
Attachment #8342268 - Flags: review?(dkeeler) → review+
(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #21)
> Thus, when we abuse the I/O layer by attempting an
> I/O operation after the failed one, we don't notice. Since the libssl I/O
> layer underneath the PSM I/O layer is in an inconsistent state (due to the
> error previously encountered), 

Hmm.. but the NSS I/O layer should be aware of this error (reset/eof/ssl-alert) so how is it that it is in an inconsistent state?  NSS should switch the socket itself to an error-state.

So, if any consumer of NSS sockets (direct NSS) will try to do a write(s) after an error has been encountered on the socket, it is possible this bug will trigger as well?

> we have unpredictable and undefined behavior.
> In this case, the result is trying to handle the same TLS record twice.
> 

Sounds more like an NSS bug to me.
Comment on attachment 8342268 [details] [diff] [review]
prevent-bad-things-from-happening-for-tls-intolerance.patch

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

checkHandshake is called after all I/O ops we can do on the socket, so this OK.

I think to fix an immediate Necko over PSM bug this is OK.  But see my previous comment, this is strange behavior of NSS.

::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ +1118,5 @@
> +    // Remember that we encountered an error so that getSocketInfoIfRunning
> +    // will correctly cause us to fail if another part of Gecko
> +    // (erroneously) calls an I/O function (PR_Send/PR_Recv/etc.) again on
> +    // this socket. Note that we use the original error because if we use
> +    // PR_CONNECT_RESET_ERROR, bad things will happen.

I agree with David, better explanation would be good, if not a sec risk.

At least explain this in more details in the bug.  I presume RESET would cause fallback when we potentially don't want it? At least it will cause any http transactions to retry.
Attachment #8342268 - Flags: review?(honzab.moz) → review+
I am giving this a sec-high rating because I'm not going to put a huge amount of effort into trying to convince myself that there is no critical security bug here. The libssl state machine is complicated, and there are so many error code paths, it basically impossible to say that none of those paths leave the connection in a non-exploitable state. However, I'm also not motivated to investigate how one would actually exploit this, and also too apathetic about security ratings to argue with anybody that lowers the severity rating. So, correct the rating as you please. :)
Keywords: sec-critical
Hardware: x86 → All
Whiteboard: [needs security rating]
And by sec-high in the previous comment, I meant sec-critical.

I would like to check this patch in soon, and then uplift it to Firefox 27 and ESR 24, at least.

[Security approval request comment]
How easily could an exploit be constructed based on the patch? It isn't obvious how to construct an exploit. It would require research into the error handling within libssl.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Definitely the patch narrows down the search space to libssl, but also libssl is pretty large.

Which older supported branches are affected by this flaw? All of them.

If not all supported branches, which bug introduced the flaw? Pretty sure this flaw has existed for a long time.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This will apply cleanly to Firefox 26, 27, 28. ESR 24 and B2G < 1.2 would require small modifications to the patch, if we're going to backport this.

How likely is this patch to cause regressions; how much testing does it need? There is a non-zero risk of this patch causing problems. It is very difficult to write an automated test for this, because the problematic behaviors are in the bowels of Necko and PSM. So, I think that this should get a few weeks of testing, at least, before release.
Attachment #8342268 - Attachment is obsolete: true
Attachment #8343641 - Flags: sec-approval?
Attachment #8343641 - Flags: review+
Attachment #8343641 - Flags: approval-mozilla-beta?
Attachment #8343641 - Flags: approval-mozilla-aurora?
Comment on attachment 8343641 [details] [diff] [review]
Patch v2, which fixes the vague comment

Giving sec-approval for trunk. In a couple of days, that will be aurora (since we're about to branch). We should land it on beta after that.
Attachment #8343641 - Flags: sec-approval? → sec-approval+
awesome thanks brian!

Also landed this on central now -> https://hg.mozilla.org/mozilla-central/rev/f749a52bd4e6

Setting in-testsuite but not sure if we can create a test here, at least i was unsuccessful creating a local testcase :(
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Can we can uplift noms please?
Flags: needinfo?(brian)
Is it cool to uplift this now? The approval request is in comment 26.
Flags: needinfo?(brian) → needinfo?(lsblakk)
(In reply to Carsten Book [:Tomcat] from comment #29)
> awesome thanks brian!
> 
> Also landed this on central now ->
> https://hg.mozilla.org/mozilla-central/rev/f749a52bd4e6

Also verified this site is no more breaking Trunk Debug Builds with this fix included on the site where i found the assertion, also bughunter confirmed this crash only on beta builds anymore
Comment on attachment 8343641 [details] [diff] [review]
Patch v2, which fixes the vague comment

This is already on Aurora by virtue of landing on m-c prior to Monday's uplift.
Attachment #8343641 - Flags: approval-mozilla-aurora? → approval-mozilla-esr24?
Comment on attachment 8343641 [details] [diff] [review]
Patch v2, which fixes the vague comment

Go ahead with uplifts to beta/esr24
Attachment #8343641 - Flags: approval-mozilla-esr24?
Attachment #8343641 - Flags: approval-mozilla-esr24+
Attachment #8343641 - Flags: approval-mozilla-beta?
Attachment #8343641 - Flags: approval-mozilla-beta+
Flags: needinfo?(lsblakk)
https://hg.mozilla.org/releases/mozilla-beta/rev/c5ed9aa75a0a

This has some non-trival conflicts for esr24. Please provide a branch patch for uplift.
Flags: needinfo?(brian)
Comment on attachment 8343641 [details] [diff] [review]
Patch v2, which fixes the vague comment

Removing the approval for this patch, we can a+ the known-good replacement when it's up.
Attachment #8343641 - Flags: approval-mozilla-esr24+ → approval-mozilla-esr24-
Brian, can you please post an esr24 patch for this sec-crit?
8 days later and we're running out of time to take this on ESR24. Are we going to get a patch for this in time for the release, Brian?
This has missed the ESR24 going out with Firefox 27. Let's try to get this before the March release for ESR24.
Whiteboard: [adv-main27+]
Comment on attachment 8373493 [details] [diff] [review]
esr24-with-extra-logic

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

This combines logic from http://hg.mozilla.org/mozilla-central/diff/2f3ce72ec2d8/security/manager/ssl/src/nsNSSIOLayer.cpp and http://hg.mozilla.org/mozilla-central/diff/f749a52bd4e6/security/manager/ssl/src/nsNSSIOLayer.cpp so that the central patch makes sense.
Attachment #8373493 - Flags: review?(honzab.moz)
Comment on attachment 8373493 [details] [diff] [review]
esr24-with-extra-logic

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

This is merge of
https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=942152&attachment=8343641
and
http://hg.mozilla.org/mozilla-central/diff/2f3ce72ec2d8/security/manager/ssl/src/nsNSSIOLayer.cpp

(as Camilo points out actually :))

and it looks good I think.
Attachment #8373493 - Flags: review?(honzab.moz) → review+
Comment on attachment 8373493 [details] [diff] [review]
esr24-with-extra-logic

Same as comment 26.
Attachment #8373493 - Flags: approval-mozilla-esr24?
Attachment #8373493 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
Whiteboard: [adv-main27+] → [adv-main27+][adv-esr24.4+]
Marking qa-, as verifying exploit mitigation in the absence of steps to reproduce would yield diminishing returns. Also, comment 25.
Whiteboard: [adv-main27+][adv-esr24.4+] → [adv-main27+][adv-esr24.4+][qa-]
The tracking flag for 29 looks to be in error, removing.
Group: core-security
You need to log in before you can comment on or make changes to this bug.