On Tue, Sep 17, 2019 at 08:32:28AM +0200, Sebastian Andrzej Siewior wrote:
> Package: python-cryptography
> Version: 2.6.1-3
> Severity: serious
>
> The upload of latest openssl 1.1.1d triggert three testsuite failures in
> python-cryptography [0]
>
> - _________________ test_buffer_protocol_alternate_modes[mode5]
> __________________
>
> |mode = <cryptography.hazmat.primitives.ciphers.modes.XTS object at
> 0x7f0c8ceaba50>
> |backend = <cryptography.hazmat.backends.openssl.backend.Backend object at
> 0x7f0c95a29cd0>
> |
> | @pytest.mark.parametrize(
> | "mode",
> | [
> | modes.CBC(bytearray(b"\x00" * 16)),
> | modes.CTR(bytearray(b"\x00" * 16)),
> | modes.OFB(bytearray(b"\x00" * 16)),
> | modes.CFB(bytearray(b"\x00" * 16)),
> | modes.CFB8(bytearray(b"\x00" * 16)),
> | modes.XTS(bytearray(b"\x00" * 16)),
> | ]
> | )
> | @pytest.mark.requires_backend_interface(interface=CipherBackend)
> | def test_buffer_protocol_alternate_modes(mode, backend):
> | data = bytearray(b"sixteen_byte_msg")
> | cipher = base.Cipher(
> | algorithms.AES(bytearray(b"\x00" * 32)), mode, backend
> | )
> |> enc = cipher.encryptor()
> |
> |tests/hazmat/primitives/test_aes.py:495:
> |_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> _ _
> |/usr/lib/python2.7/dist-packages/cryptography/hazmat/primitives/ciphers/base.py:121:
> in encryptor
> | self.algorithm, self.mode
> |/usr/lib/python2.7/dist-packages/cryptography/hazmat/backends/openssl/backend.py:295:
> in create_symmetric_encryption_ctx
> | return _CipherContext(self, cipher, mode, _CipherContext._ENCRYPT)
> |/usr/lib/python2.7/dist-packages/cryptography/hazmat/backends/openssl/ciphers.py:116:
> in __init__
> | self._backend.openssl_assert(res != 0)
> |/usr/lib/python2.7/dist-packages/cryptography/hazmat/backends/openssl/backend.py:125:
> in openssl_assert
> | return binding._openssl_assert(self._lib, ok)
>
> This is due to commit 2a5f63c9a61be ("Allow AES XTS decryption using duplicate
> keys.").
> https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=2a5f63c9a61be
I'm guessing the commit message is a little bit misleading. I
think it now rejects it for encryption, but still allows it for
decryption. I think in the master branch we started with rejecting
both, and then allowed decryption again.
>
> - _____________________ TestDH.test_dh_parameters_supported
> ______________________
>
> |self = <tests.hazmat.primitives.test_dh.TestDH object at 0x7f0c65bbb3d0>
> |backend = <cryptography.hazmat.backends.openssl.backend.Backend object at
> 0x7f0c95a29cd0>
> |
> | def test_dh_parameters_supported(self, backend):
> | assert backend.dh_parameters_supported(23, 5)
> |> assert not backend.dh_parameters_supported(23, 18)
> |E assert not True
> |E + where True = <bound method Backend.dh_parameters_supported of
> <cryptography.hazmat.backends.openssl.backend.Backend object at
> 0x7f0c95a29cd0>>(23, 18)
> |E + where <bound method Backend.dh_parameters_supported of
> <cryptography.hazmat.backends.openssl.backend.Backend object at
> 0x7f0c95a29cd0>> = <cryptography.hazmat.backends.openssl.backend.Backend
> object at 0x7f0c95a29cd0>.dh_parameters_supported
> |
> |tests/hazmat/primitives/test_dh.py:161: AssertionError
>
> This is due to commit ddd16c2fe988e ("Change DH parameters to generate the
> order q subgroup instead of 2q").
> https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=ddd16c2fe988e
>
> - _____________ TestECDSACertificate.test_load_ecdsa_no_named_curve
> ______________
>
> |self = <tests.x509.test_x509.TestECDSACertificate object at 0x7f0c609e3590>
> |backend = <cryptography.hazmat.backends.openssl.backend.Backend object at
> 0x7f0c95a29cd0>
> |
> | def test_load_ecdsa_no_named_curve(self, backend):
> | _skip_curve_unsupported(backend, ec.SECP256R1())
> | cert = _load_cert(
> | os.path.join("x509", "custom", "ec_no_named_curve.pem"),
> | x509.load_pem_x509_certificate,
> | backend
> | )
> | with pytest.raises(NotImplementedError):
> |> cert.public_key()
> |E Failed: DID NOT RAISE <type 'exceptions.NotImplementedError'>
> |
> |tests/x509/test_x509.py:3722: Failed
>
> This is due to commit 9a43a733801bd ("[ec] Match built-in curves on
> EC_GROUP_new_from_ecparameters").
> https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff;h=9a43a733801bd
So the no named curve test fails now because OpenSSL now compares
it to built-in curves, and uses the built-in curves instead if it
has the parameters of a known curve.
The commit has a very long message about why this is done. so I'll
just duplicate it here:
[ec] Match built-in curves on EC_GROUP_new_from_ecparameters
Description
-----------
Upon `EC_GROUP_new_from_ecparameters()` check if the parameters match any
of the built-in curves. If that is the case, return a new
`EC_GROUP_new_by_curve_name()` object instead of the explicit parameters
`EC_GROUP`.
This affects all users of `EC_GROUP_new_from_ecparameters()`:
- direct calls to `EC_GROUP_new_from_ecparameters()`
- direct calls to `EC_GROUP_new_from_ecpkparameters()` with an explicit
parameters argument
- ASN.1 parsing of explicit parameters keys (as it eventually
ends up calling `EC_GROUP_new_from_ecpkparameters()`)
A parsed explicit parameter key will still be marked with the
`OPENSSL_EC_EXPLICIT_CURVE` ASN.1 flag on load, so, unless
programmatically forced otherwise, if the key is eventually serialized
the output will still be encoded with explicit parameters, even if
internally it is treated as a named curve `EC_GROUP`.
Before this change, creating any `EC_GROUP` object using
`EC_GROUP_new_from_ecparameters()`, yielded an object associated with
the default generic `EC_METHOD`, but this was never guaranteed in the
documentation.
After this commit, users of the library that intentionally want to
create an `EC_GROUP` object using a specific `EC_METHOD` can still
explicitly call `EC_GROUP_new(foo_method)` and then manually set the
curve parameters using `EC_GROUP_set_*()`.
Motivation
----------
This has obvious performance benefits for the built-in curves with
specialized `EC_METHOD`s and subtle but important security benefits:
- the specialized methods have better security hardening than the
generic implementations
- optional fields in the parameter encoding, like the `cofactor`, cannot
be leveraged by an attacker to force execution of the less secure
code-paths for single point scalar multiplication
- in general, this leads to reducing the attack surface
Check the manuscript at https://arxiv.org/abs/1909.01785 for an in depth
analysis of the issues related to this commit.
It should be noted that `libssl` does not allow to negotiate explicit
parameters (as per RFC 8422), so it is not directly affected by the
consequences of using explicit parameters that this commit fixes.
On the other hand, we detected external applications and users in the
wild that use explicit parameters by default (and sometimes using 0 as
the cofactor value, which is technically not a valid value per the
specification, but is tolerated by parsers for wider compatibility given
that the field is optional).
These external users of `libcrypto` are exposed to these vulnerabilities
and their security will benefit from this commit.
Related commits
---------------
While this commit is beneficial for users using built-in curves and
explicit parameters encoding for serialized keys, commit
b783beeadf6b80bc431e6f3230b5d5585c87ef87 (and its equivalents for the
1.0.2, 1.1.0 and 1.1.1 stable branches) fixes the consequences of the
invalid cofactor values more in general also for other curves
(CVE-2019-1547).
The following list covers commits in `master` that are related to the
vulnerabilities presented in the manuscript motivating this commit:
- d2baf88c43 [crypto/rsa] Set the constant-time flag in multi-prime RSA too
- 311e903d84 [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key
validation.
- b783beeadf [crypto/ec] for ECC parameters with NULL or zero cofactor,
compute it
- 724339ff44 Fix SCA vulnerability when using PVK and MSBLOB key formats
Note that the PRs that contributed the listed commits also include other
commits providing related testing and documentation, in addition to
links to PRs and commits backporting the fixes to the 1.0.2, 1.1.0 and
1.1.1 branches.
This commit includes a partial backport of
https://github.com/openssl/openssl/pull/8555
(commit 8402cd5f75f8c2f60d8bd39775b24b03dd8b3b38)
for which the main author is Shane Lontis.
Responsible Disclosure
----------------------
This and the other issues presented in https://arxiv.org/abs/1909.01785
were reported by Cesar Pereida García, Sohaib ul Hassan, Nicola Tuveri,
Iaroslav Gridin, Alejandro Cabrera Aldaya and Billy Bob Brumley from the
NISEC group at Tampere University, FINLAND.
The OpenSSL Security Team evaluated the security risk for this
vulnerability as low, and encouraged to propose fixes using public Pull
Requests.
