libressl / openbsd

Source code pulled from OpenBSD for LibreSSL - this includes most of the library and supporting code. The place to contribute to this code is via the OpenBSD CVS tree. Please mail patches to tech@openbsd.org, instead of submitting pull requests, since this tree is often rebased.
231 stars 92 forks source link

!tlsv1.0 and !tlsv1.1 disables tlsv1.2 in `tls_config_parse_protocols` #151

Closed nak3 closed 2 months ago

nak3 commented 2 months ago

tls_config_parse_protocols() assigns tlsv1.0 and tlsv1.1 to tlsv1.2 as:

https://github.com/libressl/openbsd/blob/9b1a061e926ab1b3854ed7128026a870600937fd/src/lib/libtls/tls_config.c#L263-L266

This makes sense for the preparation to deprecate TLS v1.0 and 1.1. However, it is confusing when we set ! like !tlsv1.1 because it disables tlsv1.2 eventually.

Some test case verify the behavior, so it is expected? For example, the following test case does not enable tlsv1.2.

https://github.com/libressl/openbsd/blob/9b1a061e926ab1b3854ed7128026a870600937fd/src/regress/lib/libtls/config/configtest.c#L91-L95

With said, it makes the following results and I think it is confusing:

nak3 commented 2 months ago

I think !tlsv1.0 and !tlsv1.1 should not flip tlsv1.2 like the patch below. However, this might disrupt the current users' configurations, so should we leave it as it is?

diff --git a/src/lib/libtls/tls_config.c b/src/lib/libtls/tls_config.c
index 10dc500..fa44698 100644
--- a/src/lib/libtls/tls_config.c
+++ b/src/lib/libtls/tls_config.c
@@ -260,11 +260,13 @@ tls_config_parse_protocols(uint32_t *protocols, const char *protostr)
            proto = TLS_PROTOCOLS_DEFAULT;
        if (strcasecmp(p, "tlsv1") == 0)
            proto = TLS_PROTOCOL_TLSv1;
-       else if (strcasecmp(p, "tlsv1.0") == 0)
+       else if (strcasecmp(p, "tlsv1.0") == 0) {
            proto = TLS_PROTOCOL_TLSv1_2;
-       else if (strcasecmp(p, "tlsv1.1") == 0)
+           negate = 0; // !tlsv1.0 should not flip v1.2.
+       } else if (strcasecmp(p, "tlsv1.1") == 0) {
            proto = TLS_PROTOCOL_TLSv1_2;
-       else if (strcasecmp(p, "tlsv1.2") == 0)
+           negate = 0; // !tlsv1.1 should not flip v1.2.
+       } else if (strcasecmp(p, "tlsv1.2") == 0)
            proto = TLS_PROTOCOL_TLSv1_2;
        else if (strcasecmp(p, "tlsv1.3") == 0)
            proto = TLS_PROTOCOL_TLSv1_3;
diff --git a/src/regress/lib/libtls/config/configtest.c b/src/regress/lib/libtls/config/configtest.c
index 5af5b56..03abc68 100644
--- a/src/regress/lib/libtls/config/configtest.c
+++ b/src/regress/lib/libtls/config/configtest.c
@@ -91,7 +91,7 @@ struct parse_protocols_test parse_protocols_tests[] = {
    {
        .protostr = "tlsv1.1,tlsv1.2,!tlsv1.1",
        .want_return = 0,
-       .want_protocols = 0,
+       .want_protocols = TLS_PROTOCOL_TLSv1_2,
    },
    {
        .protostr = "unknown",
@@ -111,17 +111,17 @@ struct parse_protocols_test parse_protocols_tests[] = {
    {
        .protostr = "all,!tlsv1.0",
        .want_return = 0,
-       .want_protocols = TLS_PROTOCOL_TLSv1_3,
+       .want_protocols = TLS_PROTOCOL_TLSv1_2 | TLS_PROTOCOL_TLSv1_3,
    },
    {
        .protostr = "!tlsv1.0",
        .want_return = 0,
-       .want_protocols = TLS_PROTOCOL_TLSv1_3,
+       .want_protocols = TLS_PROTOCOL_TLSv1_2 | TLS_PROTOCOL_TLSv1_3,
    },
    {
        .protostr = "!tlsv1.0,!tlsv1.1,!tlsv1.3",
        .want_return = 0,
-       .want_protocols = 0,
+       .want_protocols = TLS_PROTOCOL_TLSv1_2,
    },
    {
        .protostr = "!tlsv1.0,!tlsv1.1,tlsv1.2,!tlsv1.3",