Return 0 if using default proto min/max (#1322)

While both AWS-LC/BoringSSL and OpenSSL allow `0` to be used in `SSL_CTX_set_min_proto_version` and `SSL_CTX_set_max_proto_version` to configure an `SSL_CTX` to use default TLS versions for its TLS "method", the two libraries differ in the behavior of their corresponding getters. When configured to use default versions, OpenSSL returns `0`:

- [OpenSSL 1.1.1 docs](https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_min_proto_version.html)
- [OpenSSL 3 docs](https://www.openssl.org/docs/manmaster/man3/SSL_CTX_set_min_proto_version.html)
- relevant code pointers [here](https://github.com/openssl/openssl/blob/b0e9d0370262ade64c55f2385fbb09ec6aa81e76/ssl/ssl_lib.c#L3894) and [here](https://github.com/openssl/openssl/blob/master/ssl/statem/statem_lib.c#L2055-L2058)

AWS-LC/BoringSSL, on the other hand, will return [the actual default value being used](https://github.com/aws/aws-lc/blob/main/include/openssl/ssl.h#L736-L740) instead of the `0` sentinel value. This PR modifies AWS-LC to conform with OpenSSL's behavior in this respect. I don't think OpenSSL's choice is generally preferable (how can applications tell which version will actually be used by default?), but this change will allow AWS-LC greater compatibility with applications that rely on AWS-LC's behavior.

include/openssl/ssl.h

@@ -375,9 +375,7 @@ OPENSSL_EXPORT int SSL_read(SSL *ssl, void *buf, int num);
 // SSL_read_ex reads up to |num| bytes from |ssl| into |buf|. It is similar to
 // |SSL_read|, but instead of returning the number of bytes read, it returns
 // 1 on success or 0 for failure. The number of bytes actually read is stored in
-// |read_bytes|. |SSL_read_ex| can be called with |num| set as 0, but will not read
-// application data from the peer.
-
+// |read_bytes|.
 //
 // This is only maintained for OpenSSL compatibility. Use |SSL_read| instead.
 OPENSSL_EXPORT int SSL_read_ex(SSL *ssl, void *buf, size_t num,
@@ -451,9 +449,7 @@ OPENSSL_EXPORT int SSL_write(SSL *ssl, const void *buf, int num);
 // SSL_write_ex writes up to |num| bytes from |buf| into |ssl|. It is similar to
 // |SSL_write|, but instead of returning the number of bytes written, it returns
 // 1 on success or 0 for failure. The number bytes actually written is stored in
-// |written|. |SSL_write_ex| can be called with |num| set as 0, but will not send
-// application data to the peer.
-
+// |written|.
 //
 // This is only maintained for OpenSSL compatibility. Use |SSL_write| instead.
 OPENSSL_EXPORT int SSL_write_ex(SSL *s, const void *buf, size_t num,
@@ -733,10 +729,12 @@ OPENSSL_EXPORT int SSL_CTX_set_min_proto_version(SSL_CTX *ctx,
 OPENSSL_EXPORT int SSL_CTX_set_max_proto_version(SSL_CTX *ctx,
                                                  uint16_t version);
 
-// SSL_CTX_get_min_proto_version returns the minimum protocol version for |ctx|
+// SSL_CTX_get_min_proto_version returns the minimum protocol version for |ctx|.
+// If |ctx| is configured to use the default minimum version, 0 is returned.
 OPENSSL_EXPORT uint16_t SSL_CTX_get_min_proto_version(const SSL_CTX *ctx);
 
-// SSL_CTX_get_max_proto_version returns the maximum protocol version for |ctx|
+// SSL_CTX_get_max_proto_version returns the maximum protocol version for |ctx|.
+// If |ctx| is configured to use the default maximum version, 0 is returned.
 OPENSSL_EXPORT uint16_t SSL_CTX_get_max_proto_version(const SSL_CTX *ctx);
 
 // SSL_set_min_proto_version sets the minimum protocol version for |ssl| to
@@ -750,11 +748,13 @@ OPENSSL_EXPORT int SSL_set_min_proto_version(SSL *ssl, uint16_t version);
 OPENSSL_EXPORT int SSL_set_max_proto_version(SSL *ssl, uint16_t version);
 
 // SSL_get_min_proto_version returns the minimum protocol version for |ssl|. If
-// the connection's configuration has been shed, 0 is returned.
+// the connection's configuration has been shed or |ssl| is configured to use
+// the default min version, 0 is returned.
 OPENSSL_EXPORT uint16_t SSL_get_min_proto_version(const SSL *ssl);
 
 // SSL_get_max_proto_version returns the maximum protocol version for |ssl|. If
-// the connection's configuration has been shed, 0 is returned.
+// the connection's configuration has been shed or |ssl| is configured to use
+// the default max version, 0 is returned.
 OPENSSL_EXPORT uint16_t SSL_get_max_proto_version(const SSL *ssl);
 
 // SSL_version returns the TLS or DTLS protocol version used by |ssl|, which is

ssl/internal.h

@@ -3303,6 +3303,20 @@ struct SSL_CONFIG {
   // of support for AES hw. The value is only considered if |aes_hw_override| is
   // true.
   bool aes_hw_override_value : 1;
+
+  // conf_max_version_use_default indicates whether the |SSL_CONFIG| is configured
+  // to use the default maximum protocol version for the relevant protocol
+  // method. By default, |SSL_new| will set this to true and connections will use
+  // the default max version. callers can change the max version used by calling
+  // |SSL_set_max_proto_version| with a non-zero value.
+  bool conf_max_version_use_default;
+
+  // conf_min_version_use_default indicates whether the |SSL_CONFIG| is configured
+  // to use the default minimum protocol version for the relevant protocol
+  // method. By default, |SSL_new| will set this to true and connections will use
+  // the default min version. callers can change the min version used by calling
+  // |SSL_set_min_proto_version| with a non-zero value.
+  bool conf_min_version_use_default;
 };
 
 // From RFC 8446, used in determining PSK modes.
@@ -3971,6 +3985,20 @@ struct ssl_ctx_st {
   // |aes_hw_override| is true.
   bool aes_hw_override_value : 1;
 
+  // conf_max_version_use_default indicates whether the |SSL_CTX| is configured
+  // to use the default maximum protocol version for the relevant protocol
+  // method. By default, |SSL_CTX_new| will set this to true and connections will
+  // use the default max version. callers can change the max version used by calling
+  // |SSL_CTX_set_max_proto_version| with a non-zero value.
+  bool conf_max_version_use_default;
+
+  // conf_min_version_use_default indicates whether the |SSL_CTX| is configured
+  // to use the default minimum protocol version for the relevant protocol
+  // method. By default, |SSL_CTX_new| will set this to true and connections will
+  // use the default min version. callers can change the min version used by calling
+  // |SSL_CTX_set_min_proto_version| with a non-zero value.
+  bool conf_min_version_use_default;
+
  private:
   ~ssl_ctx_st();
   friend OPENSSL_EXPORT void SSL_CTX_free(SSL_CTX *);

ssl/ssl_lib.cc

@@ -591,6 +591,13 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *method) {
     return nullptr;
   }
 
+  // By default, use the method's default min/max version. Make sure to set
+  // this after calls to |SSL_CTX_set_{min,max}_proto_version| because those
+  // calls modify these values if |method->version| is not 0. We should still
+  // defer to the protocol method's default min/max values in that case.
+  ret->conf_max_version_use_default = true;
+  ret->conf_min_version_use_default = true;
+
   return ret.release();
 }
 
@@ -650,6 +657,8 @@ SSL *SSL_new(SSL_CTX *ctx) {
   }
   ssl->config->conf_min_version = ctx->conf_min_version;
   ssl->config->conf_max_version = ctx->conf_max_version;
+  ssl->config->conf_min_version_use_default = ctx->conf_min_version_use_default;
+  ssl->config->conf_max_version_use_default = ctx->conf_max_version_use_default;
 
   ssl->config->cert = ssl_cert_dup(ctx->cert.get());
   if (ssl->config->cert == nullptr) {
@@ -1026,10 +1035,6 @@ static int ssl_read_impl(SSL *ssl) {
 }
 
 int SSL_read_ex(SSL *ssl, void *buf, size_t num, size_t *read_bytes) {
-  if (num == 0 && read_bytes != nullptr) {
-    *read_bytes = 0;
-    return 1;
-  }
   int ret = SSL_read(ssl, buf, (int)num);
   if (ret <= 0) {
     return 0;
@@ -1125,10 +1130,6 @@ int SSL_write(SSL *ssl, const void *buf, int num) {
 }
 
 int SSL_write_ex(SSL *ssl, const void *buf, size_t num, size_t *written) {
-  if (num == 0 && written != nullptr) {
-    *written = 0;
-    return 1;
-  }
   int ret = SSL_write(ssl, buf, (int)num);
   if (ret <= 0) {
     return 0;

ssl/ssl_test.cc

@@ -1667,8 +1667,21 @@ static void ExpectDefaultVersion(uint16_t min_version, uint16_t max_version,
                                  const SSL_METHOD *(*method)(void)) {
   bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(method()));
   ASSERT_TRUE(ctx);
+  bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get()));
+  ASSERT_TRUE(ssl);
+  EXPECT_EQ(0, SSL_CTX_get_min_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_CTX_get_max_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_get_min_proto_version(ssl.get()));
+  EXPECT_EQ(0, SSL_get_max_proto_version(ssl.get()));
+
+  EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), min_version));
+  EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), max_version));
   EXPECT_EQ(min_version, SSL_CTX_get_min_proto_version(ctx.get()));
   EXPECT_EQ(max_version, SSL_CTX_get_max_proto_version(ctx.get()));
+  EXPECT_TRUE(SSL_set_min_proto_version(ssl.get(), min_version));
+  EXPECT_TRUE(SSL_set_max_proto_version(ssl.get(), max_version));
+  EXPECT_EQ(min_version, SSL_get_min_proto_version(ssl.get()));
+  EXPECT_EQ(max_version, SSL_get_max_proto_version(ssl.get()));
 }
 
 TEST(SSLTest, DefaultVersion) {
@@ -4619,6 +4632,8 @@ TEST(SSLTest, EarlyCallbackVersionSwitch) {
 TEST(SSLTest, SetVersion) {
   bssl::UniquePtr<SSL_CTX> ctx(SSL_CTX_new(TLS_method()));
   ASSERT_TRUE(ctx);
+  bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get()));
+  ASSERT_TRUE(ssl);
 
   // Set valid TLS versions.
   for (const auto &vers : kAllVersions) {
@@ -4628,6 +4643,10 @@ TEST(SSLTest, SetVersion) {
       EXPECT_EQ(SSL_CTX_get_max_proto_version(ctx.get()), vers.version);
       EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), vers.version));
       EXPECT_EQ(SSL_CTX_get_min_proto_version(ctx.get()), vers.version);
+      EXPECT_TRUE(SSL_set_max_proto_version(ssl.get(), vers.version));
+      EXPECT_EQ(SSL_get_max_proto_version(ssl.get()), vers.version);
+      EXPECT_TRUE(SSL_set_min_proto_version(ssl.get(), vers.version));
+      EXPECT_EQ(SSL_get_min_proto_version(ssl.get()), vers.version);
     }
   }
 
