Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbedtls-security-advisory-2021-07-1, -2021-07-2. CVE-2021-44732. Update to Mbed TLS 2.28.0 #43677

Closed
mkitti opened this issue Jan 6, 2022 · 25 comments
Labels
security System security concerns and vulnerabilities upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@mkitti
Copy link
Contributor

mkitti commented Jan 6, 2022

There have been several references to these security advisories for mbedtls in the issues / PRs in past few months. I'm not sure if they have received the needed attention since security advisory was not obvious in the title and there are no replies on the previous posts.

To make the issue clearer, I have produced the text of the advisories below.

Local side channel attack on RSA

https://tls.mbed.org/tech-updates/security-advisories/mbedtls-security-advisory-2021-07-1

Vulnerability

The Montgomery curves Curve25519 and Curve448, also known as x25519 and x448 when used for Diffie-Hellman, were designed to minimize the number of checks an implementation needs to do for secure use.

In particular, validity of the peer's public key needs not be checked, as long as the underlying multi-precision (bignum) arithmetic is constant-time. This is not the case in Mbed TLS, but validity checks were still skipped, so an attacker could exploit special inputs (low-order points) in order to cause variations in timing and memory access patterns that would in turn leak information about the private key.

Impact

An attacker with access to precise enough timing and memory access information (for example, able to execute arbitrary code and sharing a memory cache with the victim) can recover the private keys used in static Diffie-Hellman with x25519 and x448.

Resolution

Affected users will want to upgrade to Mbed TLS 3.0.0, 2.27.0 or 2.16.11 depending on the branch they're currently using.

Local side channel attack on static Diffie-Hellman with Montgomery curves

https://tls.mbed.org/tech-updates/security-advisories/mbedtls-security-advisory-2021-07-2

Vulnerability

The modular exponentiation operation in RSA uses a sliding window algorithm, with a memory access pattern that depends on the bits of the secret key.

Exponent blinding is used as a counter-measure: it prevents an attacker from correlating informations gathered on successive operation, but researchers found a way to recover enough information by observing a single operation, therefore by-passing this counter-measure.

Impact

An attacker with access to precise enough timing and memory access information (typically an untrusted operating system attacking a secure enclave such as SGX or the TrustZone secure world) can recover the private keys used in RSA.

Resolution

Affected users will want to upgrade to Mbed TLS 3.0.0, 2.27.0 or 2.16.11 depending on the branch they're currently using.

Prior References

#42634
#42311 (comment)

Potential double-free after an out of memory error

https://tls.mbed.org/tech-updates/security-advisories/mbedtls-security-advisory-2021-12

Vulnerability

If mbedtls_ssl_set_session() or mbedtls_ssl_get_session() were to fail with MBEDTLS_ERR_SSL_ALLOC_FAILED (in an out of memory condition), then calling mbedtls_ssl_session_free() and mbedtls_ssl_free() in the usual manner would cause an internal session buffer to be freed twice, due to two structures both having valid pointers to it after a call to ssl_session_copy().

Impact

An attacker could potentially trigger the out of memory condition, and therefore use this bug to create memory corruption, which could then be further exploited or targetted.

Resolution

Affected users will want to upgrade to Mbed TLS 3.1.0, 2.28.0 or 2.16.12 depending on the branch they're currently using.

Work-around

Either do not call mbedtls_ssl_session_free() (which will unfortunately cause a memory leak) or set the mbedtls_ssl_session field ticket to NULL manually, in the case where either mbedtls_ssl_set_session() or mbedtls_ssl_get_session() returns MBEDTLS_ERR_SSL_ALLOC_FAILED.

@mkitti mkitti changed the title mbedtls-security-advisory-2021-07-1, -2021-07-2. Update to Mbed TLS 2.27.0 mbedtls-security-advisory-2021-07-1, -2021-07-2. Update to Mbed TLS 2.28.0 Jan 6, 2022
@mkitti mkitti changed the title mbedtls-security-advisory-2021-07-1, -2021-07-2. Update to Mbed TLS 2.28.0 mbedtls-security-advisory-2021-07-1, -2021-07-2. CVE-2021-44732. Update to Mbed TLS 2.28.0 Jan 6, 2022
@JeffBezanson JeffBezanson added security System security concerns and vulnerabilities upstream The issue is with an upstream dependency, e.g. LLVM labels Jan 7, 2022
@nsslh
Copy link

nsslh commented Jan 10, 2022

