1631597 - (CVE-2020-12402) side channel vulnerabilities during RSA key generation

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Daniel Veditz [:dveditz]]

Attachment: nss_rsa_beea.png

[filed from mail to security@ from Billy Brumley]
Hello,

I lead a team of researchers specializes in security and applied
cryptography, in particular both SW and HW-based Side Channel Analysis
(SCA). We recently started a project to assess SCA security in NSS. Part
of that assessment turned up some weaknesses in your implementation of
RSA, led by Nacho (in CC).

Please find the report below -- we look forward to opening a dialogue with
you during the disclosure process.

Billy Brumley, D.Sc. (Tech.)
Associate Professor
Tampere University
Tampere, FINLAND
https://research.tuni.fi/nisec/

Code path

In the attached debug session (debug.txt), using certutil to generate an
RSA key pair, in computation of

mp_lcm(&psub1, &qsub1, &phi)

calls

mp_gcd(&psub1, &qsub1, &gcd)

where psub1 = p - 1 and qsub1 = q -1 are both secrets.

It continues to compute

mp_invmod(e, &phi, d)

where phi = (p - 1)*(q - 1) is a secret.

It continues to compute

mp_invmod(q, p, &tmp)

where q and p are both secrets.

It continues to compute

mp_gcd(&e, &psub1, &res)

where psub1 = p - 1 is a secret.

It continues to compute

mp_gcd(&e, &qsub1, &res)

where qsub1 = q - 1 is a secret.

Vulnerabilities

Both mp_gcd and mp_invmod are based on variations of the Binary Extended
Euclidean Algorithm (BEEA), which is known to be susceptible to various
forms of SCA due to it's extremely input-dependent control flow. In our
particular attack, we targeted mp_gcd with an EM attack, measure
ElectroMagnetic (EM) emanations. In the attached figure, you see a
post-filtered trace showing part of the computations mp_gcd(&e, &psub1,
&res) which is followed (not shown) by mp_gcd(&e, &qsub1, &res). The peaks
identify multi-precision subtractions, while the distance between the
peeks identify the number of shifts. Using these traces and the
number-theoretic guarantees of RSA, this allows us to completely recover
the RSA primes and thus secret key.

While our end-to-end attack implementation uses the EM signal, it is
important to note this codepath is susceptible to any number of SCA
techniques such as cache-timings and controlled-channel attacks. Aldaya et
al. 1 exploited such a weakness in OpenSSL to recover RSA keys with
cache timings, and Weiser et al. 2 with page faults.

To summarize, this vulnerability is mostly analogous to CVE-2018-0737 4
which was issued by OpenSSL for their library.

Countermeasures

There are several possible countermeasures, but these are not oneliner
fixes and will take some effort. However, my team has put such effort into
the OpenSSL library 3, and we are willing to assist to arrive at a
suitable solution for you and your customers.

Disclosure

We are aiming for public disclosure on 18 May. However, this date is up
for negotiation. Our goal is to open a dialogue with you to ultimately
assist in improving the implementation security of your library.

Links

Comment 1

[Daniel Veditz [:dveditz]]

Attachment: gdb.txt

Comment 2

[J.C. Jones [:jcj] (he/they)]

Thank you and your whole research group for this collection of reports. This comment is being added to each.

I’m assigning each of these to a team member for more detailed analysis beyond our initial triage. As all of these affect RHEL, I am also requesting impact analysis from our RedHat peers as well, as even if a side-channel is local, that might indicate higher-severity for server software than for Firefox. The analysis will also determine how to allocate CVE numbers, as needed.

I would like to advise that the embargo period your team has initially indicated is quite short, and in this case conflicts with both RedHat and Firefox release cycles. For a bug being actively exploited, we can do what is necessary; in the general case, our Release Calendar [0] indicates that to meet a 18 May disclosure date, we would need to make and uplift a point release of NSS from Nightly to Firefox Beta 76 within 8 days, which, given the complexity involved, seems unrealistic - and moreso for all four bugs at once. We’ll have a better idea as we complete our analysis of each bug.

Again, thank you for bringing these issues to our attention. Research such as yours makes the whole world safer!

[0] https://wiki.mozilla.org/Release_Management/Calendar

Comment 3

[Billy Brumley]

Hi J.C.,

Thanks for acknowledging the reports.

OK -- I see your release schedule now. Yea there is no way these fixes are ready in 8 days. So right out of the gate, we can align disclosure with the next cycle -- 02 June if I read that correctly. Even that is perhaps ambitious so let's keep it open as the discussion progresses.

