Обсуждение: Force disable of SSL renegociation in the server
Hi all, Robert has mentioned https://nvd.nist.gov/vuln/detail/CVE-2021-3449 on the -security list where a TLS server could crash with some crafted renegociation message. We already disable SSL renegociation initialization for some time now, per 48d23c72, but we don't prevent the server from complying should the client wish to use renegociation. In terms of robustness and because SSL renegociation had its set of flaws and issues for many years, it looks like it would be a good idea to disable renegociation on the backend (not the client as that may be used with older access points where renegociation is still used, per an argument from Andres). In flavor, this is controlled in a way similar to SSL_OP_NO_COMPRESSION that we already enforce in the backend to disable SSL compression. However, there are a couple of compatibility tweaks regarding this one: - SSL_OP_NO_RENEGOTIATION controls that. It is present in OpenSSL >= 1.1.1 and has been backported in 1.1.0h (it is not present in older versions of 1.1.0). - In 1.0.2 and older versions, OpenSSL has an undocumented flag called SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS, able to do the same as far as I understand. Attached is a patch to use SSL_OP_NO_RENEGOTIATION if it exists, and force that in the backend. We could go further down, but using undocumented things looks rather unsafe here, to say the least. Could there be a point in backpatching that, in light of what we have done in 48d23c72 in the past, though? Thoughts? -- Michael
Вложения
> On 20 May 2021, at 13:00, Michael Paquier <michael@paquier.xyz> wrote: > - SSL_OP_NO_RENEGOTIATION controls that. It is present in OpenSSL >= > 1.1.1 and has been backported in 1.1.0h (it is not present in older > versions of 1.1.0). For OpenSSL 1.1.0 versions < 1.1.0h it will be silently accepted without actually doing anything, so we might want to combine it with the below. > - In 1.0.2 and older versions, OpenSSL has an undocumented flag called > SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS, able to do the same as far as I > understand. Well, it's documented in the changelog that it's undocumented (sigh..) along with a note stating that it works like SSL_OP_NO_RENEGOTIATION. Skimming the code it seems to ring true. For older OpenSSL versions there's also SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION which controls renegotiation for an older OpenSSL reneg bug. That applies to 0.9.8 versions which we don't support, but a malicious user can craft whatever they feel like so maybe we should ensure it's off as well? > Could there be a point in backpatching that, in light of what we have done in > 48d23c72 in the past, though? I think there is merit to that idea, especially given the precedent. > Thoughts? + /* disallow SSL renegociation, option available since 1.1.0h */ s/renegociation/renegotiation/ +1 on disabling renegotiation, but I think it's worth considering using SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS as well. One could also argue that extending the comment with a note that it only applies to TLSv1.2 and lower could be helpful to readers who aren't familiar with TLS protocol versions. TLSv1.3 did away with renegotiation. -- Daniel Gustafsson https://vmware.com/
On Thu, May 20, 2021 at 02:15:52PM +0200, Daniel Gustafsson wrote: > On 20 May 2021, at 13:00, Michael Paquier <michael@paquier.xyz> wrote: >> - SSL_OP_NO_RENEGOTIATION controls that. It is present in OpenSSL >= >> 1.1.1 and has been backported in 1.1.0h (it is not present in older >> versions of 1.1.0). > > For OpenSSL 1.1.0 versions < 1.1.0h it will be silently accepted without > actually doing anything, so we might want to combine it with the below. Yeah, still that stresses me quite a bit. OpenSSL does not have a good history with compatibility, and we are talking about something that does not officially exist on the map. >> - In 1.0.2 and older versions, OpenSSL has an undocumented flag called >> SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS, able to do the same as far as I >> understand. > > Well, it's documented in the changelog that it's undocumented (sigh..) along > with a note stating that it works like SSL_OP_NO_RENEGOTIATION. I'd say that this is still part of the definition of undocumented. There is no mention of it in their online documentation :) > Skimming the > code it seems to ring true. For older OpenSSL versions there's also > SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION which controls renegotiation for an > older OpenSSL reneg bug. That applies to 0.9.8 versions which we don't > support, but a malicious user can craft whatever they feel like so maybe we > should ensure it's off as well? If I am getting it right by reading upstream, SSL_OP_NO_RENEGOTIATION takes priority over that. Hence, if we force SSL_OP_NO_RENEGOTIATION, then SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION has no effect anyway. > + /* disallow SSL renegociation, option available since 1.1.0h */ > s/renegociation/renegotiation/ Argh, French-ism here. > +1 on disabling renegotiation, but I think it's worth considering using > SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS as well. This one can be set within ssl->s3->flags in the port information. Still that's not completely feasable either as some versions of OpenSSL hide the internals of a bunch of internal structures, and some distributions patch the upstream code? At the end of the day, I think that I would stick with simplicity and use SSL_OP_NO_RENEGOTIATION. It is not our job to go around any decision OpenSSL has poorly done either over the years. At least this part is officially documented :) > One could also argue that extending > the comment with a note that it only applies to TLSv1.2 and lower could be > helpful to readers who aren't familiar with TLS protocol versions. TLSv1.3 did > away with renegotiation. Good idea to mention that. -- Michael
Вложения
On Fri, May 21, 2021 at 10:41:34AM +0900, Michael Paquier wrote: > This one can be set within ssl->s3->flags in the port information. > Still that's not completely feasable either as some versions of > OpenSSL hide the internals of a bunch of internal structures, and some > distributions patch the upstream code? At the end of the day, I think > that I would stick with simplicity and use SSL_OP_NO_RENEGOTIATION. > It is not our job to go around any decision OpenSSL has poorly done > either over the years. At least this part is officially documented :) I got to look at that in details, and the attached would be able to do the job with OpenSSL 1.0.2 and older versions. The main idea is to set up SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS once the SSL object is created when opening the TLS connection to business. I have tested that down to 0.9.8 on all supported branches with the protocols we support (heads up to ssl_min_protocol_version here), and that looks to work as I'd expect. It is not a good idea to rely on OPENSSL_VERSION_NUMBER for such version checks as I am doing here, as we've been bitten with compatibility with LibreSSL in the past. So this had better use a check based on HAVE_OPENSSL_INIT_SSL to make sure that 1.1.0 is the version of OpenSSL used. Anyway, I really don't like using this undocumented option, and there is nothing that can be done with OpenSSL < 1.1.0h in the 1.1.0 series as the s3 part of the *SSL object gets hidden to the application, so it is not possible to set SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS there. And so, I would like to stick with a backpatch here, only for the part of the patch involving be_tls_init(). Full patch is attached for reference. While on it, I have added a comment about TLSv1.2 being the last protocol supporting renegotiation. Any objections? -- Michael
Вложения
> On 24 May 2021, at 03:29, Michael Paquier <michael@paquier.xyz> wrote: > I got to look at that in details, and the attached would be able to do > the job with OpenSSL 1.0.2 and older versions. The main idea is to > set up SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS once the SSL object is > created when opening the TLS connection to business. I have tested > that down to 0.9.8 on all supported branches with the protocols we > support (heads up to ssl_min_protocol_version here), and that looks to > work as I'd expect. > > It is not a good idea to rely on OPENSSL_VERSION_NUMBER for such > version checks as I am doing here, as we've been bitten with > compatibility with LibreSSL in the past. So this had better use a > check based on HAVE_OPENSSL_INIT_SSL to make sure that 1.1.0 is the > version of OpenSSL used. I agree that a capability based check is better than using the version numbers as their is a collision risk between distributions (and even within OpenSSL as NetBSD for example invented their own version etc). > Anyway, I really don't like using this undocumented option, and there is > nothing that can be done with OpenSSL < 1.1.0h in the 1.1.0 series as the s3 > part of the *SSL object gets hidden to the application, so it is not possible > to set SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS there. 1.1.0d killed what was left of SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS while keeping it defined, so there is also very little value in even attempting it there. +1 on the patch, LGTM. -- Daniel Gustafsson https://vmware.com/
On Mon, May 24, 2021 at 11:09:38AM +0200, Daniel Gustafsson wrote: > 1.1.0d killed what was left of SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS while keeping > it defined, so there is also very little value in even attempting it there. > > +1 on the patch, LGTM. Thanks, applied. I was having a very hard time putting a T instead of a C to renegotiation.. -- Michael