Thanks for the issue and the security tag. Much appreciated.

@ViralBShah
Copy link
Member

ViralBShah commented Jan 30, 2022

We need a bump on mbedtls due to the CVE for the next release. Conversation here: Homebrew/discussions#2749 (reply in thread)

The WIP PR is #42311

@mbauman
Copy link
Member

mbauman commented May 24, 2022

So we're currently on mbedtls v2.28.0 — the latest of the v2 branch — on v1.8 and above. v1.6 is on v2.24. Unfortunately, mbedtls does not follow semver and a simple backport of the upgrade is breaking. I think our only hope of addressing these security issues on the LTS is finding and carrying the individual security patches ourselves.

@ViralBShah
Copy link
Member

Does any Linux distro carry 2.24 in their LTS? If so, they may have backports.

@mbauman
Copy link
Member

mbauman commented May 24, 2022

The only one tracked by pkgs.org is NetBSD 8.2... but it doesn't look like that's actually the case. In any case, they're definitely not carrying the patches.

@mkitti
Copy link
Contributor Author

mkitti commented May 24, 2022

We may need to admit that we are underresourced in the security domain.

To mitigate this, I suggest that we be selective about which versions we incorporate into base Julia, especially LTS releases going forward.

Perhaps we should evaluate if OS distributions have incorporated that particular version into stable or LTS releases. For example, if we were discussing 2.16 we might have an example to follow from the Debian security team.

https://packages.debian.org/search?keywords=mbedtls

Regarding mbedtls in particular, I have lost confidence that mbedtls is a good SSL/TLS dependency for Julia. Upstream's release notes clearly expect that dependents can address ABI breakage simply through recompilation. This is not the case for Julia and BinaryBuilder.

We should strongly consider switching to OpenSSL. I recognize this is easier said than done.

@mkitti
Copy link
Contributor Author

mkitti commented May 24, 2022

Other options include BoringSSL
https://boringssl.googlesource.com/boringssl/ or LibreSSL https://www.libressl.org/ .

However, BoringSSL is not intended for 3rd party packages. LibreSSL has some issues regarding Linux support https://lwn.net/Articles/841664/ .

@ViralBShah
Copy link
Member

Yes @nalimilan had proposed this as well. It is a sound strategy to use versions of mbedtls from Linux distro LTS versions.

I know about debian/ubuntu processes but not so much how to look for the same on redhat.

@ViralBShah
Copy link
Member

If mbedtls is breaking, can we not upgrade all the dependent stdlibs too on 1.6? Won't Pkg deal with upgrading any package dependencies?

@mkitti
Copy link
Contributor Author

mkitti commented May 25, 2022

Here is an excerpt from the mbedtls release notes:

Some fields of mbedtls_ssl_session and mbedtls_ssl_config are in a different order. This only affects applications that define such structures directly or serialize them.

https://github.com/Mbed-TLS/mbedtls/releases/tag/v2.28.0

@mkitti
Copy link
Contributor Author

mkitti commented May 25, 2022

If mbedtls is breaking, can we not upgrade all the dependent stdlibs too on 1.6? Won't Pkg deal with upgrading any package dependencies?

@giordano has answered this in the past:

JuliaPackaging/Yggdrasil#3024

MbedTLS_jll is a mess. The soversion of at least one of the libraries provided changes between minor versions (example 2.26 -> 2.27), but not always (e.g., nothing changed 2.25 -> 2.26). On top of that, MbedTLS_jll is a Julia dependency, so you can't possibly build a single library that can be loaded by different Julia versions which ship different versions of MbedTLS_jll, as they'd have different ABIs:

JuliaPackaging/Yggdrasil#4190 (comment)

For point 4, downstream libraries need to do something only if the affected library changes ABI/API in the new version, which is what happens with mbedtls. But you can't just rebuild the downstream libraries against the new fixed library: there are compatibility bounds to respect, accept the fact we can't change compat bounds inside the same patch version (changing only the build number), and if the affected library is shipped by Julia, you have also to deal with julia compatibility bounds and not break existing packages

JuliaPackaging/Yggdrasil#4179 (comment)

I don't think we've ever changed binary dependencies within a minor release. Also, this will break about 1779 packages depending on MbedTLS_jll, because 2.28 breaks the ABI compared to v2.24

@mkitti
Copy link
Contributor Author

mkitti commented Jun 7, 2022

We need to consider switching LTS to 1.7. I'm not seeing a way forward to fix these security issues in 1.6.