@@ -4638,18 +4657,30 @@ TEST(SSLTest, SetVersion) {
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), DTLS1_VERSION));
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), 0x0200));
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), 0x1234));
+  EXPECT_FALSE(SSL_set_max_proto_version(ssl.get(), 0x0200));
+  EXPECT_FALSE(SSL_set_max_proto_version(ssl.get(), 0x1234));
+  EXPECT_FALSE(SSL_set_min_proto_version(ssl.get(), DTLS1_VERSION));
+  EXPECT_FALSE(SSL_set_min_proto_version(ssl.get(), 0x0200));
+  EXPECT_FALSE(SSL_set_min_proto_version(ssl.get(), 0x1234));
 
-  // Zero is the default version.
+  // Zero represents the default version.
   EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), 0));
-  EXPECT_EQ(TLS1_3_VERSION, SSL_CTX_get_max_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_CTX_get_max_proto_version(ctx.get()));
   EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), 0));
-  EXPECT_EQ(TLS1_VERSION, SSL_CTX_get_min_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_CTX_get_min_proto_version(ctx.get()));
+  EXPECT_TRUE(SSL_set_max_proto_version(ssl.get(), 0));
+  EXPECT_EQ(0, SSL_get_max_proto_version(ssl.get()));
+  EXPECT_TRUE(SSL_set_min_proto_version(ssl.get(), 0));
+  EXPECT_EQ(0, SSL_get_min_proto_version(ssl.get()));
 
   // SSL 3.0 is not available.
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), SSL3_VERSION));
+  EXPECT_FALSE(SSL_set_min_proto_version(ssl.get(), SSL3_VERSION));
 
   ctx.reset(SSL_CTX_new(DTLS_method()));
   ASSERT_TRUE(ctx);
