Repository: pyca/pyopenssl
Pull request #933 (2020-08-07 23:13:07)

Context.set_verify: allow omission of callback
This PR allows it to call `set_verify` without specifying an explicit callback:

> If no verify_callback is specified, the default callback will be used. Its return value is identical to preverify_ok, so that any verification failure will lead to a termination of the TLS/SSL handshake with an alert message, if SSL_VERIFY_PEER is set.

This simplifies the most common case to `context.set_verify(SSL.VERIFY_PEER)`. Now that OpenSSL has builtin hostname validation, it's probably a good thing to discourage people from writing custom certificate verification.

reaperhulk: Based on the tests this appears to be true for every openssl in our CI. Do you happen to know when OpenSSL started defaulting to this behavior?
mhils: I don't, but going by the docs this has been the case since at least 1.0.2: https://www.openssl.org/docs/man1.0.2/man3/SSL_CTX_set_verify.html
The next pyOpenSSL release is supposed to drop 1.0.1, so we should be good here?
reaperhulk: Yep, 1.0.2 is minimum supported going forward so that’s fine. 

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 9ceedd0..e8c52c2 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -29,6 +29,9 @@ Changes:
 - Added ``OpenSSL.SSL.Connection.get_verified_chain`` to retrieve the
   verified certificate chain of the peer.
   `#894 <https://github.com/pyca/pyopenssl/pull/894>`_.
+- Make verification callback optional in ``Context.set_verify``.
+  If omitted, OpenSSL's default verification is used.
+  `#933 <https://github.com/pyca/pyopenssl/pull/933>`_
 
 
 19.1.0 (2019-11-18)
diff --git a/src/OpenSSL/SSL.py b/src/OpenSSL/SSL.py
index 29e489a..d889624 100644
--- a/src/OpenSSL/SSL.py
+++ b/src/OpenSSL/SSL.py
@@ -1003,7 +1003,7 @@ class Context(object):
         """
         return _lib.SSL_CTX_get_session_cache_mode(self._context)
 
-    def set_verify(self, mode, callback):
+    def set_verify(self, mode, callback=None):
         """
         et the verification flags for this Context object to *mode* and specify
         that *callback* should be used for verification callbacks.
@@ -1013,11 +1013,12 @@ class Context(object):
             :const:`VERIFY_PEER` is used, *mode* can be OR:ed with
             :const:`VERIFY_FAIL_IF_NO_PEER_CERT` and
             :const:`VERIFY_CLIENT_ONCE` to further control the behaviour.
-        :param callback: The Python callback to use.  This should take five
-            arguments: A Connection object, an X509 object, and three integer
-            variables, which are in turn potential error number, error depth
-            and return code. *callback* should return True if verification
-            passes and False otherwise.
+        :param callback: The optional Python verification callback to use.
+            This should take five arguments: A Connection object, an X509
+            object, and three integer variables, which are in turn potential
+            error number, error depth and return code. *callback* should
+            return True if verification passes and False otherwise.
+            If omitted, OpenSSL's default verification is used.
         :return: None
 
         See SSL_CTX_set_verify(3SSL) for further details.
@@ -1025,12 +1026,17 @@ class Context(object):
         if not isinstance(mode, integer_types):
             raise TypeError("mode must be an integer")
 
-        if not callable(callback):
-            raise TypeError("callback must be callable")
+        if callback is None:
+            self._verify_helper = None
+            self._verify_callback = None
+            _lib.SSL_CTX_set_verify(self._context, mode, _ffi.NULL)
+        else:
+            if not callable(callback):
+                raise TypeError("callback must be callable")
 
-        self._verify_helper = _VerifyHelper(callback)
-        self._verify_callback = self._verify_helper.callback
-        _lib.SSL_CTX_set_verify(self._context, mode, self._verify_callback)
+            self._verify_helper = _VerifyHelper(callback)
+            self._verify_callback = self._verify_helper.callback
+            _lib.SSL_CTX_set_verify(self._context, mode, self._verify_callback)
 
     def set_verify_depth(self, depth):
         """


Test patch:
diff --git a/tests/test_ssl.py b/tests/test_ssl.py
index 9f134b4..0860eb8 100644
--- a/tests/test_ssl.py
+++ b/tests/test_ssl.py
@@ -1378,6 +1378,29 @@ class TestContext(object):
 
         assert "silly verify failure" == str(exc.value)
 
+    @pytest.mark.parametrize("mode", [SSL.VERIFY_PEER, SSL.VERIFY_NONE])
+    def test_set_verify_default_callback(self, mode):
+        """
+        If the verify callback is omitted, the preverify value is used.
+        """
+        serverContext = Context(TLSv1_2_METHOD)
+        serverContext.use_privatekey(
+            load_privatekey(FILETYPE_PEM, root_key_pem)
+        )
+        serverContext.use_certificate(
+            load_certificate(FILETYPE_PEM, root_cert_pem)
+        )
+
+        clientContext = Context(TLSv1_2_METHOD)
+        clientContext.set_verify(mode, None)
+
+        if mode == SSL.VERIFY_PEER:
+            with pytest.raises(Exception) as exc:
+                self._handshake_test(serverContext, clientContext)
+            assert "certificate verify failed" in str(exc.value)
+        else:
+            self._handshake_test(serverContext, clientContext)
+
     def test_add_extra_chain_cert(self, tmpdir):
         """
         `Context.add_extra_chain_cert` accepts an `X509`
@@ -1509,9 +1532,7 @@ class TestContext(object):
         """
         context = Context(SSLv23_METHOD)
         assert context.get_verify_mode() == 0
-        context.set_verify(
-            VERIFY_PEER | VERIFY_CLIENT_ONCE, lambda *args: None
-        )
+        context.set_verify(VERIFY_PEER | VERIFY_CLIENT_ONCE)
         assert context.get_verify_mode() == (VERIFY_PEER | VERIFY_CLIENT_ONCE)
 
     @pytest.mark.parametrize("mode", [None, 1.0, object(), "mode"])
@@ -1522,9 +1543,9 @@ class TestContext(object):
         """
         context = Context(SSLv23_METHOD)
         with pytest.raises(TypeError):
-            context.set_verify(mode=mode, callback=lambda *args: None)
+            context.set_verify(mode=mode)
 
-    @pytest.mark.parametrize("callback", [None, 1.0, "mode", ("foo", "bar")])
+    @pytest.mark.parametrize("callback", [1.0, "mode", ("foo", "bar")])
     def test_set_verify_wrong_callable_arg(self, callback):
         """
         `Context.set_verify` raises `TypeError` if the second argument