| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
A bug in the implementation of the Tonelli-Shanks algorithm can lead to
an infinite loop. This loop can be hit in various ways, in particular on
decompressing an elliptic curve public key via EC_POINT_oct2point() - to
do this, one must solve y^2 = x^3 + ax + b for y, given x.
If a certificate uses explicit encoding for elliptic curve parameters,
this operation needs to be done during certificate verification, leading
to a DoS. In particular, everything dealing with untrusted certificates
is affected, notably TLS servers explicitly configured to request
client certificates (httpd, smtpd, various VPN implementations, ...).
Ordinary TLS servers do not consume untrusted certificates.
The problem is that we cannot assume that x^3 + ax + b is actually a
square on untrusted input and neither can we assume that the modulus
p is a prime. Ensuring that p is a prime is too expensive (it would
likely itself lead to a DoS). To avoid the infinite loop, fix the logic
to be more resilient and explicitly limit the number of iterations that
can be done. The bug is such that the infinite loop can also be hit for
primes = 3 (mod 4) but fortunately that case is optimized earlier.
It's also worth noting that there is a size bound on the field size
enforced via OPENSSL_ECC_MAX_FIELD_BITS (= 661), which help mitigate
further DoS vectors in presence of this fix.
Reported by Tavis Ormandy and David Benjamin, Google
Patch based on the fixes by David Benjamin and Tomas Mraz, OpenSSL
ok beck inoguchi
This is errata/6.9/032_bignum.patch.sig
|
|
|
|
|
|
| |
ok sthen, beck, jsing, tb, etc etc
This cannot be issued as an errata/syspatch, because syspatch cannot
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In order to work around the expired DST Root CA X3 certficiate, enable
X509_V_FLAG_TRUSTED_FIRST in the legacy verifier. This means that the
default chain provided by Let's Encrypt will stop at the ISRG Root X1
intermediate, rather than following the DST Root CA X3 intermediate.
Note that the new verifier does not suffer from this issue, so only a
small number of things will hit this code path.
ok millert@ robert@ tb@
this is errata 6.9/018_cert
|
|
|
|
|
|
|
|
|
|
|
|
| |
The length checks need to be >= rather than > in order to ensure the string
remains NUL terminated. While here consistently check wi before using it
so we have the same idiom throughout this function.
Issue reported by GoldBinocle on GitHub.
ok deraadt@ tb@
this is 6.9 errata 017
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
X509_CERT_AUX_print().
Commit in -current:
CVSROOT: /cvs
Module name: src
Changes by: schwarze@cvs.openbsd.org 2021/07/10 11:45:16
Modified files:
lib/libcrypto/asn1: t_x509a.c
Log message:
Fix a read buffer overrun in X509_CERT_AUX_print(3),
which by implication also affects X509_print(3).
The ASN1_STRING_get0_data(3) manual explitely cautions the reader
that the data is not necessarily NUL-terminated, and the function
X509_alias_set1(3) does not sanitize the data passed into it in
any way either, so we must assume the alias->data field is merely
a byte array and not necessarily a string in the sense of the C
language.
I found this bug while writing manual pages for these functions.
OK tb@
As an aside, note that the function still produces incomplete and
misleading results when the data contains a NUL byte in the middle
and that error handling is consistently absent throughout, even
though the function provides an "int" return value obviously intended
to be 1 for success and 0 for failure, and even though this function
is called by another function that also wants to return 1 for success
and 0 for failure and even does so in many of its code paths, though
not in others. But let's stay focussed. Many things would be nice
to have in the wide wild world, but a buffer overflow must not be
allowed to remain in our backyard.
This is patches/6.9/common/015_x509.patch.sig
|
|
|
|
|
|
|
|
|
|
|
| |
This is disappointing as a lot of work was put into the new verifier
during this cycle. However, there are still too many known bugs and
incompatibilities. It is better to be faced with known broken behavior
than with new broken behavior and to switch now rather than via errata.
This way we have another cycle to iron out the kinks and to fix some of
the remaining bugs.
ok jsing
|
|
|
|
|
|
|
|
|
| |
For dynamically allocated verify parameters, param->name is only ever set
in X509_VERIFY_set1_name() where the old one is freed and the new one is
assigned via strdup(). Setting it to NULL without freeing it beforehand is
a leak.
looks correct to millert, ok inoguchi
|
| |
|
|
|
|
| |
ok bcook inoguchi jsing
|
|
|
|
| |
ok bcook inoguchi jsing
|
|
|
|
| |
ok bcook inoguchi jsing
|
|
|
|
| |
ok bcook inoguchi jsing
|
|
|
|
| |
Based on some text in OpenSSL 1.1.1's EVP_PKEY_new.pod.
|
|
|
|
|
|
|
| |
sebastia ran into this when attempting to update security/hcxtools.
This will be tested via wycheproof.go once the symbol is public.
ok jsing, tested by sebastia
|
|
|
|
|
|
|
| |
Instead of blindly skipping 14 characters, we can use the return
value of snprintf() to determine how much we should skip.
From Martin Vahlensieck with minor tweaks by me
|
|
|
|
|
|
|
| |
Found the hard way by lists y42 org via an OCSP validation failure that
in turn caused pkg_add over TLS to fail. Detailed report by sthen.
ok sthen
|
|
|
|
|
|
| |
x509v3_cache_extensions().
ok tb@
|
|
|
|
| |
suggested by jsing
|
|
|
|
| |
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x509_internal.h defines caps on the number of name constraints and
other names (such as subjectAltNames) that we want to allocate per
cert chain. These limits are checked too late. In a particularly
silly cert that jan found on ugos.ugm.ac.id 443, we ended up
allocating six times 2048 x509_constraint_name structures before
deciding that these are more than 512.
Fix this by adding a names_max member to x509_constraints_names which
is set on allocation against which each addition of a name is checked.
cluebat/ok jsing
ok inoguchi on earlier version
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
| |
If we're about to add a chain we have a trust path, so we have at least
one trusted certificate. This fixes a thinko from r1.31 and fixes the
openssl(1) cms verify test.
ok jsing (who had the same diff)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
To integrate the new X.509 verifier, X509_verify_cert() was refactored.
The code building chains in the legacy verifier was split into a
separate function. The first bug is that its return value was treated
as a Boolean although it wasn't. Second, the return alone is not enough
to decide whether to carry on the validation or not.
Slightly rearrange things to restore the behavior of the legacy verifier
prior to this refactoring.
Issue found and test case provided by Anton Borowka and jan.
ok jan jsing
|
|
|
|
| |
ok tb@
|
|
|
|
|
|
|
|
|
| |
In x509_verify_ctx_set_xsc_chain(), an ENOMEM case is currently passing
the last certificate and depth (which is no longer actually depth) to
x509_verify_cert_error(). Given we've hit an ENOMEM situation, neither
of these are useful so remove both.
ok tb@
|
|
|
|
|
|
| |
num_untrusted, but unfortunately it's public...
ok jsing tobhe
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
As should be obvious from the name and the comment in x509_vfy.h
int last_untrusted; /* index of last untrusted cert */
last_untrusted actually counts the number of untrusted certs at the
bottom of the chain.
Unfortunately, an earlier fix introducing x509_verify_set_xsc_chain()
assumed that last_untrusted actually meant the index of the last
untrusted cert in the chain, resulting in an off-by-one, which in turn
led to x509_vfy_check_chain_extension() skipping the check for the
EXFLAG_CRITICAL flag.
A second bug in x509_verify_set_xsc_chain() assumed that it is always
called with a trusted root, which is not necessarily the case anymore.
Address this with a temporary fix which will have to be revisited once
we will allow chains with more than one trusted cert.
Reported with a test case by tobhe.
ok jsing tobhe
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ok inoguchi
commit 6a51b9e1d0cf0bf8515f7201b68fb0a3482b3dc1
Author: Matt Caswell <matt@openssl.org>
Date: Tue Feb 2 17:17:23 2021 +0000
Don't overflow the output length in EVP_CipherUpdate calls
CVE-2021-23840
Reviewed-by: Paul Dale <pauli@openssl.org>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Notably this update removes various old Symantec roots (GeoTrust,
thawte, VeriSign) that were set in NSS to be distrusted on 1/1/2021.
Nobody should have been using these for years; only certain subCAs
signed by these were valid in NSS in that time due to an exemption:
https://wiki.mozilla.org/CA/Additional_Trust_Changes#Symantec
Notably Apple's "Apple IST CA 2 - G1" which is still in use for
some endpoints (it is cross signed by another CA too but these
endpoints are publishing the GeoTrust intermediate cert).
So for now I have skipped removal of "GeoTrust Global CA" to avoid
affecting these sites. Debian ran into this when they updated their
cert database and had to back this part out, affected sites are
not reachable on Android Firefox and maybe other newer Firefoxes.
Some sites that were affected have moved to a different CA in the
last few days but others, notably api.push.apple.com, remain
(I can only guess that there is a complicated problem involved,
possibly cert pinning on old devices - the clock is ticking though
as this expires in May 2022 anyway ;)
Additions:
/C=RO/O=CERTSIGN SA/OU=certSIGN ROOT CA G2
/C=HU/L=Budapest/O=Microsec Ltd./2.5.4.97=VATHU-23584497/CN=e-Szigno Root CA 2017
/C=KR/O=NAVER BUSINESS PLATFORM Corp./CN=NAVER Global Root Certification Authority
/C=US/ST=Illinois/L=Chicago/O=Trustwave Holdings, Inc./CN=Trustwave Global Certification Authority
/C=US/ST=Illinois/L=Chicago/O=Trustwave Holdings, Inc./CN=Trustwave Global ECC P256 Certification Authority
/C=US/ST=Illinois/L=Chicago/O=Trustwave Holdings, Inc./CN=Trustwave Global ECC P384 Certification Authority
Removals:
/C=US/O=GeoTrust Inc./CN=GeoTrust Primary Certification Authority
/C=US/O=GeoTrust Inc./CN=GeoTrust Universal CA
/C=US/O=GeoTrust Inc./CN=GeoTrust Universal CA 2
/C=US/O=GeoTrust Inc./OU=(c) 2008 GeoTrust Inc. - For authorized use only/CN=GeoTrust Primary Certification Authority - G3
/C=TW/O=Government Root Certification Authority
/C=LU/O=LuxTrust S.A./CN=LuxTrust Global Root 2
/C=US/O=thawte, Inc./OU=(c) 2007 thawte, Inc. - For authorized use only/CN=thawte Primary Root CA - G2
/C=US/O=thawte, Inc./OU=Certification Services Division/OU=(c) 2006 thawte, Inc. - For authorized use only/CN=thawte Primary Root CA
/C=US/O=thawte, Inc./OU=Certification Services Division/OU=(c) 2008 thawte, Inc. - For authorized use only/CN=thawte Primary Root CA - G3
/C=US/O=VeriSign, Inc./OU=VeriSign Trust Network/OU=(c) 1999 VeriSign, Inc. - For authorized use only/CN=VeriSign Class 3 Public Primary Certification Authority - G3
/C=US/O=VeriSign, Inc./OU=VeriSign Trust Network/OU=(c) 2006 VeriSign, Inc. - For authorized use only/CN=VeriSign Class 3 Public Primary Certification Authority - G5
/C=US/O=VeriSign, Inc./OU=VeriSign Trust Network/OU=(c) 2007 VeriSign, Inc. - For authorized use only/CN=VeriSign Class 3 Public Primary Certification Authority - G4
/C=CH/O=WISeKey/OU=Copyright (c) 2005/OU=OISTE Foundation Endorsed/CN=OISTE WISeKey Global Root GA CA
|
| |
|
|
|
|
|
|
|
|
|
| |
https://tools.ietf.org/html/draft-ietf-opsawg-finding-geofeeds describes
a mechanism to authenticate RFC 8805 Geofeed data files through the RPKI.
OpenSSL counterpart https://github.com/openssl/openssl/pull/14050
OK tb@ jsing@
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
RFC6482 - A Profile for Route Origin Authorizations (ROAs)
RFC6484 - Certificate Policy (CP) for the RPKI
RFC6493 - The RPKI Ghostbusters Record
RFC8182 - The RPKI Repository Delta Protocol (RRDP)
RFC8360 - RPKI Validation Reconsidered
draft-ietf-sidrops-rpki-rta - A profile for RTAs
Also in OpenSSL: https://github.com/openssl/openssl/commit/d3372c2f35495d0c61ab09daf7fba3ecbbb595aa
OK sthen@ tb@ jsing@
|
|
|
|
|
|
|
|
| |
Prior to calling the callback, ensure that the current (invalid and likely
incomplete) chain is set on the xsc. Some things (like auto chain) depend
on this functionality.
ok beck@
|
|
|
|
|
|
|
|
| |
x509_vfy and have an xsc. There's no point in finding more chains since that
API can not return them, and all we do is trigger buggy callbacks in
calling software.
ok jsing@
|
|
|
|
|
|
|
| |
this in the comments. helps avoid annoying situations with the legacy
callback
ok jsing@
|
|
|
|
|
|
| |
Yet another mostly meaningless error value...
Noted by and ok tb@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
When a certificate (namely a root) is specified as both a trusted and
untrusted certificate, the new verifier will find multiple chains - the
first being back to the trusted root certificate and a second via the root
that is untrusted, followed by the trusted root certificate. This situation
can be triggered by a server that (unnecessarily) includes the root
certificate in its certificate list.
While this validates correctly (using the first chain), it means that we
encounter a failure while building the second chain due to the root
certificate already being in the chain. When this occurs we call the verify
callback indicating a bad certificate. Some sensitive software (including
bacula and icinga), treat this single bad chain callback as terminal, even
though we successfully verify the certificate.
Avoid this problem by simply dumping the chain if we encounter a situation
where the certificate is already in the chain and also a trusted root -
we'll have already picked up the trusted root as a shorter path.
Issue with icinga2 initially reported by Theodore Wynnychenko.
Fix tested by sthen@ for both bacula and icinga2.
ok tb@
|
| |
|
|
|
|
| |
pointed out by jsing
|
|
|
|
|
|
|
|
|
|
|
| |
When parsing an UTCTime into a struct tm that wasn't cleared by the caller,
the years would be added to the already present value, which could give an
incorrect result. This is an issue in ASN1_UTCTIME_cmp_time_t(), which is
practically unused. Fix this by always zeroing the passed struct tm.
Issue reported by Olivier Taïbi, thanks!
ok jsing
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Comparing two GENERAL_NAME structures containing an EDIPARTYNAME can lead
to a crash. This enables a denial of service attack for an attacker who can
control both sides of the comparison.
Issue reported to OpenSSL on Nov 9 by David Benjamin.
OpenSSL shared the information with us on Dec 1st.
Fix from Matt Caswell (OpenSSL) with a few small tweaks.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Bad API design makes it possible to set an EC_KEY public key to
a point not on the curve. As a consequence, it was possible to
have bogus ECDSA signatures validated. In practice, all software
uses either EC_POINT_oct2point*() to unmarshal public keys or
issues a call to EC_KEY_check_key() after setting it. This way,
a point on curve check is performed and the problem is mitigated.
In OpenSSL commit 1e2012b7ff4a5f12273446b281775faa5c8a1858, Emilia
Kasper moved the point-on-curve check from EC_POINT_oct2point to
EC_POINT_set_affine_coordinates_*, which results in more checking.
In addition to this commit, we also check in the currently unused
codepath of a user set callback for setting compressed coordinates,
just in case this will be used at some point in the future.
The documentation of EC_KEY_check_key() is very vague on what it
checks and when checks are needed. It could certainly be improved
a lot. It's also strange that EC_KEY_set_key() performs no checks,
while EC_KEY_set_public_key_affine_coordinates() implicitly calls
EC_KEY_check_key().
It's a mess.
Issue found and reported by Guido Vranken who also tested an earlier
version of this fix.
ok jsing
|
| |
|
|
|
|
|
|
|
|
|
| |
This happens if name->der_len == 0. Since we already have a length
check, we can malloc and memcpy inside the conditional. This also
makes the code easier to read.
agreement from millert
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
x509_verify_chain_new() allocates a few members of a certificate chain:
an empty stack of certificates, a list of errors encountered while
validating the chain, and a list of name constraints. The function to
copy a chain would allocate a new chain using x509_verify_chain_new()
and then clobber its members by copies of the old chain. Fix this by
replacing x509_verify_chain_new() with calloc().
Found by review while investigating the report by Hanno Zysik who
found the same leak using valgrind. This is a cleaner version of
my initial fix from jsing.
ok jsing
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The legacy validator would only call x509_vfy_check_policy() once at
the very end after cobbling together a chain. Therefore it didn't
matter that X509_policy_check() always allocates a new tree on top of
the one that might have been passed in. This is in stark contrast to
other, similar APIs in this code base. The new validator calls this
function several times over while building its chains. This adds up
to a sizable leak in the new validator.
Reported with a reproducer by Hanno Zysik on github, who also bisected
this to the commit enabling the new validator.
Narrowed down to x509_vfy_check_policy() by jsing.
We simultaenously came up with a functionally identical fix.
ok jsing
|