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

Status: RESOLVED FIXED

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

Description

[Carsten Book [:Tomcat]]

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

Comment 1

[Carsten Book [:Tomcat]]

Attachment: windows stack

Comment 2

[Kai Engert (:KaiE:)]

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.

Comment 3

[Kai Engert (:KaiE:)]

Upgrading to blocker, because we are intending to release NSS within the next few days.

Comment 4

[Kai Engert (:KaiE:)]

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

Comment 5

[Kai Engert (:KaiE:)]

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...

Comment 6

[Kai Engert (:KaiE:)]

The asserting code was introduced in bug 480514 (implement TLS 1.2).

Comment 7

[Adam Langley]

I cannot reproduce in Chromium I'm afraid.

Comment 8

[Bob Clary [:bc] (inactive)]

fyi, this has occurred a total of 13 times in the past week counting the occurrences on Linux, OSX, Windows on Aurora and Nightly.

Comment 9

[Kai Engert (:KaiE:)]

(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?

Comment 10

[Kai Engert (:KaiE:)]

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

Comment 11

[Adam Langley]

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.

Comment 12

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The problem seems to have to deal with the handleRecordNow logic in ssl3_GatherCompleteHandshake. I am investigating it now.

Comment 13

[Wan-Teh Chang]

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.

Comment 14

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

Comment 15

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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

Comment 16

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 17

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

I traced the regression to:

changeset:   10951:968dc8105c00
user:        Brian Smith <[e-mail]>
date:        Mon Nov 04 18:30:57 2013 -0800
summary:     Bug 936828: Change order of cipher suites offered in client hello ...

Comment 18

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 19

[Andrew McCreight [:mccr8]]

Maybe Dan has some idea about a sec rating here.

Comment 20

[Daniel Veditz [: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.

Comment 21

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: prevent-bad-things-from-happening-for-tls-intolerance.patch

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.

Comment 22

[Dana Keeler (she/her) (use needinfo) [:keeler]]

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.

Comment 23

[Honza Bambas (:mayhemer)]

(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 24

[Honza Bambas (:mayhemer)]

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.

Comment 25

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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. :)

Comment 26

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Patch v2, which fixes the vague comment

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.

Comment 27

[Al Billings [:abillings - ex-MoCo]]

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.

Comment 28

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f749a52bd4e6

Comment 29

[Carsten Book [:Tomcat]]

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 :(

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Can we can uplift noms please?

Comment 31

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Is it cool to uplift this now? The approval request is in comment 26.

Comment 32

[Carsten Book [:Tomcat]]

(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 33

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 34

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 8343641 [details] [diff] [review]
Patch v2, which fixes the vague comment

Go ahead with uplifts to beta/esr24

Comment 35

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c5ed9aa75a0a

This has some non-trival conflicts for esr24. Please provide a branch patch for uplift.

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/69e730e3f88a

Comment 37

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

Brian, can you please post an esr24 patch for this sec-crit?

Comment 39

[Al Billings [:abillings - ex-MoCo]]

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?

Comment 40

[Al Billings [:abillings - ex-MoCo]]

This has missed the ESR24 going out with Firefox 27. Let's try to get this before the March release for ESR24.

Comment 41

[Camilo Viecco (:cviecco)]

Attachment: esr24-with-extra-logic

Comment 42

[Camilo Viecco (:cviecco)]

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.

Comment 43

[Honza Bambas (:mayhemer)]

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.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8373493 [details] [diff] [review]
esr24-with-extra-logic

Same as comment 26.

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/66a389d8a877
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ee38869a684b

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/ee38869a684b

Comment 47

[Matt Wobensmith [:mwobensmith][:matt:]]

Marking qa-, as verifying exploit mitigation in the absence of steps to reproduce would yield diminishing returns. Also, comment 25.

Comment 48

[Lukas Blakk [:lsblakk] use ?needinfo]

The tracking flag for 29 looks to be in error, removing.