+  ssl.reset(SSL_new(ctx.get()));
+  ASSERT_TRUE(ssl);
 
   // Set valid DTLS versions.
   for (const auto &vers : kAllVersions) {
@@ -4672,11 +4703,11 @@ TEST(SSLTest, SetVersion) {
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), 0xfffe /* DTLS 0.1 */));
   EXPECT_FALSE(SSL_CTX_set_min_proto_version(ctx.get(), 0x1234));
 
-  // Zero is the default version.
+  // Zero represents the default version.
   EXPECT_TRUE(SSL_CTX_set_max_proto_version(ctx.get(), 0));
-  EXPECT_EQ(DTLS1_2_VERSION, SSL_CTX_get_max_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_CTX_get_max_proto_version(ctx.get()));
   EXPECT_TRUE(SSL_CTX_set_min_proto_version(ctx.get(), 0));
-  EXPECT_EQ(DTLS1_VERSION, SSL_CTX_get_min_proto_version(ctx.get()));
+  EXPECT_EQ(0, SSL_CTX_get_min_proto_version(ctx.get()));
 }
 
 static const char *GetVersionName(uint16_t version) {
@@ -6510,18 +6541,6 @@ TEST_P(SSLVersionTest, SSLPendingEx) {
   ASSERT_EQ(buf_len, (size_t)2);
   EXPECT_EQ(3, SSL_pending(client_.get()));
   EXPECT_EQ(1, SSL_has_pending(client_.get()));
-
-  // 0-sized IO with valid inputs should succeed but not read/write nor effect
-  // buffer state. However, NULL |read_bytes|/|written| pointer should fail.
-  const int client_pending = SSL_pending(client_.get());
-  ASSERT_EQ(1, SSL_read_ex(client_.get(), (void *)"", 0, &buf_len));
-  ASSERT_EQ(0UL, buf_len);
-  ASSERT_EQ(client_pending, SSL_pending(client_.get()));
-  ASSERT_EQ(1, SSL_write_ex(client_.get(), (void *)"", 0, &buf_len));
-  ASSERT_EQ(0UL, buf_len);
-  ASSERT_EQ(client_pending, SSL_pending(client_.get()));
-  ASSERT_EQ(0, SSL_read_ex(client_.get(), (void *)"", 0, nullptr));
-  ASSERT_EQ(0, SSL_write_ex(client_.get(), (void *)"", 0, nullptr));
 }
 
 // Test that post-handshake tickets consumed by |SSL_shutdown| are ignored.

ssl/ssl_transfer_asn1.cc

@@ -930,6 +930,12 @@ static const unsigned kSSLConfigOcspStaplingEnabledTag =
 static const unsigned kSSLConfigJdk11WorkaroundTag =
     CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 1;
 
+static const unsigned kSSLConfigConfMaxVersionUseDefault =
+    CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 2;
+
+static const unsigned kSSLConfigConfMinVersionUseDefault =
+    CBS_ASN1_CONSTRUCTED | CBS_ASN1_CONTEXT_SPECIFIC | 3;
+
 // *** EXPERIMENTAL — DO NOT USE WITHOUT CHECKING ***
 // These SSL_CONFIG serialization functions are developed to support SSL
 // transfer. Most fields of SSL_CONFIG are not used after handshake completes.
@@ -944,7 +950,9 @@ static const unsigned kSSLConfigJdk11WorkaroundTag =
 //    version                           INTEGER (1),  -- SSL_CONFIG structure
 //    version confMaxVersion                    INTEGER, confMinVersion INTEGER,
 //    ocspStaplingEnabled               [0] BOOLEAN OPTIONAL,
-//    jdk11Workaround                   [1] BOOLEAN OPTIONAL
+//    jdk11Workaround                   [1] BOOLEAN OPTIONAL,
+//    confMaxVersionUseDefault          [2] BOOLEAN OPTIONAL,
+//    confMinVersionUseDefault          [3] BOOLEAN OPTIONAL
 // }
 static int SSL_CONFIG_to_bytes(SSL_CONFIG *in, CBB *cbb) {
   if (in == NULL || cbb == NULL) {
@@ -971,12 +979,25 @@ static int SSL_CONFIG_to_bytes(SSL_CONFIG *in, CBB *cbb) {
       return 0;
     }
   }
+  if (in->conf_max_version_use_default) {
+    if (!CBB_add_asn1(&config, &child, kSSLConfigConfMaxVersionUseDefault) ||
+        !CBB_add_asn1_bool(&child, true)) {
+      return 0;
+    }
+  }
+  if (in->conf_min_version_use_default) {
+    if (!CBB_add_asn1(&config, &child, kSSLConfigConfMinVersionUseDefault) ||
+        !CBB_add_asn1_bool(&child, true)) {
+      return 0;
+    }
+  }
   return CBB_flush(cbb);
 }
 
 static int SSL_CONFIG_from_bytes(SSL_CONFIG *out, CBS *cbs) {
   CBS config;
   int ocsp_stapling_enabled, jdk11_workaround;
+  int conf_max_version_use_default, conf_min_version_use_default;
   uint64_t version, conf_max_version, conf_min_version;
   if (!CBS_get_asn1(cbs, &config, CBS_ASN1_SEQUENCE) ||
       !CBS_get_asn1_uint64(&config, &version) || version != kSSLConfigVersion ||
@@ -988,12 +1009,20 @@ static int SSL_CONFIG_from_bytes(SSL_CONFIG *out, CBS *cbs) {
       !CBS_get_optional_asn1_bool(&config, &jdk11_workaround,
                                   kSSLConfigJdk11WorkaroundTag,
                                   0 /* default to false */) ||
+      !CBS_get_optional_asn1_bool(&config, &conf_max_version_use_default,
+                                  kSSLConfigConfMaxVersionUseDefault,
+                                  1 /* default to true */) ||
+      !CBS_get_optional_asn1_bool(&config, &conf_min_version_use_default,
+                                  kSSLConfigConfMinVersionUseDefault,
+                                  1 /* default to true */) ||
       CBS_len(&config) != 0) {
     OPENSSL_PUT_ERROR(SSL, SSL_R_SERIALIZATION_INVALID_SSL_CONFIG);
     return 0;
   }
   out->conf_max_version = conf_max_version;
   out->conf_min_version = conf_min_version;
+  out->conf_max_version_use_default = !!conf_max_version_use_default;
+  out->conf_min_version_use_default = !!conf_min_version_use_default;
   out->ocsp_stapling_enabled = !!ocsp_stapling_enabled;
   out->jdk11_workaround = !!jdk11_workaround;
   // handoff will always be the normal state(false) after handshake completes.

ssl/ssl_versions.cc

@@ -335,44 +335,54 @@ BSSL_NAMESPACE_END
 using namespace bssl;
 
 int SSL_CTX_set_min_proto_version(SSL_CTX *ctx, uint16_t version) {
+  ctx->conf_min_version_use_default = (version == 0);
   return set_min_version(ctx->method, &ctx->conf_min_version, version);
 }
 
 int SSL_CTX_set_max_proto_version(SSL_CTX *ctx, uint16_t version) {
+  ctx->conf_max_version_use_default = (version == 0);
   return set_max_version(ctx->method, &ctx->conf_max_version, version);
 }
 
 uint16_t SSL_CTX_get_min_proto_version(const SSL_CTX *ctx) {
+  if (ctx->conf_min_version_use_default) {
+    return 0;
+  }
   return ctx->conf_min_version;
 }
 
 uint16_t SSL_CTX_get_max_proto_version(const SSL_CTX *ctx) {
+  if (ctx->conf_max_version_use_default) {
+    return 0;
+  }
   return ctx->conf_max_version;
 }
 
 int SSL_set_min_proto_version(SSL *ssl, uint16_t version) {
   if (!ssl->config) {
     return 0;
   }
+  ssl->config->conf_min_version_use_default = (version == 0);
   return set_min_version(ssl->method, &ssl->config->conf_min_version, version);
 }
 
 int SSL_set_max_proto_version(SSL *ssl, uint16_t version) {
   if (!ssl->config) {
     return 0;
   }
+  ssl->config->conf_max_version_use_default = (version == 0);
   return set_max_version(ssl->method, &ssl->config->conf_max_version, version);
 }
 
 uint16_t SSL_get_min_proto_version(const SSL *ssl) {
-  if (!ssl->config) {
+  if (!ssl->config || ssl->config->conf_min_version_use_default) {
     return 0;
   }
   return ssl->config->conf_min_version;
 }
 
 uint16_t SSL_get_max_proto_version(const SSL *ssl) {
-  if (!ssl->config) {
+  if (!ssl->config || ssl->config->conf_max_version_use_default) {
     return 0;
   }
   return ssl->config->conf_max_version;

ssl/tls_transfer.asn

@@ -126,7 +126,9 @@ SSLConfig ::= SEQUENCE {
     confMaxVersion INTEGER,
     confMinVersion INTEGER,
     ocspStaplingEnabled [0] BOOLEAN OPTIONAL,
-    jdk11Workaround [1] BOOLEAN OPTIONAL
+    jdk11Workaround [1] BOOLEAN OPTIONAL,
+    confMaxVersionUseDefault [2] BOOLEAN OPTIONAL,
+    confMinVersionUseDefault [3] BOOLEAN OPTIONAL
 }
 
 END