Discussion:
CVE-2018-5407: new side-channel vulnerability on SMT/Hyper-Threading architectures
(too old to reply)
Billy Brumley
2018-11-01 22:12:27 UTC
Permalink
Howdy Folks,

We recently discovered a new CPU microarchitecture attack vector. The
nature of the leakage is due to execution engine sharing on SMT (e.g.
Hyper-Threading) architectures. More specifically, we detect port
contention to construct a timing side channel to exfiltrate
information from processes running in parallel on the same physical
core. Report is below.

Thanks for reading!

BBB

# Report

We steal an OpenSSL (<= 1.1.0h) P-384 private key from a TLS server
using this new side-channel vector. It is a local attack in the sense
that the malicious process must be running on the same physical core
as the victim (an OpenSSL-powered TLS server in this case).

## Affected hardware

SMT/Hyper-Threading architectures (verified on Skylake and Kaby Lake)

## Affected software

OpenSSL <= 1.1.0h (but in general, software that has secret dependent
control flow at any granularity; this particular application is a
known vulnerability since 2009 only recently fixed)

Ubuntu 18.04 (again, it is really a hardware issue, but anyway this
distro is where we ran our experiments)

## Classification and rating

Tracked by CVE-2018-5407.

CWE wise, I would label it like

CWE-208: Information Exposure Through Timing Discrepancy

At a very high level (e.g. CVSS string), it is similar to this CVE:

https://nvd.nist.gov/vuln/detail/CVE-2005-0109

But the underlying uarch component is totally different. Our attack
has nothing to do with the memory subsystem or caching, and that CVE
is specifically for data caching (e.g. some fixes for CVE-2005-0109 do
not address this new attack vector at all).

## Disclosure timeline

01 Oct 2018: Notified Intel Security
26 Oct 2018: Notified openssl-security
26 Oct 2018: Notified CERT-FI
26 Oct 2018: Notified oss-security distros list
01 Nov 2018: Embargo expired

## Fix

Disable SMT/Hyper-Threading in the bios

Upgrade to OpenSSL 1.1.1 (or >= 1.1.0i if you are looking for patches)

## Credit

Billy Bob Brumley, Cesar Pereida Garcia, Sohaib ul Hassan, Nicola
Tuveri (Tampere University of Technology, Finland)
Alejandro Cabrera Aldaya (Universidad Tecnologica de la Habana CUJAE, Cuba)

## Refs

https://marc.info/?l=openbsd-cvs&m=152943660103446
https://marc.info/?l=openbsd-tech&m=153504937925732

## Exploit

Attached exploit code (password "infected") should work out of the box
for Skylake and Kaby Lake. Said code, soon to be followed by a
preprint with all the nitty-gritty details, is also here:

https://github.com/bbbrumley/portsmash
Solar Designer
2018-11-02 11:46:56 UTC
Permalink
Hi BBB,
Post by Billy Brumley
We recently discovered a new CPU microarchitecture attack vector. The
nature of the leakage is due to execution engine sharing on SMT (e.g.
Hyper-Threading) architectures. More specifically, we detect port
contention to construct a timing side channel to exfiltrate
information from processes running in parallel on the same physical
core. Report is below.
I think your work is top-notch and much needed. Thank you!

I'm surprised this specific side-channel wasn't(?) explored in academic
papers before. I had suggested it should be:

https://www.openwall.com/lists/oss-security/2015/08/12/8

"Yet another thing to target, and one I considered
and briefly played with on P4 with HT in 2005 when I saw Colin
Percival's paper, would be utilization of different execution units
within a core, which is measurable from another hardware thread running
on the same core. Surprisingly, I am still unaware of published
research on that."

As you correctly point out, it's (also) execution port contention,
rather than only execution unit contention.

However, I feel the blame might be misplaced here. I think the
existence of this side-channel in SMT should be obvious to the extent
that it's not considered a vulnerability, but a fully expected by-design
property. Maybe the problem is it wasn't documented as such. Maybe we
should have put more effort into making it more obvious to everyone in
2005, like it's finally done now.

