Reject inverted protocol version range in SSLContext (#142)

OpenSSL validates each boundary individually but does not cross-check
min against max. A caller could set min above max without raising,
producing a context where every session fails its handshake.

Both setters now compare the new value against the existing opposite
boundary before calling SSL_CTX_ctrl, so the context is never left
with an inverted range. SSLAutoVersion (0) bypasses the check, since
a zero boundary means the library picks.

Closes #129

Author: SeanTAllen

.release-notes/fix-inverted-proto-version-range.md

@@ -0,0 +1,3 @@
+## Fix SSLContext accepting an inverted protocol version range
+
+`set_min_proto_version(TLS1u3Version())` followed by `set_max_proto_version(TLS1u2Version())` passed without raising. Every session from that context then failed its handshake with "no protocols available." Both setters now raise when the new value would leave the minimum above the maximum.

ssl/net/_test.pony

@@ -60,6 +60,8 @@ actor \nodoc\ Main is TestList
     test(_TestSSLContextALPNResolverRootedBySession)
     test(_TestSSLContextALPNResolverUnreferenced)
     test(_TestSSLContextALPNSetClientProtocolsAfterDispose)
+    test(_TestSSLContextSetMinProtoVersionInvertedRange)
+    test(_TestSSLContextSetMaxProtoVersionInvertedRange)
     test(_TestSSLContextSetMinProtoVersionAfterDispose)
     test(_TestSSLContextSetMaxProtoVersionAfterDispose)
     test(_TestSSLContextGetMinProtoVersionAfterDispose)
@@ -3406,6 +3408,106 @@ class \nodoc\ iso _TestSSLContextALPNSetClientProtocolsAfterDispose is UnitTest
       ctx.alpn_set_client_protocols(["h2"]),
       "alpn_set_client_protocols() on a disposed context should return false")
 
+class \nodoc\ iso _TestSSLContextSetMinProtoVersionInvertedRange is UnitTest
+  """
+  `set_min_proto_version` raises when the new minimum is above the current
+  maximum, and does not change the minimum.
+
+  An equal range is valid: it pins the context to one protocol version.
+  `SSLAutoVersion` bypasses the check, since a zero boundary means the library
+  picks.
+  """
+  fun name(): String =>
+    "net/ssl/SSLContext.set_min_proto_version/inverted_range"
+
+  fun apply(h: TestHelper) =>
+    let ctx = SSLContext
+
+    try
+      ctx.set_max_proto_version(TLS1u2Version())?
+    else
+      h.fail("set_max_proto_version(TLS1u2Version) should not raise")
+      return
+    end
+
+    try
+      ctx.set_min_proto_version(TLS1u3Version())?
+      h.fail(
+        "set_min_proto_version(TLS1u3Version) should raise when max is "
+          + "TLS 1.2")
+    end
+
+    h.assert_eq[ILong](
+      TLS1u2Version().ilong(),
+      ctx.get_min_proto_version(),
+      "min should still be TLS 1.2 after the rejected call")
+
+    try
+      ctx.set_min_proto_version(SSLAutoVersion())?
+    else
+      h.fail("set_min_proto_version(SSLAutoVersion) should not raise")
+    end
+
+    try
+      ctx.set_min_proto_version(TLS1u2Version())?
+    else
+      h.fail(
+        "set_min_proto_version(TLS1u2Version) should not raise when max is "
+          + "TLS 1.2")
+    end
+
+    ctx.dispose()
+
+class \nodoc\ iso _TestSSLContextSetMaxProtoVersionInvertedRange is UnitTest
+  """
+  `set_max_proto_version` raises when the new maximum is below the current
+  minimum, and does not change the maximum.
+
+  An equal range is valid: it pins the context to one protocol version.
+  `SSLAutoVersion` bypasses the check, since a zero boundary means the library
+  picks.
+  """
+  fun name(): String =>
+    "net/ssl/SSLContext.set_max_proto_version/inverted_range"
+
+  fun apply(h: TestHelper) =>
+    let ctx = SSLContext
+
+    try
+      ctx.set_min_proto_version(TLS1u3Version())?
+    else
+      h.fail("set_min_proto_version(TLS1u3Version) should not raise")
+      return
+    end
+
+    try
+      ctx.set_max_proto_version(TLS1u2Version())?
+      h.fail(
+        "set_max_proto_version(TLS1u2Version) should raise when min is "
+          + "TLS 1.3")
+    end
+
+    h.assert_eq[ILong](
+      SSLAutoVersion().ilong(),
+      ctx.get_max_proto_version(),
+      "max should still be auto after the rejected call")
+
+    try
+      ctx.set_max_proto_version(TLS1u3Version())?
+    else
+      h.fail(
+        "set_max_proto_version(TLS1u3Version) should not raise when min is "
+          + "TLS 1.3")
+    end
+
+    try
+      ctx.set_max_proto_version(SSLAutoVersion())?
+    else
+      h.fail("set_max_proto_version(SSLAutoVersion) should not raise")
+    end
+
+    ctx.dispose()
+
 class \nodoc\ iso _TestSSLContextSetMinProtoVersionAfterDispose is UnitTest
   """
   `set_min_proto_version` on a disposed context raises an error rather than

ssl/net/ssl_context.pony

@@ -332,19 +332,27 @@ class val SSLContext
 
   fun ref set_min_proto_version(version: ULong) ? =>
     """
-    Set minimum protocol version. Set to SSLAutoVersion, 0,
-    to automatically manage lowest version. Raises an error if the context has
-    been disposed.
+    Set minimum protocol version. Set to SSLAutoVersion, 0, to automatically
+    manage lowest version.
+
+    Raises an error if the context has been disposed, if the version is above
+    the current maximum, or if the SSL library rejects the version.
 
     Supported versions: SSL3Version, TLS1Version, TLS1u1Version,
                         TLS1u2Version, TLS1u3Version, DTLS1Version,
                         DTLS1u2Version
     """
     if _ctx.is_null() then error end
 
+    let new_min = version.ilong()
+    let max_v = get_max_proto_version()
+    if (new_min != 0) and (max_v != 0) and (new_min > max_v) then
+      error
+    end
+
     let result =
       @SSL_CTX_ctrl(
-        _ctx, _SSLCtrlSetMinProtoVersion(), version.ilong(), Pointer[None])
+        _ctx, _SSLCtrlSetMinProtoVersion(), new_min, Pointer[None])
     if result == 0 then
       error
     end
@@ -365,19 +373,27 @@ class val SSLContext
 
   fun ref set_max_proto_version(version: ULong) ? =>
     """
-    Set maximum protocol version. Set to SSLAutoVersion, 0,
-    to automatically manage higest version. Raises an error if the context has
-    been disposed.
+    Set maximum protocol version. Set to SSLAutoVersion, 0, to automatically
+    manage highest version.
+
+    Raises an error if the context has been disposed, if the version is below
+    the current minimum, or if the SSL library rejects the version.
 
     Supported versions: SSL3Version, TLS1Version, TLS1u1Version,
                         TLS1u2Version, TLS1u3Version, DTLS1Version,
                         DTLS1u2Version
     """
     if _ctx.is_null() then error end
 
+    let new_max = version.ilong()
+    let min_v = get_min_proto_version()
+    if (new_max != 0) and (min_v != 0) and (new_max < min_v) then
+      error
+    end
+
     let result =
       @SSL_CTX_ctrl(
-        _ctx, _SSLCtrlSetMaxProtoVersion(), version.ilong(), Pointer[None])
+        _ctx, _SSLCtrlSetMaxProtoVersion(), new_max, Pointer[None])
     if result == 0 then
       error
     end