Regarding complexity of the fixes, just eyeballing things from experience.

#1631576 is an easy fix. A couple of lines. Also mostly independent of what else is going on here.

While #1631573 is also technically a small fix, ideally the fix we have in mind for #1631583 makes it unnecessary. But the latter fix is going to take some time.

This one (#1631597) probably takes the most time because of all the low level details to review for a proper fix.

My team will also have to invest some effort in getting familiar with the NSS testing framework. We want to ensure we have all the proper unit tests in place before we start poking :)

Comment 4

[J.C. Jones [:jcj] (he/they)]

RSA key generation is used by Firefox for TLS and DTLS key agreements, for WebCrypto, and the Firefox Accounts Identity service. Generally this looks to be a sec-moderate for NSS servers / RedHat.

Ryan: Is RSA also actively used for Firefox Accounts? If so the impacts to Firefox Accounts are likely sec-high.

After some initial analysis of https://eprint.iacr.org/2019/266.pdf and OpenSSL’s commits to solve their similar issue, I do want to ask the research team whether they would indeed help us assemble a patch, understanding that this is going to take some time. Mozilla would be happy to collaborate on any aspect of the patch, including taking on some of the test burden.

Thank you again for this finding!

Comment 5

[Ryan Kelly [:rfkelly]]

Ryan: Is RSA also actively used for Firefox Accounts? If so the impacts to Firefox Accounts are likely sec-high.

There is code in the FxA client implementation in Desktop for generating RSA keys (e.g. in GenerateKeyPair services/crypto/component/IdentityCryptoService.cpp) but I don't believe that code is actually used in practice. All the callers I can find ask for DSA keys rather than RSA keys (and hence hit Bug 1631576 rather than this one).

We also use RSA keys on the server side in FxA, but they're generated via OpenSSL (via the nodejs crypto builtins) which IIUC from comments above, has already had a fix for a similar issue.

So I think that luckily, FxA is not affected by this one in practice.

Comment 6

[Billy Brumley]

After some initial analysis of https://eprint.iacr.org/2019/266.pdf and OpenSSL’s commits to solve their similar issue, I do want to ask the research team whether they would indeed help us assemble a patch, understanding that this is going to take some time. Mozilla would be happy to collaborate on any aspect of the patch, including taking on some of the test burden.

Sure -- I reckon it'll take us about a week. And we should be able to get started next Wednesday. I'll report back when we have something concrete to show :)

Comment 7

[Billy Brumley]

