Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ssl: TLS 1.0 and 1.1 are enabled by default #8079

Closed
adrien-n opened this issue Jan 31, 2024 · 2 comments
Closed

ssl: TLS 1.0 and 1.1 are enabled by default #8079

adrien-n opened this issue Jan 31, 2024 · 2 comments
Assignees
Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS

Comments

@adrien-n
Copy link

I was going over the Ubuntu packages (in "main", not "universe"), looking for servers that still offer TLS 1.0 and 1.1 and noticed that rabbitmq does. It's not a default from rabbitmq; instead it's the behavior from ssl unless rabbitmq passes specific configuration. Note that I'm not well-versed in erlang and OTP so I may be missing obvious things but the main difficulty might be various parts of the code drifting apart.

Consider this from lib/ssl/src/ssl_internal.hrl:

%% sslv3 is considered insecure due to lack of padding check (Poodle attack)
%% Keep as interop with legacy software but do not support as default
%% tlsv1.0 and tlsv1.1 is now also considered legacy
%% tlsv1.3 is under development (experimental).
-define(ALL_AVAILABLE_VERSIONS, ['tlsv1.3', 'tlsv1.2', 'tlsv1.1', tlsv1]).
-define(ALL_AVAILABLE_DATAGRAM_VERSIONS, ['dtlsv1.2', dtlsv1]).
%% Defines the default versions when not specified by an ssl option.
-define(ALL_SUPPORTED_VERSIONS, ['tlsv1.3', 'tlsv1.2']).
-define(MIN_SUPPORTED_VERSIONS, ['tlsv1.1']).

The comment should be updated although I don't know if it states the TLS 1.1 protocol is under development, or its support in otp is.

In any case, the more important part is probably about TLS 1.3 and 1.2 being the default versions which doesn't match what I saw. I believe the issue lies in lib/ssl/src/ssl.erl:

ImplementedTLSVsns =  ?ALL_AVAILABLE_VERSIONS,

Replacing with the line that follows makes the behavior match my understanding of the comments above:

ImplementedTLSVsns =  ?ALL_SUPPORTED_VERSIONS,

After this change, rabbitmq offers only TLS 1.3 and 1.2 if not configured otherwise and can still be configured to offer TLS 1.1 and 1.0 too.

While effective and maybe appropriate as a distribution patch, this change is probably not going to be merged here because I'm not sure it matches the intent of the code. Except that I don't think I really understand the intent of the code very well: what do "supported" or "min_supported" mean? (well, I can find a possible explanation if the "min_supported" is only used as a bound and not included in "all_supported", but overall I'm lost)

By the way, in dtls_record.erl, the supported_protocol_versions function uses ?MIN_SUPPORTED_VERSIONS but I would have expected MIN_DATAGRAM_SUPPORTED_VERSIONS instead based on the names. Similarly, I can't tell if it's a bug or a construct I don't understand.

I saw this on OTP 25 but it is still the case in git master AFAICT.

Thanks

@adrien-n adrien-n added the bug Issue is reported as a bug label Jan 31, 2024
@u3s u3s added the team:PS Assigned to OTP team PS label Feb 1, 2024
@IngelaAndin
Copy link
Contributor

IngelaAndin commented Feb 2, 2024

If you execute ssl:versions() you will get

  • supported (the default if now configuration whatsoever is supplied)
  • available (what can be configured with the current linked crypto-lib support)
  • implemented (the versions of TLS that are implemented and thereby possible to configure if sufficient crypto-lib is linked)

The application will check and use application environment variable that can be set by node start parameters or by node configuration file, this however will always be overridden by explicit configuration in to ssl API functions. And the application variable will override the default.

In OTP-25 shell

Erlang/OTP 25 [erts-13.0.4] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit:ns]

Eshell V13.0.4  (abort with ^G)
1>  tls_record:supported_protocol_versions().
[{3,4},{3,3}]
2>  dtls_record:supported_protocol_versions().
[{254,253}]
3>

{3,4} = TLS-1.3 and {3,3} = TLS-1.2
{254,253} = DTLS-1.2

So it appears that the application variable has been set in your case if you have support for TLS-1.0 and TLS-1.1

?MIN_SUPPORTED_VERSIONS in dtls_record is however a bug , thank you for pointing it out. We will fix that.

@adrien-n
Copy link
Author

adrien-n commented Feb 2, 2024 via email

IngelaAndin added a commit that referenced this issue Feb 5, 2024
* ingela/ssl/dtls-default/GH-8079/OTP-18962:
  ssl: Correct default value
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:PS Assigned to OTP team PS
Projects
None yet
Development

No branches or pull requests

3 participants