Closed Bug 1293334 (CVE-2016-9074) Opened 8 years ago Closed 8 years ago

Replace unreliable divSpoiler (timing side-channel defense)

Categories

(NSS :: Libraries, defect)

defect
Not set
normal

Tracking

(firefox48 wontfix, firefox49+ wontfix, firefox-esr4550+ fixed, firefox50+ fixed, firefox51+ fixed)

RESOLVED FIXED
Tracking Status
firefox48 --- wontfix
firefox49 + wontfix
firefox-esr45 50+ fixed
firefox50 + fixed
firefox51 + fixed

People

(Reporter: franziskus, Assigned: franziskus)

Details

(Keywords: csectype-other, sec-moderate, Whiteboard: [post-critsmash-triage][adv-main50+][adv-esr45.5+])

Attachments

(6 files, 3 obsolete files)

As agl notes in [0] the divSpoiler hack to avoid CPU optimisation and thus timing side channels isn't working. We have the same code [1] that we should replace with something similar (Barrett reduction sounds like a good choice).

[0] https://github.com/openssl/openssl/pull/1027
[1] http://searchfox.org/nss/source/lib/ssl/ssl3con.c#13102
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #0)
> [1] http://searchfox.org/nss/source/lib/ssl/ssl3con.c#13102

As a general suggestion, I recommend to mention affected function names, as links with line numbers, that don't refer to a specific revision, will get outdated very quickly.
Attached patch divSpoil.patch (obsolete) — Splinter Review
this is basically the solution from open/boringssl. I'm sure agl doesn't mind, but we can ask him.
Assignee: nobody → franziskuskiefer
Attachment #8779342 - Flags: review?(ekr)
Franziskus, can you ask Watson?
Attachment #8779342 - Flags: review?(wladd)
Comment on attachment 8779342 [details] [diff] [review]
divSpoil.patch

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

Looks good: I've verified that the ranges are sufficient for removing the padding lengths we need to deal with, and that they are right.
Attachment #8779342 - Flags: review?(wladd) → review+
What should the security rating on this be?
https://wiki.mozilla.org/Security_Severity_Ratings
Flags: needinfo?(franziskuskiefer)
Considering that the original bug 822365 that was thought to fix this issue is sec-high, this one should be sec-high as well.
Flags: needinfo?(franziskuskiefer)
Comment on attachment 8779342 [details] [diff] [review]
divSpoil.patch

Richard can you have a look at this?
Attachment #8779342 - Flags: review?(ekr) → review?(rlb)
Keywords: sec-high
Comment on attachment 8779342 [details] [diff] [review]
divSpoil.patch

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

r=me with the documentation comments below addressed.

::: lib/ssl/ssl3con.c
@@ +13102,5 @@
> +
> +    /* The time to compute (macStart - scanStart) % macSize varies based on
> +     * the amount of padding. Thus we explicitely handle all mac sizes with
> +     * (hopefully) constant time modulo.
> +     */

It would be helpful to note the general formula here, e.g.:

/*
 * rotateOffset must be smaller than 1/e, where e is the error in
 * the approximation of 1/macSize by a fraction whose denominator
 * is a power of two.
 */

How do we know that this is the case?  It doesn't seem to follow from the inequalities at the top of the method.  It looks like Watson agrees that we're OK here, but we should describe that here so that people don't have to puzzle over it in the future.

@@ +13148,5 @@
>      /* Now rotate the MAC. If we knew that the MAC fit into a CPU cache line
>       * we could line-align |rotatedMac| and rotate in place. */
>      memset(out, 0, macSize);
> +    rotateOffset = macSize - rotateOffset;
> +    rotateOffset &= ~ssl_ConstantTimeGE(rotateOffset, macSize);