Update: Sohaib (who reported unrelated #1631583) started digging into this RSA issue yesterday. Cesar (who reported unrelated #1631576) will assist since he did the GCD work for OpenSSL so is familiar with the pitfalls.

Maybe a little over a week and we should have something working.

Comment 8

[Billy Brumley]

Hi JC,

Thank you again for this finding!

Could you please add Sohaib ul Hassan <[e-mail]> (reported of unrelated #1631583) to this thread? He's making good progress on the fix so I reckon he'll have something to show soon.

Also, where are you at with the severity and CVE assignment?

Thanks,

BBB

Comment 9

[Julien Cristau [:jcristau]]

cc:ed

Comment 10

[J.C. Jones [:jcj] (he/they)]

(In reply to bbrumley from comment #8)

Also, where are you at with the severity and CVE assignment?

For this one, as far as I can tell severity stands as sec-high for NSS and sec-moderate for Firefox.

We had run dangerously low on CVE #s, but Dan tells me we're replenished now so I'm needinfoing him on these bugs to get #s assigned.

Comment 11

[Daniel Veditz [:dveditz]]

assigned CVE-2020-12402

Comment 12

[J.C. Jones [:jcj] (he/they)]

Not going to make the next uplift.

Comment 13

[Kevin Jacobs [:kjacobs]]

Hello Tampere University team,

The fix for Bug 1631576 will go into release Firefox 77 and ESR 68.9 at the beginning of June, starting with Beta 77 next week to meet our testing requirements.

For the other bugs, the time window for that release is closing rapidly, with uplifts approved for next Tuesday the 19th. If we aren’t able to review and test by then, we may need to slip another 4-week release cycle to target Beta 78 for ultimate release on 2020-06-30. Can we agree to re-target disclosure of Bug 1631597, Bug 1631583, Bug 1623116, and Bug 1631573 to 30 June?

Thank you again for your engagement on all of these security bugs!

Comment 14

[Billy Brumley]

Can we agree to re-target disclosure of Bug 1631597, Bug 1631583, Bug 1623116, and Bug 1631573 to 30 June?

Yea let's do that. Ofc the delays are all from my team's side. But it's not for lack of effort -- we're making progress and we'll have code to show soonish :)

Thank you again for your engagement on all of these security bugs!

Glad we can help -- thanks for the fruitful discourse :)

Comment 15

[J.C. Jones [:jcj] (he/they)]

Retargeting to 30 June per https://bugzilla.mozilla.org/show_bug.cgi?id=1631597#c14 - thanks bbrumley and team!

Comment 16

[Billy Brumley]

Update: We are almost there on this one. We should be able to deliver a patch this week. The testing of mp_gcd and mp_invmod are not super extensive internally in NSS, so we're augmenting with external tests and a custom test harness.

Sohaib wrote the code and we were able to do some optimization from SOTA CT inv and gcd (Bernstein and Yang, TCHES 2019 which is too focused on fixed or special moduli for NSS purposes). I reviewed it and it looks clean. I'm having another team member (Cesar, who wrote the CT GCD for OpenSSL) review it independently now.

If NSS developers want to help, our changes are really only to these high level functions. But there is a leak in s_mp_div_2d that you could solve. (We know because we went down this path with OpenSSL for this PR and this PR. It's L3084 in mpi.c: if (d) {. That branch needs to be straightlined.

Comment 17

[Sohaib]

Attachment: Constant-time GCD and modular inversion implementation to fix the RSA key generation vulnerability

Hello Mozilla team,

We have come up with a patch to fix the RSA key generation vulnerability, that should replace the old implementation of mp_gcd and s_mp_invmod_odd_m with a constant time version. The implementation is based on the work by Bernstein and Yang (https://eprint.iacr.org/2019/266) "Fast constant-time gcd computation and modular inversion". We validated the functionality at our end with extensive testing using a custom test harness. Also thanks to bbrumley and Cesar for their valuable suggestions. Please find the attached patch and let us know if you need more information.
-sohaib

Comment 18

[Kevin Jacobs [:kjacobs]]

Excellent, thank you! Initial performance results look good (roughly equivalent on MacOS x64), and we'll carve out time next week for review and testing.

Comment 19

[Billy Brumley]

Attachment: bbb_shifting.patch

On top of Sohaib's patch (that is great!), here are a few fixes for some of the important underlying MP operations we're using. Summary:

1. s_mp_clamp has a bug in master, where if you right shift a negative number and end up with zero, the library doesn't adjust the sign accordingly so e.g. it's not picked up by mp_cmp_z etc

2. Did some straightlining in s_mp_mul_2d. The code will still leak d (upto word size) if it's over a word size, in multiples of the word size.

3. Ditto for s_mp_div_2d.

4. Did some straightlining in mpl_significant_bits -- that's particularly not CT friendly and used in many, many places across the library so tsk tsk.

You're lacking sufficient unit testing for a lot of the mpi.c functionality. So like with Sohaib's patch, we augmented with an external test harness. Some derived from OpenSSL. Some homebrew.

In general, you're leaking MP_USED all over the place in mpi.c. That's something you'll have to decide how to handle on your own. OpenSSL does it with the fixed_top concept, but I won't comment more on that.

Here is a great paper If you want to read more about the conflict between CT vs "fully flexible multi precision integer" library.

For my own patch (and I think Sohaib's), I didn't hit it with asan ubsan etc so that would be useful testing NSS can do.

If you have questions or comments, please don't hesitate.

Comment 20

[Sohaib]

following up on bbrumley's last message, we have now enabled the --asan option for NSS build, along with asan flag for our test harness (for both patches), we did not find any problems during the tests !

Comment 21

[Sohaib]

(In reply to Sohaib from comment #17)

Created attachment 9151051 [details] [diff] [review]
Constant-time GCD and modular inversion implementation to fix the RSA key generation vulnerability

Hello Mozilla team,

We have come up with a patch to fix the RSA key generation vulnerability, that should replace the old implementation of mp_gcd and s_mp_invmod_odd_m with a constant time version. The implementation is based on the work by Bernstein and Yang (https://eprint.iacr.org/2019/266) "Fast constant-time gcd computation and modular inversion". We validated the functionality at our end with extensive testing using a custom test harness. Also thanks to bbrumley and Cesar for their valuable suggestions. Please find the attached patch and let us know if you need more information.
-sohaib

For clarification, the patch also fixed a bug inside s_mp_invmod_2d (found in master) .

s_mp_invmod_2d computes the inversion for the even factor of the modulus (for an even modulus). For a small factor , s_mp_invmod_2d calls s_mp_invmod_radix to compute the inversion. On returning from s_mp_invmod_radix, sign of the input mp_int a is not propagated to the inversion result mp_digit i. For instance, a negative input will produce an incorrect result.

Comment 22

[Kevin Jacobs [:kjacobs]]

Thanks. We are in the process of reviewing both patches and should have feedback by the end of next week.

I did notice one issue in the patched s_mp_mul_2d: pa is set to MP_DIGITS(mp) + dshift, then incremented MP_USED(mp) times, potentially shifting out past the end of the buffer. I think this should be for (i = MP_USED(mp) - dshift; i > 0; i--) instead.

Bob, setting a NI to make sure that RedHat sees these and has a chance to test/review independently.

Comment 23

[Sohaib]

(In reply to Kevin Jacobs [:kjacobs] from comment #22)

Thanks. We are in the process of reviewing both patches and should have feedback by the end of next week.

I did notice one issue in the patched s_mp_mul_2d: pa is set to MP_DIGITS(mp) + dshift, then incremented MP_USED(mp) times, potentially shifting out past the end of the buffer. I think this should be for (i = MP_USED(mp) - dshift; i > 0; i--) instead.

Yea we agree MP_USED(mp) - dshift should be the loop bound. Thanks for pointing out !

Comment 24

[Robert Relyea]

Comment on attachment 9151646 [details] [diff] [review]
bbb_shifting.patch

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

In the mpi file, we are working really hard to make sure we don't shift by a full MP_DIGIT_BIT (which would always shift the entire mp_digit out). I'm presuming this is to avoid potential compiler/processor issues on some platforms?

I don't think the mp_significant_bits in mpi/mplogic.c function is quite right.

::: lib/freebl/mpi/mpi.c
@@ +2990,5 @@
>  
>      dshift = d / MP_DIGIT_BIT;
> +    d %= MP_DIGIT_BIT;
> +    rshift = MP_DIGIT_BIT - d;
> +    rshift %= MP_DIGIT_BIT;

probably a comment here, that if d is zero, we want rshift to also be zero to explain the modulo operation here.

::: lib/freebl/mpi/mplogic.c
@@ +440,5 @@
> +        LZCNTLOOP(8);
> +        LZCNTLOOP(4);
> +        LZCNTLOOP(2);
> +        LZCNTLOOP(1);
> +        break;

The old function and the new one are not equivalent. You can check by looking at a couple of cases: MP_USED(a) = 1, MP_DIGIT(a,0) = 1, the previous code returns 1 and this code returns 2.

I think you can fix this by changing the addition in your loop to an |, but then I still think there's an error in the case where we have an even case. I suspect we can't initialize bits to 1 and keep the old semantics of the previous function, (which means we probably need to restore the (!bits)->bits = 1, or a constant time equivalent.

Comment 25

[Billy Brumley]

Attachment: bbb_fixup1.patch

Hey Kevin, Bob,

Thanks both for the feedback. I've attached a fixup commit that addresses the points you raised.

I did notice one issue ... potentially shifting out past the end of the buffer

@Kevin Very nice catch -- thank you! Fixed per your suggestion.

probably a comment here, that if d is zero, we want rshift to also be zero to explain the modulo operation here.

@Bob yea in the end it's to avoid undefined behavior. I've added comments accordingly.

The old function and the new one are not equivalent

@Bob I've added comments to clarify this. Your can check your inputs in the debugger (I did), and you'll see it correctly returns 1 for the a=1 case.

In fact that value was already a part of our external unit test harness we developed. Here is a preview:

test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST 0 | grep ^1#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -0 | grep ^1#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -1 | grep ^1#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST DEADBEEF | grep ^32#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST 1DEADBEEF | grep ^33#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST BAAAAAADDEADBEEF | grep ^64#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST 1BAAAAAADDEADBEEF | grep ^65#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -DEADBEEF | grep ^32#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -1DEADBEEF | grep ^33#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -BAAAAAADDEADBEEF | grep ^64#"
test_expect_success "NSS mpl_significant_bits corner case test" "$NSS_BLTEST -1BAAAAAADDEADBEEF | grep ^65#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 1 | grep ^1#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 3 | grep ^2#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 6 | grep ^3#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 9 | grep ^4#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 17 | grep ^5#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 2B | grep ^6#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 49 | grep ^7#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST A0 | grep ^8#"
test_expect_success "NSS mpl_significant_bits test" "$NSS_BLTEST 1B1 | grep ^9#"

Followed by several thousand more test cases.

The reason is the macro is basically doing a binary search for the position of the top set bit, so len(a) is one more than that.

Comment 26

[Robert Relyea]

OK, thanks. It would be good to include those unit tests in the patch. There are two ways of doing this: either adding tests to the gtests (if you know google gtest harness), or adding a test file to nss/cmd/{test_cmd_name}/{test_cmd_name}.c and then adding a call to the all.sh scripts (probably just adding a new test to tests/bltest/bltest.sh)

Comment 27

[Kevin Jacobs [:kjacobs]]

Okay, I've only found one other issue specific to 32-bit builds:

../../lib/freebl/mpi/mplogic.c:436:13: error: right shift count >= width of type [-Werror]
             LZCNTLOOP(32); 

where we just need to cast d.

Would it be possible to submit these patches (one per-author, or one single) to Phabricator? That will come in handy for future patches, too :).

I believe JC plans to add his r+ and a request for landing/uplift approval next.

Thanks again!

Comment 28

[J.C. Jones [:jcj] (he/they)]

I have reviewed all of the changes with scratchpads, my Python interpreter, and a trusty calculator. I haven't found any logic errors. I am somewhat worried that we'll find compile issues on some of the less common architectures (particularly big-endian), but we'll have to catch those as they report, since none are part of our officially supported platforms.

To get this into the next Release (and ESR 78), we'll need to produce an NSS point release, 3.53.1, by no later than Tuesday the 16th. Given the nature of the patches I'd like maximum testing possible. When I ask for sec-approval, if there's time and if RedHat agrees, I'm going to ask that we land into trunk (and Firefox Nightly) for 48 hours before making that 3.53.1 point release and going to Beta.

I don't think this warrants a backport to the NSS 3.44.x series (for ESR 68, et. al), but if RedHat requests it, the patches almost apply cleanly to NSS_3_44_BRANCH. So at this time I intend to only ask for sec-approval to Firefox 78 in Beta.

I would like to consolidate bbrumley's two patches when we go for the actual r+ and for landing. If we could do that via Phabricator as Kevin says in Comment 27, that would be nice since it supports all our tooling that avoids getting authorship messed up, etc. (The moz-phab tool is easy to install, see https://moz-conduit.readthedocs.io/en/latest/mozphab-linux.html ). If not, not a big deal, we can do that for the next bug down the road.

I also agree with Bob that it would be very nice to get some of your test harness as well, if that's possible - we could even take on converting it from the scripted version implied in your comment to a the mpi_unittest.cc . I don't think we have to hold up landing this patch on those tests though, we can do them in a follow-on IMO.

At any rate, I think the TODOs outstanding are:

• Research team to address the 32-bit compile issue Kevin found
• Research team to consider whether they could add to our gtest, or maybe help us along by giving us their test cases that we could do the same
• Research team to consolidate at least the fixup patches together, optionally move to Phabricator
• Bob and Kevin to add their r+'s directly to the consolidated patches
• J.C. to file for NSS 3.53.1 point release and alert releng
• J.C. to request sec-approval as soon as we have those r+s.

I can't thank you enough, Bob, Sohaib, and your whole team. This is great work.

Comment 29

[Sohaib]

Attachment: Bug 1631597 - Constant-time GCD and modular inversion r=rrelyea,kjacobs

The implementation is based on the work by Bernstein and Yang
(https://eprint.iacr.org/2019/266) "Fast constant-time gcd computation
and modular inversion". It fixes the old mp_gcd and s_mp_invmod_odd_m
functions.

The patch also fix mpl_significant_bits s_mp_div_2d and s_mp_mul_2d
by having less control flow to reduce side-channel leaks.

Notes:

1. Fix a bug in s_mp_invmod_2d, which computes the inversion for the even
   factor of the (even) modulus. For a small factor, s_mp_invmod_2d calls
   s_mp_invmod_radix to compute the inversion. On returning from
   s_mp_invmod_radix, sign of the input (mp_int a) is not propagated to
   the inversion result (mp_digit i). For instance, a negative input will
   produce an incorrect result.

2. s_mp_clamp has a bug in master, where if you right shift a negative
   number and end up with zero, the library doesn't adjust the sign accordingly
   so e.g. it's not picked up by mp_cmp_z etc.

3. Some straightlining in s_mp_mul_2d. The code will still leak d (upto word size)
   if it's over a word size, in multiples of the word size.

4. Ditto for s_mp_div_2d.

5. Some straightlining in mpl_significant_bits.

Co Author : Billy Bob Brumley

Comment 30

[Sohaib]

(In reply to J.C. Jones [:jcj] (he/him) [increased latency due to COVID-19] from comment #28)

At any rate, I think the TODOs outstanding are:

• Research team to address the 32-bit compile issue Kevin found

This has now been fixed in the latest patch. The macro MP_USE_UINT_DIGIT guards with conditional compilation.

• Research team to consolidate at least the fixup patches together, optionally move to Phabricator

We have now submitted both patches as a single review in Phabricator

I can't thank you enough, Bob, Sohaib, and your whole team. This is great work.

We are happy to help.

Comment 31

[J.C. Jones [:jcj] (he/they)]

Bob, since we're looking to land this Tuesday, can you complete a review this week? (Not sure if you saw the pings in Phabricator)

Comment 32

[Robert Relyea]

I'll have to do it monday I'm off in about an hour for the weekend.

Comment 33

[J.C. Jones [:jcj] (he/they)]

Any updates, Bob?

Comment 34

[Robert Relyea]

ready to land.

Comment 35

[J.C. Jones [:jcj] (he/they)]

NSS try push for NSS_3_53_BRANCH: https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=7499d10c19d922bd3f66cf10d34e15e8376e3604
Mozilla-Central try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=21cdf9f087c8fd748c6075ee7dc9565a98aaa00f

Comment 36

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 9154911 [details]
Bug 1631597 - Constant-time GCD and modular inversion r=rrelyea,kjacobs

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Medium difficultly
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?:
• How likely is this patch to cause regressions; how much testing does it need?: The regressions are likely to be obvious connection failures due to cipher mismatches. We do have significant automated interop testing however, so this is unlikely.

Comment 37

[Daniel Veditz [:dveditz]]

Comment on attachment 9154911 [details]
Bug 1631597 - Constant-time GCD and modular inversion r=rrelyea,kjacobs

sec-approval+ We should definitely take this on ESR and forestall all the arguments about whether this is or isn't "bad enough". In the right circumstances it is (sec-high) but that's not common for most browser users (thus sec-moderate). The ESR audience is more likely to run into those cases.

Comment 38

[J.C. Jones [:jcj] (he/they)]

pragma fixup for Windows:
https://hg.mozilla.org/projects/nss-try/rev/e698f26bbc616faa5fc9529e652bdaa40a83e7ef
Try push:
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=a26ff47dd442

Comment 39

[J.C. Jones [:jcj] (he/they)]

NSS_3_53_BRANCH:
https://hg.mozilla.org/projects/nss/rev/c5c89b18053aad6147f82abecc568653b78095b4
https://hg.mozilla.org/projects/nss/rev/fca7a9ba4da2735a3d844aac4411cd5074d456f7
https://hg.mozilla.org/projects/nss/rev/688d2a7257586ba8ca7febe46e6ae43c4c1fe04e

Comment 40

[J.C. Jones [:jcj] (he/they)]

https://hg.mozilla.org/projects/nss/rev/699541a7793bbe9b20f1d73dc49e25c6054aa4c1

Comment 41

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 42

[Tom Ritter [:tjr]]

Attachment: advisory.txt

I've attached an advisory; credited to 'Billy Brumley, et al' - please let me know if this credit is appropriate or what you would like.

Comment 43

[Billy Brumley]

Hey Tom,

I've attached an advisory; credited to 'Billy Brumley, et al' - please let me know if this credit is appropriate or what you would like.

The technical description is accurate :)

For the credit line, please use this one:

Sohaib ul Hassan, Iaroslav Gridin, Ignacio M. Delgado-Lozano, Cesar Pereida García, Jesús-Javier Chi-Domínguez, Alejandro Cabrera Aldaya, and Billy Bob Brumley, Network and Information Security (NISEC) Group, Tampere University, Finland

Comment 44

[Tom Ritter [:tjr]]

Attachment: advisory.txt

Comment 45

[Daniel Veditz [:dveditz]]

If this is a "sec-high" for RedHat (presumably server products?), won't they also need it fixed on the ESR-68 branch?

Comment 46

[J.C. Jones [:jcj] (he/they)]

I am unaware of such a need from the earlier discussions. Bob, do you need me to cut a 3.44 point release?

Comment 47

[Robert Relyea]

No we're moving to 3.53.1 on all our critical platforms.