Closed
Bug 629090
Opened 13 years ago
Closed 13 years ago
NSS's J-PAKE Implementation Allows Session Key Retrieval Without Knowing The Shared Secret
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(blocking2.0 betaN+)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: mcoates, Assigned: briansmith)
Details
(Whiteboard: [MOZ2-8][sg:high][infrasec:crypto][ws:high][hardblocker][has patch])
Attachments
(2 files, 2 obsolete files)
123.93 KB,
application/pdf
|
Details | |
6.40 KB,
patch
|
briansmith
:
review+
|
Details | Diff | Splinter Review |
This issue was reported by our third party security reviewers. The full security issue is attached and includes steps to reproduce this issue. Summary: The J-PAKE implementation in Firefox browser's Sync client doesn't seem to check if the other party has provided g^x2 equal to 1 (which would mean that x2 is 0). Additionally, neither Firefox Sync nor Firefox Home seem to set the upper bound for g^x2 (or perform an initial modulo p operation on it) to prevent the other party from providing number p+1 which is in effect identical to 1. This allows an active attacker (rogue server) to masquerade as a legitimate peer without knowing the shared secret. As a result, a rogue server can first extract the Sync credentials from the "sender", and then immediately pass them on to the "receiver", thus avoiding the legitimate J-PAKE parties from noticing anything suspect. Recommendations: We recommend that the Firefox Sync client, like the Firefox Home, checks the received g^x2 (or g^x4) to be not equal to 1. In order to avoid the vulnerability described in http://seb.dbzteam.org/crypto/jpake-session-key-retrieval.pdf, the received g^x2 (or g^x4) must either be checked not to exceed p or be reduced modulo p immediately upon receipt (since p+1, 2p+1, etc. are also identical to 1 in the context of this calculation). Finally, the received A (or B) must also be not equal to 1.
This issue as reported is in the fx-sync and Firefox Home products, not the server product. Moving to client component.
Component: Server: Key Exchange → Firefox Sync: Backend
QA Contact: key-exchange-server → sync-backend
Comment 2•13 years ago
|
||
Brian, please take a look here.
Assignee: nobody → bsmith
blocking2.0: --- → betaN+
Component: Firefox Sync: Backend → Firefox Sync: Crypto
QA Contact: sync-backend → sync-crypto
Whiteboard: [MOZ2-8][infrasec:crypto][ws:high] → [MOZ2-8][infrasec:crypto][ws:high][hardblocker]
Comment 3•13 years ago
|
||
Interesting. We did actually fix the code based on http://seb.dbzteam.org/crypto/jpake-session-key-retrieval.pdf I will take a thorough look to see if we missed a case. (The changes that I made were reviewed by the author of the above paper, Ben Laurie (who put the same fixes in OpenSSL) and the original author or JPAKE. It is of course very possible that we missed something.
Comment 4•13 years ago
|
||
(In reply to comment #3) > Interesting. We did actually fix the code based on > > http://seb.dbzteam.org/crypto/jpake-session-key-retrieval.pdf > > I will take a thorough look to see if we missed a case. Note that Firefox Home has a separate bug 629133. Clarifying that this bug is about the implementation in NSS.
Summary: J-PAKE Implementation Allows Session Key Retrieval Without Knowing The Shared Secret → NSS's J-PAKE Implementation Allows Session Key Retrieval Without Knowing The Shared Secret
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to comment #0) > We recommend that the Firefox Sync client, like the Firefox Home, checks the > received g^x2 (or g^x4) to be not equal to 1. IIRC, this check is subsumed by one of the other checks, but I can't remember which one. But, maybe I remember incorrectly. This is something I will investigate tonight. > In order to avoid the vulnerability described in > http://seb.dbzteam.org/crypto/jpake-session-key-retrieval.pdf, the received > g^x2 (or g^x4) must either be checked not to exceed p or be reduced modulo p > immediately upon receipt (since p+1, 2p+1, etc. are also identical to 1 in the > context of this calculation). NSS's J-PAKE does this check in JPAKE_Verify in jpake.c: /* Check g^x is in [1, p-2], R is in [0, q-1], and (g^x)^q mod p == 1 */ 292 if (!(mp_cmp_z(&GX) > 0 && 293 mp_cmp(&GX, &p_minus_1) < 0 && 294 mp_cmp(&R, &q) < 0 && 295 mp_cmp_d(&one, 1) == 0)) { 296 goto badSig; 297 }
Comment 6•13 years ago
|
||
If this is an NSS issue, shouldn't this be moved over to the NSS product since that's where the code lives and where the NSS team can put any fixes through their QA process?
Comment 7•13 years ago
|
||
(In reply to comment #6) > If this is an NSS issue, shouldn't this be moved over to the NSS product since > that's where the code lives and where the NSS team can put any fixes through > their QA process? Quite likely. It feels like this kind of check should live in the NSS bits and pieces (if it doesn't already) and not in the glue code in services/crypto/. I'll leave it to bsmith to make that call.
Assignee | ||
Comment 8•13 years ago
|
||
To minimize scheduling risk, I am add the necessary check that gx^4 != 0 to the services/crypto code. This way, we do not end up waiting for a new NSS release and it will also protect Firefox from the user/administrator configuring Firefox to use an older version of FreeBL. I am building+testing this patch now. In addition, I will file a new bug in NSS to track the addition of the same check to FreeBL so that the check is done where it should be.
Assignee | ||
Comment 9•13 years ago
|
||
(In reply to comment #8) > To minimize scheduling risk, I am add the necessary check that gx^4 != 0 I meant g^x4 > 1.
Comment 10•13 years ago
|
||
Brian, ETA on the fix?
Assignee | ||
Comment 11•13 years ago
|
||
This afternoon.
Updated•13 years ago
|
Assignee: bsmith → nobody
Group: client-services-security → core-security
Component: Firefox Sync: Crypto → Libraries
Product: Mozilla Services → NSS
QA Contact: sync-crypto → libraries
Updated•13 years ago
|
Assignee: nobody → bsmith
Assignee | ||
Comment 12•13 years ago
|
||
ETA: today. The actual patch has been written, but I am working on getting the testcase to actually reproduce the issue.
Updated•13 years ago
|
Whiteboard: [MOZ2-8][infrasec:crypto][ws:high][hardblocker] → [MOZ2-8][sg:high][infrasec:crypto][ws:high][hardblocker]
Comment 13•13 years ago
|
||
Brian, where are we here? If we can't effectively test this yet, can we at least get the patch in place?
Assignee | ||
Comment 14•13 years ago
|
||
Verification procedure: 1. Apply this patch 2. Run: make -C services/crypto/component/tests/ xpcshell-tests Before the bug fix patch (coming next) is applied, the first test (g^x == p + 1) is successfully blocked but the second one (g^x == 1) is not blocked. After the bug fix, the g^x==1 case is successfully blocked.
Attachment #509874 -
Flags: review?(philipp)
Assignee | ||
Comment 15•13 years ago
|
||
The upcoming changes to FreeBL are being tracked in Bug 631654. This patch will to Firefox's nsSyncJPAKE component will allow us to safely ship Firefox 4 without waiting for an updated NSS release with the check added.
Attachment #509878 -
Flags: review?(mconnor)
Updated•13 years ago
|
Whiteboard: [MOZ2-8][sg:high][infrasec:crypto][ws:high][hardblocker] → [MOZ2-8][sg:high][infrasec:crypto][ws:high][hardblocker][has patch]
Updated•13 years ago
|
Attachment #509878 -
Flags: review?(philipp)
Attachment #509878 -
Flags: review?(mconnor)
Attachment #509878 -
Flags: review+
Updated•13 years ago
|
Attachment #509878 -
Flags: review?(philipp) → review+
Comment 16•13 years ago
|
||
Comment on attachment 509874 [details] [diff] [review] Testcase demonstrating g^x==1 is not rejected but g^x==p+1 is rejected >+function test_gx4_zero() { That name might be a bit misleading, I suggest test_x4_zero() or test_gx4_one(). >+ // x is NIST 3072's p + 1, (p + 1) mod p == 1, g^1 == 0 I think you meant to say here: // gx is NIST 3072's p + 1, (p + 1) mod p == 1, x == 0 Looks good otherwise!
Attachment #509874 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 17•13 years ago
|
||
Updated testcase patch with philiKON's comments addressed combined with the wallpaper fix patch. r+ carried over.
Attachment #509874 -
Attachment is obsolete: true
Attachment #509878 -
Attachment is obsolete: true
Attachment #510938 -
Flags: review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 18•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/2e453fece8c9
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
This isn't checkin-needed anymore, is it?
Updated•13 years ago
|
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•