This notation is pretty opaque.  It's equivalent to an ssl_constantTimeSelect, right?  If so, that would be clearer.
Attachment #8779342 - Flags: review?(rlb) → review+
Attachment #8779342 - Attachment is obsolete: true
Attachment #8783517 - Flags: review+
Which Firefox branches need this fix?
This bug fixes what ought to be fixed in bug 822365. So the exploit is known for about 4 years already. Though the code changed considerably as a result of bug 822365 such that it's not immediately clear to me if/how the original attack still works. It'll land with 3.27 and and such be in Firefox 51.
Dan, do we want to backport the fix to other Firefox versions? I think an uplift to 50 would be reasonable. Not sure about 49 and ESR.
Flags: needinfo?(dveditz)
This looks like a small part of the fix in bug 822365. Does this flawed timing attack defense (which seemed to work OK at the time) really defeat the whole of the Lucky13 defense? I'm guessing sec-moderate is more appropriate, but if this does resurrect Lucky13 then we should go back to sec-high.

If it's sec-high we should definitely push for Firefox 49 (NSS 3.25?) -- we still have a week or two. If it's sec-moderate and we're still mostly safe from lucky13 then 50 is probably OK. Either way we'll want this on the ESR release that matches when it ships.
Flags: needinfo?(dveditz)
Summary: Replace insecure divSpoiler → Replace unreliable divSpoiler (timing side-channel defense)
Comment on attachment 8783517 [details] [diff] [review]
divSpoil.patch (OK for 3.26 branch)

sec-approval=dveditz
Attachment #8783517 - Flags: sec-approval+
Track for 49+ to monitor if it will become sec-high.
I think sec-moderate is fine, but let's wait for opinions from Wan-Teh and Bob who oversaw the original lucky13 fix.
Flags: needinfo?(wtc)
Flags: needinfo?(rrelyea)
I agree sec-moderate is appropriate.
Flags: needinfo?(wtc)
I think waiting for 50 will be best here. We only have a week to go till release and have a lot of other uncertainties still in play.
https://hg.mozilla.org/projects/nss/rev/1e202f0a01b9
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(rrelyea)
Resolution: --- → FIXED
Target Milestone: --- → 3.27
Based on comments from Dan and Liz, it seems the request is to fix this 
for Firefox 50, and ESR 45.5, scheduled for 2016-11-08

Firefox 50 will require a backport of the fix to NSS_3_26_BRANCH and a NSS 3.26.1 release.

ESR 45.5 will require a backport of the fix to NSS_3_21_BRANCH and a NSS 3.21.2 release.

