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