[ Non-security:
A related area for further research is looking into use of this
side-channel for performance optimization - to probabilistically
(de)synchronize hardware threads sharing a physical core in a way
minimizing their competition for resources. I'm already parsing
OS-provided info and use per-core mutexes for this, achieving a few
percent speedup in a certain production setup, but maybe an
OS-independent and/or lower-overhead approach can be developed. ]
Post by Billy Brumley
We steal an OpenSSL (<= 1.1.0h) P-384 private key from a TLS server
using this new side-channel vector. It is a local attack in the sense
that the malicious process must be running on the same physical core
as the victim (an OpenSSL-powered TLS server in this case).
Are you also releasing manuscript.pdf you had attached to your distros
list posting? You must be.

I only skimmed it, but as I understand the OpenSSL code in question
is branching upon a secret. This is generally considered high-risk
even without SMT. While it'd be harder and less practical to exploit
without SMT, the state of instruction cache changes in a way visible to
other processes that might be scheduled to run on the same core.
Perhaps it'd take orders of magnitude more observations since the OS
scheduler won't kick in very frequently, but eventually the secret
should be obtainable.

I guess this commit is (part of?) the fix:

https://github.com/openssl/openssl/commit/5d92b853f6b875ba8d1a1b51b305f14df5adb8aa

In there, we see a ladder of function calls separated by "||", which in
C guarantees short-circuit evaluation. This is data-dependent
branching, and it remains such after that commit. Being unfamiliar with
ECC and with this code, I don't know whether the branching is (still) by
secret or not (anymore). I'd appreciate your comments on this.
Post by Billy Brumley
Upgrade to OpenSSL 1.1.1 (or >= 1.1.0i if you are looking for patches)
OpenSSL recently issued two security advisories suggesting a further
upgrade to 1.1.1a or 1.1.0j, but then mentioning that "a new side
channel attack was created" and listing commits with even further fixes
(not releases):

https://www.openssl.org/news/secadv/20181029.txt

---
Timing vulnerability in ECDSA signature generation (CVE-2018-0735)
==================================================================

Severity: Low

The OpenSSL ECDSA signature algorithm has been shown to be vulnerable to a
timing side channel attack. An attacker could use variations in the signing
algorithm to recover the private key.

Due to the low severity of this issue we are not issuing a new release
of OpenSSL 1.1.1 or 1.1.0 at this time. The fix will be included in
OpenSSL 1.1.1a and OpenSSL 1.1.0j when they become available. The fix
is also available in commit b1d6d55ece (for 1.1.1) and commit 56fb454d28
(for 1.1.0) in the OpenSSL git repository.

This issue was reported to OpenSSL on 25th October 2018 by Samuel Weiser.
---

https://www.openssl.org/news/secadv/20181030.txt

---
Timing vulnerability in DSA signature generation (CVE-2018-0734)
================================================================

Severity: Low

The OpenSSL DSA signature algorithm has been shown to be vulnerable to a
timing side channel attack. An attacker could use variations in the signing
algorithm to recover the private key.

Due to the low severity of this issue we are not issuing a new release
of OpenSSL 1.1.1, 1.1.0 or 1.0.2 at this time. The fix will be included
in OpenSSL 1.1.1a, OpenSSL 1.1.0j and OpenSSL 1.0.2q when they become
available. The fix is also available in commit 8abfe72e8c (for 1.1.1),
ef11e19d13 (for 1.1.0) and commit 43e6a58d49 (for 1.0.2) in the OpenSSL
git repository.

This issue was reported to OpenSSL on 16th October 2018 by Samuel Weiser.

As a result of the changes made to mitigate this vulnerability, a new
side channel attack was created. The mitigation for this new vulnerability
can be found in these commits: 6039651c43 (for 1.1.1), 26d7fce13d (for 1.1.0)
and 880d1c76ed (for 1.0.2)
---

I don't know to what extent this is related or not.

Thanks again,

