A mirror of https://github.com/libressl/openbsd.git https://git.lua4.win/openbsd/atom?h=OPENBSD_7_9 2025-06-09T10:14:38+00:00 2025-06-09T10:14:38+00:00 tb 2025-06-09T10:14:38+00:00 urn:sha1:cce0557a3d1826b9dc413a714eaa4c844b74a497 reported by smatch via jsg ok beck 2025-06-02T12:18:22+00:00 jsg 2025-06-02T12:18:22+00:00 urn:sha1:0a9ae15841c7f994ec48b5317b89dbfe586ddcc7 found with smatch, ok tb@ 2025-03-12T14:03:55+00:00 jsing 2025-03-12T14:03:55+00:00 urn:sha1:b150ad681869b78ec3662f92df947a5790b32862 In January 2017 we added SSL_OP_NO_CLIENT_RENEGOTIATION, which results in a SSL_AD_NO_RENEGOTIATION fatal alert if a ClientHello message is seen on an active connection (client initiated renegotation). Then in May 2017 OpenSSL added SSL_OP_NO_RENEGOTIATION, which results in a SSL_AD_NO_RENEGOTIATION warning alert if a server receives a ClientHello on an active connection (client initiated renegotation), or a client receives a HelloRequest (server requested renegotation). This option also causes calls to SSL_renegotiate() and SSL_renegotiate_abbreviated() to fail. Then in 2021, OpenSSL also added SSL_OP_ALLOW_CLIENT_RENEGOTIATION, which trumps SSL_OP_NO_RENEGOTIATION but only for incoming ClientHello messages (apparently unsetting SSL_OP_NO_RENEGOTIATION is too hard). Provide SSL_OP_NO_RENEGOTIATION and SSL_OP_ALLOW_CLIENT_RENEGOTIATION, primarily to make life easier for ports. If SSL_OP_NO_CLIENT_RENEGOTIATION is set it will take precedence and render SSL_OP_ALLOW_CLIENT_RENEGOTIATION ineffective. The rest of the behaviour should match OpenSSL, with the exception of ClientHellos triggering fatal alerts instead of warnings. ok tb@ 2024-09-22T14:59:48+00:00 tb 2024-09-22T14:59:48+00:00 urn:sha1:e58cba35ab15d6597f0c9cd8d6fba1928ade3acf From Kenjiro Nakayama Closes https://github.com/libressl/portable/issues/1097 2024-08-03T04:50:27+00:00 tb 2024-08-03T04:50:27+00:00 urn:sha1:7ffee9d08a91191b5a4fb21336efef092b583c3e SSL_CTX_set_cert_store() should have been called SSL_CTX_set0_cert_store() since it takes ownership of the store argument. Apparently a few people ran into the issue of not bumping the refcount themselves, leading to use after frees about 10 years ago. This is a quite rarely used API and there are no misuses in the ports tree, but since someone did the work of writing a diff, we can still add it. Needless to say that SSL_CTX_get_cert_store() obviously has the exact same issue and nobody seems to have thought of adding a get0 or get1 version to match... Fixes https://github.com/libressl/openbsd/issues/71 From Kenjiro Nakayama 2024-07-20T04:04:23+00:00 jsing 2024-07-20T04:04:23+00:00 urn:sha1:026ea65c83ed46dcfd89ada1f6250daa4fcc01b3 For a long time SSL_SESSION has had both a cipher ID and a pointer to an SSL_CIPHER (and not both are guaranteed to be populated). There is also a pointer to an SSL_CIPHER in the SSL_HANDSHAKE that denotes the cipher being used for this connection. Some code has been using the cipher from SSL_SESSION and some code has been using the cipher from SSL_HANDSHAKE. Remove cipher from SSL_SESSION and use the version in SSL_HANDSHAKE everywhere. If resuming from a session then we need to use the SSL_SESSION cipher ID to set the SSL_HANDSHAKE cipher. And we still need to ensure that we update the cipher ID in the SSL_SESSION whenever the SSL_HANDSHAKE cipher changes (this only occurs in a few places). ok tb@ 2024-07-19T08:54:31+00:00 jsing 2024-07-19T08:54:31+00:00 urn:sha1:f5a7be1aa230b5dad73001a01843e188c2475c6a SSL_SESSION has a 'ciphers' member which contains a list of ciphers that were advertised by the client. Move this from SSL_SESSION to SSL_HANDSHAKE and rename it to match reality. ok tb@ 2024-07-11T13:48:52+00:00 tb 2024-07-11T13:48:52+00:00 urn:sha1:a5c7700cf78a06c8fe14f0f35da3def39cc6f10d SSL_select_next_poto() was written with NPN in mind. NPN has a weird fallback mechanism which is baked into the API. This is makes no sense for ALPN, where the API behavior is undesirable since it a server should not end up choosing a protocol it doesn't (want to) support. Arguably, ALPN should simply have had its own API for protocol selection supporting the proper semantics, instead of shoehorning an NPN API into working for ALPN. Commit https://boringssl-review.googlesource.com/c/boringssl/+/17206/ renamed the arguments to work for both NPN and ALPN, with the slight downside of honoring client preference instead of the SHOULD in RFC 7301, section 3.2. This grates for most consumers in the wild, but so be it. The behavior is saner and safer. discussed with davidben ok beck 2024-06-29T07:34:12+00:00 tb 2024-06-29T07:34:12+00:00 urn:sha1:a0a4254e1ee48ea9b6a2489779f3f697c7f3fcae 2024-06-28T14:46:19+00:00 tb 2024-06-28T14:46:19+00:00 urn:sha1:30515f1008f9f0aec1133dd2814ec607bbd5dc52 SSL_select_next_proto() is already quite broken by its design: const in, non-const out, with the intention of pointing somewhere inside of the two input pointers. A length returned in an unsigned char (because, you know, the individual protocols are encoded in Pascal strings). Can't signal uailure either. It also has an unreachable public return code. Also, due to originally catering to NPN, this function opportunistically selects a protocol from the second input (client) parameters, which makes little sense for ALPN since that means the server falls back to a protocol it doesn't (want to) support. If there's no overlap, it's the callback's job to signal error to its caller for ALPN. As if that wasn't enough misdesign and bugs, the one we're concerned with here wasn't reported to us twice in ten years is that if you pass this API a zero-length (or a sufficiently malformed client protocol list), it would return a pointer pointing somewhere into the heap instead into one of the two input pointers. This pointer could then be interpreted as a Pascal string, resulting in an information disclosure of up to 255 bytes from the heap to the peer, or a crash. This can only happen for NPN (where it does happen in old python and node). A long time ago jsing removed NPN support from LibreSSL, because it had an utter garbage implementation and because it was practically unused. First it was already replaced by the somewhat less bad ALPN, and the only users were the always same language bindings that tend to use every feature they shouldn't use. There were a lot of complaints due to failing test cases in there, but in the end the decision turned out to be the right one: the consequence is that LibreSSL isn't vulnerable to CVE-2024-5535. Still, there is a bug here to fix. It is completely straightforward to do so. Rewrite this mess using CBS, preserving the current behavior. Also, we do not follow BoringSSL's renaming of the variables. It would result in confusing code in almost all alpn callbacks I've seen in the wild. The only exception is the accidental example of Qt. ok jsing