A mirror of https://github.com/libressl/openbsd.git https://git.lua4.win/openbsd/atom?h=OPENBSD_6_9 2022-03-15T15:55:07+00:00 2022-03-15T15:55:07+00:00 tb 2022-03-15T15:55:07+00:00 urn:sha1:15f3857c4527a0091bfd5e9bf62cf9d16e9e42f3 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 2021-09-30T18:28:20+00:00 deraadt 2021-09-30T18:28:20+00:00 urn:sha1:83bc5cc5ee52fc0847ad86d9499c1ac8b9de9fe0 ok sthen, beck, jsing, tb, etc etc This cannot be issued as an errata/syspatch, because syspatch cannot 2021-09-30T18:25:43+00:00 deraadt 2021-09-30T18:25:43+00:00 urn:sha1:8e81e40d0c1296f1862d4a6749edd4cba53c4a23 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 2021-09-26T14:07:40+00:00 deraadt 2021-09-26T14:07:40+00:00 urn:sha1:35f8ef07a93e59616eb96dc41d4f8e6a21a6319b 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 2021-08-20T19:54:59+00:00 benno 2021-08-20T19:54:59+00:00 urn:sha1:3627e057ddf00759d5985e80a8bb317a1b071a4a 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 2021-04-15T14:15:03+00:00 tb 2021-04-15T14:15:03+00:00 urn:sha1:013c39e97f0af8342cdb560b4a2a45f87602f7b7 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 2021-04-05T07:02:50+00:00 tb 2021-04-05T07:02:50+00:00 urn:sha1:57e3ff55d71172acc1caf21e8c346e67b7089676 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 2021-03-31T17:02:18+00:00 tb 2021-03-31T17:02:18+00:00 urn:sha1:0c1b885ba6292c3b2d58d343977ac4cab728cc2b 2021-03-31T16:59:32+00:00 tb 2021-03-31T16:59:32+00:00 urn:sha1:2ebb1bafcf20e3417b35907fc27572bb0ff9faac ok bcook inoguchi jsing 2021-03-31T16:51:06+00:00 tb 2021-03-31T16:51:06+00:00 urn:sha1:a2c7dc9f61c905842b4ecaed7ee8beba13289e15 ok bcook inoguchi jsing