@mkitti mkitti closed this as completed Jun 7, 2022
@mkitti mkitti reopened this Jun 7, 2022
@giordano
Copy link
Contributor

giordano commented Jun 7, 2022

Backport the patches? As long as they don't break the ABI

@mkitti
Copy link
Contributor Author

mkitti commented Jun 10, 2022

Backport the patches? As long as they don't break the ABI

Yes, but who will do this? It has been many months. Do I need to moonlight as a security engineer?

@KristofferC
Copy link
Member

Does anyone have a list of the actual commits that need to be backported?

@ViralBShah
Copy link
Member

ViralBShah commented Jun 13, 2022

Just noting here that mbedtls 2.28 is LTS until 2024: https://github.com/Mbed-TLS/mbedtls/releases/tag/v2.28.0

Unfortunately the release notes do not note which particular commits address the security issues. tls.mbedtls.org seems down.

@staticfloat
Copy link
Member

Identifying the patches that must be backported:

Next, I will be rebasing the patches on top of 2.24, creating a single, squashed diff for each CVE, then adding them to our from-source builds and Yggdrasil.

In the future, we should really stick to their LTS branches, so that they do this work for us. Luckily, we're already on 2.28, so there's no change to be made for master.

@staticfloat
Copy link
Member

Yggdrasil PR: JuliaPackaging/Yggdrasil#5081

Julia binaries bump and from-source patch bump PR: #45848

@KristofferC
Copy link
Member

https://tls.mbed.org has been down for me for weeks... And http://downforeveryoneorjustme.com agrees with me. Does it work for you all?

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Jun 29, 2022

TL;DR Why do we need mbedTLS (or any TLS at all)?

download() is no longer implemented by Julia (and in the new place, in THAT package, implemented by curl, i.e. LIbCurl.jl and LibCURL_jll.jl, which however, currently, depends on MbedTLS_jll), and is deprecated:

julia/base/download.jl

Lines 15 to 16 in bb5b98e

Since Julia 1.6, this function is deprecated and is just a thin wrapper
around `Downloads.download`. In new code, you should use that function

We may need to admit that we are underresourced in the security domain. [..] I have lost confidence that mbedtls is a good SSL/TLS dependency for Julia. [..]
We should strongly consider switching to OpenSSL. I recognize this is easier said than done.

I feel bad about this since if I recall, I was the one to suggest mbedTLS (at a time when OpenSSL was a mess). I'm curious what actual programs need from mbedTLS. I see we have libssh2, and why do we need security? :) It's a serious question. It seems to me Pkg needs it, and download capability, but should none of this (including Pkg?) be bundled with Julia? Why I drop it in #45853. [One position is nothing should be in Base unless needed by Julia itself, and Julia doesn't need the internet, only Pkg.]

Can we drop download() in 1.8, right now, before it's released, since it's deprecated anyway, to allow getting rid of mbedTLS (or do that at the same time). Makes it easier for it to be LTS later?

We need to consider switching LTS to 1.7.

Well, I wanted that all along (for other reasons). Since 1.7 is out of support, and if it's really considered for LTS, can download() be dropped there, for a kind of 1.7b. Since it's out of support anyway, I don't see we need to honor download() if 1.7 is resurrected as such a b-version.

1.7 should be compatible with 1.6 (well any new versions should be), and faster and with better newer syntax, not breaking any old. Would people be confident 1.7 has no new bugs over 1.6? We can have two LTS, at least for a while...

@ViralBShah
Copy link
Member

Let's slow down a bit! So long as we use the LTS version of mbedtls, there is really no issue.

@staticfloat
Copy link
Member

https://tls.mbed.org has been down for me for weeks... And http://downforeveryoneorjustme.com agrees with me. Does it work for you all?

No, I have to use archive.org.

@mkitti
Copy link
Contributor Author

mkitti commented Jun 30, 2022

https://tls.mbed.org has been down for me for weeks...

It redirects to https://www.trustedfirmware.org/projects/mbed-tls/ for me.

@KristofferC
Copy link
Member

Seems like it has been fixed now, indeed.

@mkitti
Copy link
Contributor Author

mkitti commented Feb 27, 2023

To clarify, this is fixed on Julia 1.6.7 and Julia 1.8.

Julia 1.7 was not and will not be patched for this since it is out of support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security System security concerns and vulnerabilities upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

No branches or pull requests

9 participants