Alexander
Billy Brumley
2018-11-02 14:42:33 UTC
Permalink
Post by Solar Designer
However, I feel the blame might be misplaced here. I think the
existence of this side-channel in SMT should be obvious to the extent
that it's not considered a vulnerability, but a fully expected by-design
property. Maybe the problem is it wasn't documented as such. Maybe we
should have put more effort into making it more obvious to everyone in
2005, like it's finally done now.
It's a fair comment.

I've been doing SCA a while now; L1 dcache timings (SMT), L1 icache
timings (SMT), remote timings, bug attacks, Flush+Reload, etc. Outside
of bug attacks (which are deterministic), this is the most
reproducible vector I've ever seen. I feel like that's one reason
holding back disabling SMT, because they are not trivial to reproduce.

If you have the setup I described:

https://github.com/bbbrumley/portsmash

Pull the code, follow the instructions. You'll see the signals we used
in the attack. No address dependencies, adapting to cache geometry,
etc -- it just works out of the box.
Post by Solar Designer
Are you also releasing manuscript.pdf you had attached to your distros
list posting? You must be.
It's coming -- I promise. I submitted it as an IACR eprint yesterday
("Port Contention for Fun and Profit") -- currently under moderation,
but will eventually pop out here:

https://eprint.iacr.org/

(Side note: I have raised this issue several times with IACR. I can't
get a permalink from them until I submit and it clears the mod queue.
But I can't submit stuff that's still under embargo. It's a catch 22.
Ofc there are technical solutions from IACR side but they won't
address it. Share your opinion: @IACR_News current co-editor is
@Leptan.)
Post by Solar Designer
I only skimmed it, but as I understand the OpenSSL code in question
is branching upon a secret. This is generally considered high-risk
even without SMT. While it'd be harder and less practical to exploit
without SMT, the state of instruction cache changes in a way visible to
other processes that might be scheduled to run on the same core.
Perhaps it'd take orders of magnitude more observations since the OS
scheduler won't kick in very frequently, but eventually the secret
should be obtainable.
The code in question certainly had lots of SCA issues :) I was the
first to show it vulnerable with an L1 dcache SMT attack (ASIACRYPT
2009). OpenSSL didn't respond during disclosure. Side note:
openssl-security is so much better since HeartBleed. They're really on
top of things, and being GitHub-based now the code is constantly
improving. If you're reading, go contribute to the project!

If there's something good about a vulnerability being unpatched for
almost a decade: that code path sparked quite a lot of academic work
in microarchitecture attacks.
Post by Solar Designer
https://github.com/openssl/openssl/commit/5d92b853f6b875ba8d1a1b51b305f14df5adb8aa
For the 1.1.0 branch, at

https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/crypto/ec/ec_mult.c

everything starting from aab7c770353b1dc4ba045938c8fb446dd1c4531e
Post by Solar Designer
In there, we see a ladder of function calls separated by "||", which in
C guarantees short-circuit evaluation. This is data-dependent
branching, and it remains such after that commit. Being unfamiliar with
ECC and with this code, I don't know whether the branching is (still) by
secret or not (anymore). I'd appreciate your comments on this.
Those branches are actually public; that is unofficial OpenSSL style
guide to avoid lots of if / else if / goto statements to detect return
errors from function calls.
Post by Solar Designer
Post by Billy Brumley
Upgrade to OpenSSL 1.1.1 (or >= 1.1.0i if you are looking for patches)
OpenSSL recently issued two security advisories suggesting a further
upgrade to 1.1.1a or 1.1.0j, but then mentioning that "a new side
channel attack was created" and listing commits with even further fixes
...
Post by Solar Designer
Timing vulnerability in ECDSA signature generation (CVE-2018-0735)
...
Post by Solar Designer
Timing vulnerability in DSA signature generation (CVE-2018-0734)
...
Post by Solar Designer
I don't know to what extent this is related or not.
These are unrelated, but you're certainly not the first to ask ;)

BBB
Solar Designer
2018-11-06 19:21:31 UTC
Permalink
Post by Billy Brumley
It's coming -- I promise. I submitted it as an IACR eprint yesterday
("Port Contention for Fun and Profit") -- currently under moderation,
https://eprint.iacr.org/
(Side note: I have raised this issue several times with IACR. I can't
get a permalink from them until I submit and it clears the mod queue.
But I can't submit stuff that's still under embargo. It's a catch 22.
Ofc there are technical solutions from IACR side but they won't
@Leptan.)
I pinged @Leptan on Twitter earlier today with:

"@Leptan Any chance you could push "Port Contention for Fun and Profit"
through @IACR_News moderation? It's in mainstream news since Friday, but
the paper is still not public. I think moderation should be quick (one
day) at least in cases like this. Thanks!"

And promptly heard back with:

"Done. Special cases where publication should be timed with the
mainstream news requires to let us know because we cannot guess..."

Cesar Pereida Garcia already posted the link to oss-security (thanks!),
but unfortunately not (reliably) to this thread (no In-Reply-To header),
so here it is again for those browsing the thread in archives:

https://eprint.iacr.org/2018/1060
Post by Billy Brumley
The code in question certainly had lots of SCA issues :) I was the
first to show it vulnerable with an L1 dcache SMT attack (ASIACRYPT
openssl-security is so much better since HeartBleed. They're really on
top of things, and being GitHub-based now the code is constantly
improving. If you're reading, go contribute to the project!
If there's something good about a vulnerability being unpatched for
almost a decade: that code path sparked quite a lot of academic work
in microarchitecture attacks.
For the 1.1.0 branch, at
https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/crypto/ec/ec_mult.c
everything starting from aab7c770353b1dc4ba045938c8fb446dd1c4531e
https://github.com/openssl/openssl/commit/aab7c770353b1dc4ba045938c8fb446dd1c4531e

Per my reading, this introduces a closer-to-constant-time implementation
and invokes it in some special cases ("the common cases where the scalar
is secret") at the start of ec_wNAF_mul, letting that function fall
through to its old presumably non-constant-time code in other cases.
Further commits don't change that. For someone like me not familiar
with ECC nor with this codebase it's tricky to figure out which
side-channel leaks and where exactly were in the old/generic
implementation (but I tried, below) and whether it's somehow safe to use
in cases where it's still reachable.

The paper says:

"In OpenSSL 1.1.0h and below, P-384 calls ecdsa_sign_setup @
crypto/ec/ecdsa_ossl.c when generating an ECDSA signature. There, the
underlying ec_wNAF_mul function gets called to perform scalar
multiplications, where r = [k]G is the relevant computation for this
work. That function first transforms the scalar representation, the
actual scalar multiplication algorithm executes a series of double and
add operations. To perform double and add operations, OpenSSL calls
ec_GFp_simple_dbl and ec_GFp_simple_add respectively. There, these
methods have several function calls to simpler and lower level
Montgomery arithmetic, e.g. shift, add, subtract, multiply, and square
operations. A single ECC double (or add) operation performs several
calls to these arithmetic functions."

I assume ec_GFp_simple_dbl and ec_GFp_simple_add are in fact called via
EC_POINT_dbl and EC_POINT_add (via function pointer indirection inside
them, which I didn't follow), respectively. This code skips the call to
EC_POINT_add when digit is 0, and the setting and handling of is_neg is
also potentially leaky:

for (k = max_len - 1; k >= 0; k--) {
if (!r_is_at_infinity) {
if (!EC_POINT_dbl(group, r, r, ctx))
goto err;
}

for (i = 0; i < totalnum; i++) {
if (wNAF_len[i] > (size_t)k) {
int digit = wNAF[i][k];
int is_neg;

if (digit) {
is_neg = digit < 0;

if (is_neg)
digit = -digit;

if (is_neg != r_is_inverted) {
if (!r_is_at_infinity) {
if (!EC_POINT_invert(group, r, ctx))
goto err;
}
r_is_inverted = !r_is_inverted;
}

/* digit > 0 */

if (r_is_at_infinity) {
if (!EC_POINT_copy(r, val_sub[i][digit >> 1]))
goto err;
r_is_at_infinity = 0;
} else {
if (!EC_POINT_add
(group, r, r, val_sub[i][digit >> 1], ctx))
goto err;
}
}
}
}
}

Wikipedia confirms that this is a known issue in double-and-add, and
Montgomery ladder is a way to avoid it:

https://en.wikipedia.org/wiki/Elliptic_curve_point_multiplication#Point_multiplication

Also, while the newly introduced implementation is still called
ec_mul_consttime in OpenSSL_1_1_0-stable, it's renamed to
ec_scalar_mul_ladder in OpenSSL_1_1_1-stable and has this comment on it:

* NB: This says nothing about the constant-timeness of the ladder step
* implementation (i.e., the default implementation is based on EC_POINT_add and
* EC_POINT_dbl, which of course are not constant time themselves) or the
* underlying multiprecision arithmetic.

Is this still an issue needing fixing, or is it e.g. believed to be
sufficiently mitigated by blinding?

Thanks,

Alexander

P.S. Congrats on receiving the grant for "SCARE: Side-Channel Aware
Engineering", and I hope we'll see more excellent research from your
team in the next 5 years:

https://pervasive.cs.tut.fi/?p=2747
Billy Brumley
2018-11-07 07:42:46 UTC
Permalink
Post by Solar Designer
Post by Billy Brumley
For the 1.1.0 branch, at
https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/crypto/ec/ec_mult.c
everything starting from aab7c770353b1dc4ba045938c8fb446dd1c4531e
This was not very responsible of me, since the changes are across
several files. I reckon the best source is checking the diff between
1.1.0h and 1.1.0i releases.

If you are a package maintainer, and are putting together a patch set
for this, please reach out to me. My team can help test.
Post by Solar Designer
I assume ec_GFp_simple_dbl and ec_GFp_simple_add are in fact called via
EC_POINT_dbl and EC_POINT_add (via function pointer indirection inside
them, which I didn't follow), respectively. This code skips the call to
EC_POINT_add when digit is 0, and the setting and handling of is_neg is
The skipping when digit is 0: yea that's basically it, and what
researchers have been targeting since 2009 (Section 3.2):

https://www.iacr.org/archive/asiacrypt2009/59120664/59120664.pdf

The setting and handling of is_neg: yea that leaks too (Section 6.4):

https://eprint.iacr.org/2015/1141
Post by Solar Designer
Also, while the newly introduced implementation is still called
ec_mul_consttime in OpenSSL_1_1_0-stable, it's renamed to
* NB: This says nothing about the constant-timeness of the ladder step
* implementation (i.e., the default implementation is based on EC_POINT_add and
* EC_POINT_dbl, which of course are not constant time themselves) or the
* underlying multiprecision arithmetic.
Is this still an issue needing fixing, or is it e.g. believed to be
sufficiently mitigated by blinding?
Does it still leak? Yes. For example, browse this PR:

https://github.com/openssl/openssl/pull/6116

Where David Benjamin (BoringSSL) states: "Note BN_mod_mul is itself
not constant-time ..."

So if a library's basic arithmetic function that takes two numbers,
multiplies them, and returns the remainder after division is not
constant time, that potentially affects quite a lot of public key
cryptography across the board

In the context of OpenSSL ECC, should people worry about it? Not
immediately. But don't take my word -- look at the data:

https://eprint.iacr.org/2018/651

(You can safely ignore the crazy Chinese crypto stuff -- that was just
a way to academically sell the ECC improvements in OpenSSL.)

That paper shows a concrete, measurable improvement in side-channel
security with the changes that went into 1.1.1 and 1.1.0i.

openssl-security reached out to me recently for some disclosure
assistance -- here is a quote from me in that discussion:

"In the future, you're going to see more and more SCA attacks drilling
down into the BN module (where it's harder to exploit). It's a good
trend, as lower hanging fruit is drying up!"

OpenSSL knows that constant time crypto is important. At the same
time, there is no quick fix because the set of crypto that OpenSSL
must support is significantly larger than you'll find in other
libraries that lack flexibility (and market share).

For OpenSSL, it's windy at the top! You will see steady improvements
in OpenSSL side-channel security moving forward.
Post by Solar Designer
P.S. Congrats on receiving the grant for "SCARE: Side-Channel Aware
Engineering", and I hope we'll see more excellent research from your
https://pervasive.cs.tut.fi/?p=2747
Thanks :) If you're still reading, here at the end a call to action
for researchers.

If you are a side-channel researcher: Don't just approach
openssl-security pointing at a line of code that you think might leak.
At a bare minimum, accompany that with patches. Even better, bring
them empirical data. They are not side-channel experts, and you are --
so act like it.

BBB
Marc Deslauriers
2018-11-09 13:03:46 UTC
Permalink
Hi,
Post by Billy Brumley
Post by Billy Brumley
For the 1.1.0 branch, at
https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/crypto/ec/ec_mult.c
everything starting from aab7c770353b1dc4ba045938c8fb446dd1c4531e
This was not very responsible of me, since the changes are across
several files. I reckon the best source is checking the diff between
1.1.0h and 1.1.0i releases.
If you are a package maintainer, and are putting together a patch set
for this, please reach out to me. My team can help test.
<snip>

Could you please confirm the following commits are sufficient to fix CVE-2018-5407?


Elliptic curve scalar multiplication with timing attack defenses (CVE-2018-5407)
https://git.openssl.org/?p=openssl.git;a=commit;h=aab7c770353b1dc4ba045938c8fb446dd1c4531e

Address code style comments
https://git.openssl.org/?p=openssl.git;a=commit;h=f06437c751d6f6ec7f4176518e2897f44dd58eb0

ladder description: why it works
https://git.openssl.org/?p=openssl.git;a=commit;h=33588c930d39d67d1128794dc7c85bae71af24ad

Pass through
https://git.openssl.org/?p=openssl.git;a=commit;h=f916a735bcdce496cebc7653a8ad2e72b333405a

Move up check for EC_R_INCOMPATIBLE_OBJECTS and for the point at infinity case
https://git.openssl.org/?p=openssl.git;a=commit;h=b43ad53119c0ac2ecfa6e4356210ccda57e0d16b

Remove superfluous NULL checks. Add Andy's BN_FLG comment.
https://git.openssl.org/?p=openssl.git;a=commit;h=2172133d0dc58256bf776da074c0d1944fef15cb


Thanks!

Marc.
--
Marc Deslauriers
Ubuntu Security Engineer | http://www.ubuntu.com/
Canonical Ltd. | http://www.canonical.com/
Billy Brumley
2018-11-09 16:41:23 UTC
Permalink
Post by Marc Deslauriers
Could you please confirm the following commits are sufficient to fix CVE-2018-5407?
Elliptic curve scalar multiplication with timing attack defenses (CVE-2018-5407)
https://git.openssl.org/?p=openssl.git;a=commit;h=aab7c770353b1dc4ba045938c8fb446dd1c4531e
Address code style comments
https://git.openssl.org/?p=openssl.git;a=commit;h=f06437c751d6f6ec7f4176518e2897f44dd58eb0
ladder description: why it works
https://git.openssl.org/?p=openssl.git;a=commit;h=33588c930d39d67d1128794dc7c85bae71af24ad
Pass through
https://git.openssl.org/?p=openssl.git;a=commit;h=f916a735bcdce496cebc7653a8ad2e72b333405a
Move up check for EC_R_INCOMPATIBLE_OBJECTS and for the point at infinity case
https://git.openssl.org/?p=openssl.git;a=commit;h=b43ad53119c0ac2ecfa6e4356210ccda57e0d16b
Remove superfluous NULL checks. Add Andy's BN_FLG comment.
https://git.openssl.org/?p=openssl.git;a=commit;h=2172133d0dc58256bf776da074c0d1944fef15cb
It's a good start! But it's more than that. But it's Friday night so
it'll have to wait until Monday.

BBB
Billy Brumley
2018-11-12 09:34:22 UTC
Permalink
Post by Marc Deslauriers
Post by Billy Brumley
If you are a package maintainer, and are putting together a patch set
for this, please reach out to me. My team can help test.
<snip>
Could you please confirm the following commits are sufficient to fix CVE-2018-5407?
Some more technical advice below. Hope it helps!

BBB

# 1.0.1

That is EOL. Try your luck with porting the 1.0.2 solution.

Shameless self plug: read Section 2

https://eprint.iacr.org/2018/354

for a related discussion about EOL issues and security in the context
of OpenSSL.

# 1.0.2

Wait until this gets merged into OpenSSL_1_0_2-stable :

https://github.com/openssl/openssl/pull/7593

# 1.1.0 up to and including 1.1.0h

So I went through the process to patch this myself:

https://github.com/bbbrumley/openssl/tree/bbb_ecc_fix_110h

Ofc I have no idea what 1.1.0 version you started with, or what
patches you're applying. So take this as more of a HOWTO build and
test your own patchset.

## CVE-2018-5407

git checkout OpenSSL_1_1_0h -b bbb_ecc_fix_110h
git cherry-pick aab7c770353b1dc4ba045938c8fb446dd1c4531e
git cherry-pick f06437c751d6f6ec7f4176518e2897f44dd58eb0
git cherry-pick 33588c930d39d67d1128794dc7c85bae71af24ad
git cherry-pick f916a735bcdce496cebc7653a8ad2e72b333405a
git cherry-pick b43ad53119c0ac2ecfa6e4356210ccda57e0d16b
git cherry-pick 2172133d0dc58256bf776da074c0d1944fef15cb
git cherry-pick cc39f9250957dfe6e9f1b62a4eca1863e8451483
git cherry-pick 7b3e775a6a78650bbd3e8e19a5aa12981880402b
git cherry-pick 5eee95a54de6854e60886c8e662a902184b12d04
git cherry-pick 875ba8b21ecc65ad9a6bdc66971e50461660fcbb
git checkout --theirs CHANGES
git add CHANGES
git cherry-pick --continue
git checkout OpenSSL_1_1_0h -- CHANGES
git add CHANGES
git commit -m "revert changelog diffs"
git rebase -i OpenSSL_1_1_0h

(I skipped 926b21117df939241f1cd63f2f9e3ab87819f0ed because it is not
related to CVE-2018-5407. See

https://github.com/openssl/openssl/issues/6302

For a lengthy discussion. I'm not familiar enough with the issue to
give advice if you need to pick it up or not.)

All of them cherry pick cleanly except for the last one, but it's only
a trivial conflict with the changelog.

I checked the scalar multiplication code paths in ecdsatest with gdb
(break ec_mult.c:423), and indeed they are early exiting to the new
function when signing.

A lot of new regression testing went into 1.1.1. Some of it was
backported 1.1.0:

https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/test

So I fetched these KATs:

https://raw.githubusercontent.com/openssl/openssl/23fe5c582a83bce394a3cdf0bc8f6f4f2eb71ebb/test/recipes/30-test_evp_data/evppkey_ecc.txt

To run those tests, you also need to pick up this bug fix for
evp_test.c (this is for testing, not part of the CVE-2018-5407 fix) :

git cherry-pick e35e5941e0b2f7af1cd56f07ee8d4eaf2b445132

Then rebuilt, and ran

$ test/evp_test /path/to/evppkey_ecc.txt
484 tests completed with 0 errors, 0 skipped

All of those (positive and negative) tests pass; they are for ECC
keygen and ECDH. I checked the scalar multiplication code paths with
gdb (break ec_mult.c:423), and indeed they all early exit to the new
function.

## CVE-2018-0735

Apply this small fix on top:

git cherry-pick 56fb454d281a023b3f950d969693553d3f3ceea1
git cherry-pick 003f1bfd185267cc67ac9dc521a27d7a2af0d0ee
git rebase -i HEAD~2

Then ofc rerun all the regression testing ("make test", as well as the
custom EVP tests described above.)
Marc Deslauriers
2018-11-12 12:24:46 UTC
Permalink
Post by Billy Brumley
Post by Marc Deslauriers
Post by Billy Brumley
If you are a package maintainer, and are putting together a patch set
for this, please reach out to me. My team can help test.
<snip>
Could you please confirm the following commits are sufficient to fix CVE-2018-5407?
Some more technical advice below. Hope it helps!
BBB
# 1.0.1
That is EOL. Try your luck with porting the 1.0.2 solution.
Shameless self plug: read Section 2
https://eprint.iacr.org/2018/354
for a related discussion about EOL issues and security in the context
of OpenSSL.
# 1.0.2
https://github.com/openssl/openssl/pull/7593
# 1.1.0 up to and including 1.1.0h
https://github.com/bbbrumley/openssl/tree/bbb_ecc_fix_110h
Ofc I have no idea what 1.1.0 version you started with, or what
patches you're applying. So take this as more of a HOWTO build and
test your own patchset.
## CVE-2018-5407
git checkout OpenSSL_1_1_0h -b bbb_ecc_fix_110h
git cherry-pick aab7c770353b1dc4ba045938c8fb446dd1c4531e
git cherry-pick f06437c751d6f6ec7f4176518e2897f44dd58eb0
git cherry-pick 33588c930d39d67d1128794dc7c85bae71af24ad
git cherry-pick f916a735bcdce496cebc7653a8ad2e72b333405a
git cherry-pick b43ad53119c0ac2ecfa6e4356210ccda57e0d16b
git cherry-pick 2172133d0dc58256bf776da074c0d1944fef15cb
git cherry-pick cc39f9250957dfe6e9f1b62a4eca1863e8451483
git cherry-pick 7b3e775a6a78650bbd3e8e19a5aa12981880402b
git cherry-pick 5eee95a54de6854e60886c8e662a902184b12d04
git cherry-pick 875ba8b21ecc65ad9a6bdc66971e50461660fcbb
git checkout --theirs CHANGES
git add CHANGES
git cherry-pick --continue
git checkout OpenSSL_1_1_0h -- CHANGES
git add CHANGES
git commit -m "revert changelog diffs"
git rebase -i OpenSSL_1_1_0h
(I skipped 926b21117df939241f1cd63f2f9e3ab87819f0ed because it is not
related to CVE-2018-5407. See
https://github.com/openssl/openssl/issues/6302
For a lengthy discussion. I'm not familiar enough with the issue to
give advice if you need to pick it up or not.)
All of them cherry pick cleanly except for the last one, but it's only
a trivial conflict with the changelog.
I checked the scalar multiplication code paths in ecdsatest with gdb
(break ec_mult.c:423), and indeed they are early exiting to the new
function when signing.
A lot of new regression testing went into 1.1.1. Some of it was
https://github.com/openssl/openssl/commits/OpenSSL_1_1_0-stable/test
https://raw.githubusercontent.com/openssl/openssl/23fe5c582a83bce394a3cdf0bc8f6f4f2eb71ebb/test/recipes/30-test_evp_data/evppkey_ecc.txt
To run those tests, you also need to pick up this bug fix for
git cherry-pick e35e5941e0b2f7af1cd56f07ee8d4eaf2b445132
Then rebuilt, and ran
$ test/evp_test /path/to/evppkey_ecc.txt
484 tests completed with 0 errors, 0 skipped
All of those (positive and negative) tests pass; they are for ECC
keygen and ECDH. I checked the scalar multiplication code paths with
gdb (break ec_mult.c:423), and indeed they all early exit to the new
function.
## CVE-2018-0735
git cherry-pick 56fb454d281a023b3f950d969693553d3f3ceea1
git cherry-pick 003f1bfd185267cc67ac9dc521a27d7a2af0d0ee
git rebase -i HEAD~2
Then ofc rerun all the regression testing ("make test", as well as the
custom EVP tests described above.)
Thank you very much for the info!

Marc.

Cesar Pereida Garcia
2018-11-06 17:26:08 UTC
Permalink
It's coming -- I promise. I submitted it as an IACR eprint yesterday
("Port Contention for Fun and Profit") -- currently under moderation,
but will eventually pop out here:

https://eprint.iacr.org/

The paper has cleared the queue and is now public at

https://eprint.iacr.org/2018/1060


Cesar Pereida Garcia
Doctoral Student
Tampere University of Technology
Loading...