Franziskus, is the patch compatible with those older branches (in other words, your patch doesn't depend on anything else that has been added after the release of NSS 3.21.1) ?
Flags: needinfo?(franziskuskiefer)
Regarding branch landing timing, I suggest that we land the updated NSS branch releases into the mozilla-beta and mozilla-esr45 branches, shortly after 2016-09-13.

Dan, Liz, does that work for you? Will be give branch commit approvals by mid september?
Flags: needinfo?(lhenry)
Flags: needinfo?(dveditz)
The patch is self-contained. I don't think there's a problem with applying this to 3.26 and 3.21.
Flags: needinfo?(franziskuskiefer)
We aren't 100% sure of the release date right now. Probably 2016-09-13, but we have a little uncertainty right now that we may need to slip the date.  Landing after we release should be ok. Dan, what do you think?

Ritu: fyi, here's the NSS update plans for 50 while it is in beta.
Flags: needinfo?(lhenry) → needinfo?(rkothari)
sounds good.
Flags: needinfo?(dveditz)
Merge attempt for NSS 3.21 branch.

I was testing on Fedora 24 with gcc 6.1.1, and I ran into a warning-as-error, misleading indentation.

I suggest to disable that error on the old branch, which doesn't have our code reformatting yet.
Attachment #8788866 - Flags: review?(franziskuskiefer)
Comment on attachment 8783517 [details] [diff] [review]
divSpoil.patch (OK for 3.26 branch)

Your original patch applies fine and builds without errors on NSS_3_26_BRANCH
Attachment #8783517 - Attachment description: divSpoil.patch → divSpoil.patch (OK for 3.26 branch)
Regarding the announcement of the NSS 3.21.2 and NSS 3.26.1 releases:

When are we allowed to send the public announcement message for those NSS security fix releases?

I'd like to land the code into Firefox beta and ESR 45 as soon as possible (soon after the mit-september releases are done), and this will require me to make the new NSS releases publicly available on FTP/S3 (which means people will find them).

Should the NSS announcement be sent immediately, with a vague text - or -
should the NSS version annoucement be delayed until some later date?
(If yes, please suggest the date.)
Will we require separate branch approval+ for landing the NSS update into mozilla-beta and mozilla-esr45, or are the existing tracking+ flags in this bugs sufficient for branch landing?
Comment on attachment 8788866 [details] [diff] [review]
backport to NSS 3.21, v1

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

I'm not sure what exactly is misleading here. But making this warning non-fatal for the branch makes sense.
Attachment #8788866 - Flags: review?(franziskuskiefer) → review+
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #29)
> I'm not sure what exactly is misleading here.

For the lines that trigger the warning we have 

  if ()
    action();

However, those two lines start with a different mixed amount of tabs and spaces. The compiler apparently wants to warn that the indenting might mislead a human reader.
(In reply to Kai Engert (:kaie) from comment #27)
> I'd like to land the code into Firefox beta and ESR 45 as soon as possible
> (soon after the mit-september releases are done), and this will require me
> to make the new NSS releases publicly available on FTP/S3 (which means
> people will find them).

Are you landing "final" versions on the "beta" branch, and thus triggering what looks like an NSS "release"? Why so early in the cycle? If we need testing could we land these as "beta" versions of NSS and then change the definition (header value) to final later in the cycle?

How has this not come up before? We've fixed NSS bugs in the past and not released advisories for them until that version of Firefox was released.

> Should the NSS announcement be sent immediately, with a vague text - or -

What else is being fixed in these versions?

(In reply to Kai Engert (:kaie) from comment #28)
> Will we require separate branch approval+ for landing the NSS update into
> mozilla-beta and mozilla-esr45, or are the existing tracking+ flags in this
> bugs sufficient for branch landing?

The tracking flags mean tracking, nothing more. When a patch for a tracked bug is produced then the approval process is there to give release managers a chance to make sure the actual patch isn't too complicated/risky/untested/late for a given milestone.
(In reply to Daniel Veditz [:dveditz] from comment #31)
> 
> Are you landing "final" versions on the "beta" branch, and thus triggering
> what looks like an NSS "release"?

Correct.

> Why so early in the cycle?

I just assumed you want all changes to mozilla-beta happening as early as possible.


> If we need
> testing could we land these as "beta" versions of NSS and then change the
> definition (header value) to final later in the cycle?

We could. What's the deadline for landing the final header version changes (and configure update to require the new version number) into the mozilla-beta and mozilla-esr45 branches?

We should start by testing with mozilla-central immediately, by uplifting a new NSS 3.27 beta to m-i today, I'll work to get this done. (It might have been a good idea to do this immediately after this fix landed into NSS trunk.)


> How has this not come up before? We've fixed NSS bugs in the past and not
> released advisories for them until that version of Firefox was released.

Not sure I understand the question. Yes, situations like that had been come up before. I made my comment to ensure everyone is aware of these needs for timing the announcements.

> 
> > Should the NSS announcement be sent immediately, with a vague text - or -
> 
> What else is being fixed in these versions?

Nothing else. The proposal is to produce security NSS releases, which are based on the currently used NSS branch releases used by the respective Firefox branches, plus this fix.

(These days we have one NSS branch for each Firefox branch.)


> The tracking flags mean tracking, nothing more.

Thanks for the clarification. We'll make attachments for the approval process.
Regarding Dan's testing question:

- the fix from this bug hasn't produced any issues with the NSS own test suite

- a try build with this fix has now been started here:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=0a08925056fc

  (if try build succeeds, I'll uplift into mozilla-inbound by tomorrow)
It's too early to land this in mozilla-beta since the release date for 49 is now Sept. 20th. We should do this after we ship 49. Kai, I should have let you know, sorry about that.
No problem, thanks for the update.

I'll work on it later next week.
Group: crypto-core-security → core-security-release
Comment on attachment 8788866 [details] [diff] [review]
backport to NSS 3.21, v1

Franziskus, the patch for NSS 3.21.x doesn't work with older gcc versions, apparently not on OSX. We need to limit -Wno-error=misleading-indentation to compiler versions that support it.
Based on other compiler version checks in the same file, I think this should work?
Attachment #8794281 - Flags: review?(franziskuskiefer)
Comment on attachment 8794281 [details] [diff] [review]
incremental for nss 3.21, restrict parameter -Wno-error=misleading-indentation to gcc >= 6

>diff --git a/coreconf/Werror.mk b/coreconf/Werror.mk
>+    CC_VERSION := $(subst ., ,$(shell $(CC) -dumpversion))
>+    ifeq (,$(filter 0 1 2 3 4 5,$(word 1,$(CC_VERSION))))
>+      WARNING_CFLAGS += -Werror -Wno-error=misleading-indentation
>+    endif

We should also add a comment that explains the intention:

# Only add -Wno-error=misleading-indentation for gcc 6 and newer
Comment on attachment 8794281 [details] [diff] [review]
incremental for nss 3.21, restrict parameter -Wno-error=misleading-indentation to gcc >= 6

sorry, let me redo this patch
Attachment #8794281 - Flags: review?(franziskuskiefer) → review-
Comment on attachment 8794281 [details] [diff] [review]
incremental for nss 3.21, restrict parameter -Wno-error=misleading-indentation to gcc >= 6

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

::: coreconf/Werror.mk
@@ +61,5 @@
>  
>    ifeq ($(NSS_ENABLE_WERROR),1)
> +    CC_VERSION := $(subst ., ,$(shell $(CC) -dumpversion))
> +    ifeq (,$(filter 0 1 2 3 4 5,$(word 1,$(CC_VERSION))))
> +      WARNING_CFLAGS += -Werror -Wno-error=misleading-indentation

This patch is wrong. -Werror is fine for all versions of GCC and clang.

I suggest we simply remove -Wno-error=misleading-indentation. If an
NSS 3.21.x package needs to be built with the latest version of GCC,
the NSS 3.21.x package spec file should add -Wno-error=misleading-indentation.

We should not add such a change to an NSS branch without adding it to
the NSS trunk first.
(In reply to Wan-Teh Chang from comment #41)
> 
> This patch is wrong. -Werror is fine for all versions of GCC and clang.

That's why I made the second version of the patch, which keeps -Werror in general.


> I suggest we simply remove -Wno-error=misleading-indentation. If an
> NSS 3.21.x package needs to be built with the latest version of GCC,
> the NSS 3.21.x package spec file should add
> -Wno-error=misleading-indentation.
> 
> We should not add such a change to an NSS branch without adding it to
> the NSS trunk first.

I don't intend to add this parameter to the NSS trunk. We don't need it there. We have reformatted our code on trunk, that prevents the error, and doesn't require the parameter.

This parameter is specifically suggested for the old NSS branch, where we shouldn't mess with cleaning up the bad whitespace.

Adding the patch to NSS directly is preferred, because this isn't just about creating NSS packages, but our continous integration includes systems that use gcc 6.
(In reply to Kai Engert (:kaie) from comment #42)
> 
> Adding the patch to NSS directly is preferred, because this isn't just about
> creating NSS packages, but our continous integration includes systems that
> use gcc 6.

... and I'd like to build the NSS 3.21 release candidate on our continous integration infrastructure, just for completeness, to ensure all tests still pass on all platforms with this fix on that branch.
GCC documentation says:

    In the case of mixed tabs and spaces, the warning uses
    the -ftabstop= option to determine if the statements
    line up (defaulting to 8).

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

NSS uses a tabstop of 8. So if GCC cannot handle that, we
should submit a bug report to GCC.
Attachment #8794285 - Flags: review?(franziskuskiefer) → review+
Checked in to the NSS_3_26_BRANCH:
https://hg.mozilla.org/projects/nss/rev/d38536fcc726

NSS tests look good.

Checked in to the NSS_3_21_BRANCH:
https://hg.mozilla.org/projects/nss/rev/6883f1fc9129
https://hg.mozilla.org/projects/nss/rev/6ca604746182

NSS tests still running.

I'll shortly proceed with creating the NSS 3.21.2 and NSS 3.26.1 releases,

and will attach branch uplift patches for beta/esr approval by monday.
I compiled NSS_3_21_BRANCH with gcc 6.2.0. The misleading
indentations are not hard to fix. I think we should fix
the misleading indentations rather than disabling the
-Wmisleading-indentation compiler warning.
Attachment #8794349 - Flags: superreview?(kaie)
Attachment #8794349 - Flags: review?(franziskuskiefer)
I updated my patch because Kai's patch has been checked in.

I hope the benefits of compiling NSS_3_21_BRANCH with -Wmisleading-indentation
are clear.
Attachment #8794349 - Attachment is obsolete: true
Attachment #8794349 - Flags: superreview?(kaie)
Attachment #8794349 - Flags: review?(franziskuskiefer)
Attachment #8794355 - Flags: superreview?(kaie)
Attachment #8794355 - Flags: review?(franziskuskiefer)
Wan-Teh, I appreciate your intention to simplify, but let me say:
- I'm trying to minimize patches that backport security fixes as much as possible,
  and white space changes in particular can usually create confusion very easily,
  and distract from the real change
- I had not attempted to go the whitespace fixing approach in the first place,
  because my expectation was that a much larger number of places might have
  required fixing, so it's good that only very few places would need a fix.
- the benefit of the compiler warning seems negligible in this particular scenario,
  because we know the code is correct, and we're not developing new code on this branch.

My preference is to keep things simple, and because buildbot seems to currently build fine with the code as checked in, avoiding additional commits seems simpler to me, so in my opinion your patch isn't necessary.

But if Franziskus thinks differently and prefers the whitespace cleanup and gives you r+, I'll check it in, and will have to trigger another round of test builds afterwards.
I was mainly curious about the speculation that -Wmisleading-indentation
cannot handle mixed tabs and spaces correctly. I found that GCC handles
mixed tabs and spaces correctly in all the four files it warns about.
* The two files in lib/dbm/src/ use a tab stop of 4. The warnings could
  also be fixed by compiling those files with the -ftabstop=4 option.
* The other two files simply have incorrect indentations.
I'm glad I don't need to submit a bug report to GCC.

I also wanted to know if any of the incorrect indentations is a bug.
Fortunately none of them is. Without looking at the warnings, we cannot
be sure.

So, whether you accept my patch or not is secondary to me.
Comment on attachment 8794355 [details] [diff] [review]
incremental for nss 3.21, fix misleading indentation (patch v2)

Wan-Teh, thanks a lot for your thorough verifications!
I'll defer to Franziskus if we should check this in, or can skip it.
Attachment #8794355 - Flags: superreview?(kaie)
Comment on attachment 8794355 [details] [diff] [review]
incremental for nss 3.21, fix misleading indentation (patch v2)

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

Thanks a lot for the investigation Wan-Teh.
But since there's no bug I think we should leave the 3.21 branch alone.
Attachment #8794355 - Flags: review?(franziskuskiefer)
Firefox-50 beta branch currently uses NSS 3.26.
A version NSS 3.26.1 has been released, which contains this bugfix as the only change since NSS 3.26.

This patch uplifts to NSS 3.26.1, which includes this bugfix, plus version number changes, only.
Attachment #8795159 - Flags: approval-mozilla-beta?
Firefox-ESR-45 branch currently uses NSS 3.21.1
A version NSS 3.21.2 has been released, which contains this bugfix as the only change since NSS 3.21.1

This patch uplifts to NSS 3.21.2, which includes this bugfix, plus version number changes, only.
Attachment #8795164 - Flags: approval-mozilla-esr45?
Comment on attachment 8795159 [details] [diff] [review]
Uplift NSS 3.26.1 to the Firefox-Beta-50 branch

NSS update, Beta50+
Attachment #8795159 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8795164 [details] [diff] [review]
Uplift NSS 3.21.2 to the Firefox-ESR-45 branch

NSS update, ESR45.5+
Attachment #8795164 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+
Hi Dan, based on comment 12, it seems Fx51 will be on NSS 3.27. What should we do about status-51 "affected" on this bug?
Flags: needinfo?(dveditz)
(In reply to Ritu Kothari (:ritu) from comment #56)
> Hi Dan, based on comment 12, it seems Fx51 will be on NSS 3.27. What should
> we do about status-51 "affected" on this bug?

Firefox 51 is already using NSS 3.27, which already contains the fix for this bug.
I'm flipping the bit to fixed.
mozilla-beta tree is closed for "uplift week".
Can you please advise when I should check in?
Flags: needinfo?(dveditz)
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Hi,

I am trying to understand if this is really a security flaw? I understand that barrett reduction is much better, but is the div_spoiler implementation broken, ie it did not implement the fix correctly?

If not, that this is just security hardening and not a flaw imo
(In reply to Huzaifa Sidhpurwala from comment #61)
> Hi,
> 
> I am trying to understand if this is really a security flaw? I understand
> that barrett reduction is much better, but is the div_spoiler implementation
> broken, ie it did not implement the fix correctly?
> 
> If not, that this is just security hardening and not a flaw imo

Yes this is a security flaw. See links comment 1, i.e. division on Intel CPUs is not constant time, which provides the attacker with a padding oracle.
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main50+][adv-esr45.5+]
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #62)
> (In reply to Huzaifa Sidhpurwala from comment #61)
> > Hi,
> > 
> > I am trying to understand if this is really a security flaw? I understand
> > that barrett reduction is much better, but is the div_spoiler implementation
> > broken, ie it did not implement the fix correctly?
> > 
> > If not, that this is just security hardening and not a flaw imo
> 
> Yes this is a security flaw. See links comment 1, i.e. division on Intel
> CPUs is not constant time, which provides the attacker with a padding oracle.

I know, but the workaround to that is to use the divspoiler mechanism, which was used everywhere, barrett reduction is just the better way of doing it.
Alias: CVE-2016-9074
Note:

Just to clarify the above, i dont understand why a CVE id was assigned in the first place. barrett reduction is a much better way of doing constant-time operation, but div_spoiler is not vulnerable in the first place (not perfect but not vulnerable).

So even that the older method isnt really vulnerable, this is not really a security issue.
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #62)
> (In reply to Huzaifa Sidhpurwala from comment #61)
> > Hi,
> > 
> > I am trying to understand if this is really a security flaw? I understand
> > that barrett reduction is much better, but is the div_spoiler implementation
> > broken, ie it did not implement the fix correctly?
> > 
> > If not, that this is just security hardening and not a flaw imo
> 
> Yes this is a security flaw. See links comment 1, i.e. division on Intel
> CPUs is not constant time, which provides the attacker with a padding oracle.

No, this is not a security flaw. div_spoiler solves the problem, its not perfect but works, barrett reduction is just a better way of doing things.
Flags: needinfo?(abillings)
Huzaifa, I'd like to suggest, please clarify what action you expect.
(In reply to Kai Engert (:kaie) from comment #66)
> Huzaifa, I'd like to suggest, please clarify what action you expect.

Not sure if this makes any difference now. I want to suggest we reject this CVE id. This is not a security flaw. div_spoiler works well, barrett reduction is just a better method of handling things. This is an enhancement bug not a security flaw.
Huzaifa: I think you're missing the point. At the danger of repeating myself let me explain again what this bug is about.
The original code added the so called divSpoiler to |macStart - scanStart| in the hope that this would make CPUs compute |divq| operations in constant time. That's however not the case (at least not everywhere).
The new code, using barrett reduction, aims to bypass the problem of non-constant-time div operations on CPUs by not using it. Barrett instead only subtracts.
So this change has nothing to with enhancement but is fixing a security issue. Because observing timing differences in CPU instructions is relatively hard, this is sec-moderate.
(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #68)
> Huzaifa: I think you're missing the point. At the danger of repeating myself
> let me explain again what this bug is about.
> The original code added the so called divSpoiler to |macStart - scanStart|
> in the hope that this would make CPUs compute |divq| operations in constant
> time. That's however not the case (at least not everywhere).
> The new code, using barrett reduction, aims to bypass the problem of
> non-constant-time div operations on CPUs by not using it. Barrett instead
> only subtracts.
> So this change has nothing to with enhancement but is fixing a security
> issue. Because observing timing differences in CPU instructions is
> relatively hard, this is sec-moderate.

Ok that makes it clear, thanks!
Flags: needinfo?(abillings)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.