Slight tweaks and integration CI to support Bind9 (#1423)

1. This adds an integration CI dimension for Bind9

2. Resolved "cmocka unit tests" for Bind9

	* Additional <openssl/asn1.h> import in <openssl/objects.h>: Bind
	depends on some ASN1 functions, but does not directly import the
	corresponding header. OpenSSL imports the asn1 header file in
	objects.h (which Bind is pulling these symbols from), so I've
	added the header file reference to objects.h.

	* SSL_get_error error anticipation fixing: There were several
	failures discovered to be related this, thanks to research done
	in Implement SSL_MODE_AUTO_RETRY #1333. The issue was pinned down
	the check implemented in google/boringssl@9a38e92. This check used
	to exist before the final return of SSL_get_error in OpenSSL.
	Upstream moved this earlier in the function with
	google/boringssl@fcf2583. However, much of the functions guards for
	i < 0 checks have been removed since OpenSSL 1.1.1, so the early
	logic no longer applies.
	This check has evolved into SSL_ERROR_ZERO_RETURN in our code.
	Moving the check further down helps us gain better parity with
	OpenSSL 1.1.1. Doing so passes the bind test failures for
	proxystream_test, tls_test, and doh_test. This also happens to help
	our integration with CPython, so I've reconfigured that patch.
	We actually already use SSL_AUTO_RETRY by default in AWS-LC. The
	recent change mentioned in the point above surrounding the flag
	(208327e) was just to make some of the errors consistent in CPython
	when the flag was used. I've reverted the special behavior
	surrounding it since it should no longer be needed.

	* Assertion for SSL_set_shutdown: The assertion was added in
	63006a9, where it’s stated that we didn’t want SSL_set_shutdown
	messing up the state machine. This assertion is causing failures
	in tlsdns_test for Bind9, so it appears that we'll have to remove
	this to gain better OpenSSL parity.

3. Patch file needed for Bind seems to be slight bug in their build
configuration. This was from a fairly recent commit. We can look to
contribute this sometime soon.

Author: samuel40791765

.github/workflows/integrations.yml

@@ -102,3 +102,14 @@ jobs:
       - name: Build AWS-LC, build python, run tests
         run: |
           ./tests/ci/integration/run_python_integration.sh
+  bind9:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Install OS Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get -y --no-install-recommends install cmake gcc ninja-build golang make python3 python3-pytest autoconf pkg-config libcmocka-dev liburcu-dev libuv1-dev libnghttp2-dev libcap-dev libprotobuf-c-dev protobuf-c-compiler libfstrm-dev libjemalloc-dev
+      - uses: actions/checkout@v3
+      - name: Run bind9 build
+        run: |
+          ./tests/ci/integration/run_bind9_integration.sh

include/openssl/objects.h

@@ -16,3 +16,4 @@
    OpenSSL easier. */
 
 #include "obj.h"
+#include "asn1.h"

include/openssl/ssl.h

@@ -836,10 +836,6 @@ OPENSSL_EXPORT uint32_t SSL_get_options(const SSL *ssl);
 // |write|. In DTLS, it does nothing.
 #define SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER 0x00000002L
 
-// SSL_MODE_AUTO_RETRY suppresses terminal errors on empty reads if the
-// underlying connection state is retryable, allowing for automatic retries.
-#define SSL_MODE_AUTO_RETRY 0x00000004L
-
 // SSL_MODE_NO_AUTO_CHAIN disables automatically building a certificate chain
 // before sending certificates to the peer. This flag is set (and the feature
 // disabled) by default.
@@ -5281,6 +5277,7 @@ DEFINE_STACK_OF(SSL_COMP)
 
 // The following flags do nothing and are included only to make it easier to
 // compile code with BoringSSL.
+#define SSL_MODE_AUTO_RETRY 0
 #define SSL_MODE_RELEASE_BUFFERS 0
 #define SSL_MODE_SEND_CLIENTHELLO_TIME 0
 #define SSL_MODE_SEND_SERVERHELLO_TIME 0
@@ -5454,9 +5451,8 @@ OPENSSL_EXPORT int SSL_state(const SSL *ssl);
 // receiving close_notify in |SSL_shutdown| by causing the implementation to
 // believe the events already happened.
 //
-// It is an error to use |SSL_set_shutdown| to unset a bit that has already been
-// set. Doing so will trigger an |assert| in debug builds and otherwise be
-// ignored.
+// Note: |SSL_set_shutdown| cannot be used to unset a bit that has already
+// been set in AWS-LC. Doing so will be ignored.
 //
 // Use |SSL_CTX_set_quiet_shutdown| instead.
 OPENSSL_EXPORT void SSL_set_shutdown(SSL *ssl, int mode);

ssl/ssl_lib.cc

@@ -1382,23 +1382,6 @@ int SSL_get_error(const SSL *ssl, int ret_code) {
     return SSL_ERROR_SSL;
   }
 
-  if (ret_code == 0) {
-    if (ssl->s3->rwstate == SSL_ERROR_ZERO_RETURN) {
-      return SSL_ERROR_ZERO_RETURN;
-    }
-    // An EOF was observed which violates the protocol, and the underlying
-    // transport does not participate in the error queue. If
-    // |SSL_MODE_AUTO_RETRY| is unset, bubble up to the caller.
-    if ((ssl->ctx->mode & SSL_MODE_AUTO_RETRY) == 0) {
-      return SSL_ERROR_SYSCALL;
-    }
-    // If |SSL_MODE_AUTO_RETRY| is set, proceed if in a retryable state.
-    if (ssl->s3->rwstate != SSL_ERROR_WANT_READ
-            && ssl->s3->rwstate != SSL_ERROR_WANT_WRITE) {
-      return SSL_ERROR_SYSCALL;
-    }
-  }
-
   switch (ssl->s3->rwstate) {
     case SSL_ERROR_PENDING_SESSION:
     case SSL_ERROR_PENDING_CERTIFICATE:
@@ -1411,6 +1394,7 @@ int SSL_get_error(const SSL *ssl, int ret_code) {
     case SSL_ERROR_WANT_CERTIFICATE_VERIFY:
     case SSL_ERROR_WANT_RENEGOTIATE:
     case SSL_ERROR_HANDSHAKE_HINTS_READY:
+    case SSL_ERROR_ZERO_RETURN:
       return ssl->s3->rwstate;
 
     case SSL_ERROR_WANT_READ: {
@@ -2611,10 +2595,6 @@ void SSL_set_quiet_shutdown(SSL *ssl, int mode) {
 int SSL_get_quiet_shutdown(const SSL *ssl) { return ssl->quiet_shutdown; }
 
 void SSL_set_shutdown(SSL *ssl, int mode) {
-  // It is an error to clear any bits that have already been set. (We can't try
-  // to get a second close_notify or send two.)
-  assert((SSL_get_shutdown(ssl) & mode) == SSL_get_shutdown(ssl));
-
   if (mode & SSL_RECEIVED_SHUTDOWN &&
       ssl->s3->read_shutdown == ssl_shutdown_none) {
     ssl->s3->read_shutdown = ssl_shutdown_close_notify;

ssl/ssl_test.cc

@@ -10458,7 +10458,9 @@ TEST(SSLTest, ErrorSyscallAfterCloseNotify) {
   write_failed = false;
 }
 
-static void TestIntermittentEmptyRead(bool auto_retry) {
+// Test that failures are supressed on (potentially)
+// transient empty reads.
+TEST(SSLTest, IntermittentEmptyRead) {
   bssl::UniquePtr<SSL_CTX> client_ctx(SSL_CTX_new(TLS_method()));
   bssl::UniquePtr<SSL_CTX> server_ctx =
       CreateContextWithTestCertificate(TLS_method());
@@ -10489,15 +10491,6 @@ static void TestIntermittentEmptyRead(bool auto_retry) {
   ASSERT_TRUE(BIO_up_ref(client_rbio.get()));
   SSL_set0_rbio(client.get(), rbio_empty.release());
 
-  if (auto_retry) {
-      // Set flag under test
-      ASSERT_TRUE(SSL_CTX_set_mode(client_ctx.get(), SSL_MODE_AUTO_RETRY));
-      ASSERT_TRUE(SSL_CTX_get_mode(client_ctx.get()) & SSL_MODE_AUTO_RETRY);
-  } else {
-      // |SSL_MODE_AUTO_RETRY| is off by default
-      ASSERT_FALSE(SSL_CTX_get_mode(client_ctx.get()) & SSL_MODE_AUTO_RETRY);
-  }
-
   // Server writes some data to the client
   const uint8_t write_data[] = {1, 2, 3};
   int ret = SSL_write(server.get(), write_data, (int) sizeof(write_data));
@@ -10507,13 +10500,9 @@ static void TestIntermittentEmptyRead(bool auto_retry) {
   uint8_t read_data[] = {0, 0, 0};
   ret = SSL_read(client.get(), read_data, sizeof(read_data));
   EXPECT_EQ(ret, 0);
-  if (auto_retry) {
-      // On empty read, client should still want a read so caller will retry
-      EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ);
-  } else {
-      // On empty read, client should error out signaling EOF
-      EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_SYSCALL);
-  }
+  // On empty read, client should still want a read so caller will retry.
+  // This would have returned |SSL_ERROR_SYSCALL| in OpenSSL 1.0.2.
+  EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ);
 
   // Reset client rbio, read should succeed
   SSL_set0_rbio(client.get(), client_rbio.release());
@@ -10528,13 +10517,6 @@ static void TestIntermittentEmptyRead(bool auto_retry) {
   EXPECT_EQ(SSL_get_error(client.get(), ret), SSL_ERROR_WANT_READ);
 }
 
-// Test that |SSL_MODE_AUTO_RETRY| suppresses failure on (potentially)
-// transient empty reads.
-TEST(SSLTest, IntermittentEmptyRead) {
-    TestIntermittentEmptyRead(false);
-    TestIntermittentEmptyRead(true);
-}
-
 // Test that |SSL_shutdown|, when quiet shutdown is enabled, simulates receiving
 // a close_notify, down to |SSL_read| reporting |SSL_ERROR_ZERO_RETURN|.
 TEST(SSLTest, QuietShutdown) {

tests/ci/integration/bind9_patch/bind-fix.patch

@@ -0,0 +1,19 @@
+diff --git a/tests/isc/Makefile.am b/tests/isc/Makefile.am
+index 5cdd915..6ee1935 100644
+--- a/tests/isc/Makefile.am
++++ b/tests/isc/Makefile.am
+@@ -115,10 +115,12 @@ proxyheader_test_SOURCES =	\
+ 	proxyheader_test_data.h
+ 
+ proxystream_test_CPPFLAGS =	\
+-	$(AM_CPPFLAGS)
++	$(AM_CPPFLAGS)			\
++	$(OPENSSL_CFLAGS)
+ 
+ proxystream_test_LDADD =	\
+-	$(LDADD)
++	$(LDADD)				\
++	$(OPENSSL_LIBS)
+ 
+ proxystream_test_SOURCES =	\
+ 	proxystream_test.c \

tests/ci/integration/python_patch/3.10/aws-lc-cpython.patch

@@ -1,5 +1,5 @@
 diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py
-index 253a6c119c..2d0d10642d 100644
+index 253a6c1..2d0d106 100644
 --- a/Lib/test/test_asyncio/test_events.py
 +++ b/Lib/test/test_asyncio/test_events.py
 @@ -1106,12 +1106,12 @@ def test_create_server_ssl_match_failed(self):
@@ -20,7 +20,7 @@ index 253a6c119c..2d0d10642d 100644
 
          # close connection
 diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
-index 77152cf645..be3d11b993 100644
+index 77152cf..be3d11b 100644
 --- a/Lib/test/test_httplib.py
 +++ b/Lib/test/test_httplib.py
 @@ -1863,7 +1863,7 @@ def test_host_port(self):
@@ -33,7 +33,7 @@ index 77152cf645..be3d11b993 100644
          # just check status of PHA flag
          h = client.HTTPSConnection('localhost', 443)
 diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py
-index b5c78a5d49..41235c17fc 100644
+index b5c78a5..41235c1 100644
 --- a/Lib/test/test_imaplib.py
 +++ b/Lib/test/test_imaplib.py
 @@ -555,9 +555,10 @@ def test_ssl_raises(self):
@@ -66,7 +66,7 @@ index b5c78a5d49..41235c17fc 100644
                  client = self.imap_class(*server.server_address,
                                           ssl_context=ssl_context)
 diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
-index a1a581a907..c69e71159a 100644
+index a1a581a..c69e711 100644
 --- a/Lib/test/test_ssl.py
 +++ b/Lib/test/test_ssl.py
 @@ -44,6 +44,7 @@
@@ -319,8 +319,8 @@ index a1a581a907..c69e71159a 100644
          protocols = [
 @@ -4752,6 +4776,31 @@ def test_internal_chain_server(self):
                  self.assertEqual(res, b'\x02\n')
-
-
+ 
+ 
 +@unittest.skipUnless(Py_OPENSSL_IS_AWSLC, "Only test this against AWS-LC")
 +class TestPostHandshakeAuthAwsLc(unittest.TestCase):
 +    def test_pha(self):
@@ -350,7 +350,7 @@ index a1a581a907..c69e71159a 100644
  requires_keylog = unittest.skipUnless(
      HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
 diff --git a/Modules/Setup b/Modules/Setup
-index 87c6a152f8..3a9bc54bab 100644
+index 87c6a15..f67d7ec 100644
 --- a/Modules/Setup
 +++ b/Modules/Setup
 @@ -208,8 +208,8 @@ _symtable symtablemodule.c
@@ -386,7 +386,7 @@ index 87c6a152f8..3a9bc54bab 100644
  # The crypt module is now disabled by default because it breaks builds
  # on many systems (where -lcrypt is needed), e.g. Linux (I believe).
 diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
-index 35addf49e9..77a12c6af5 100644
+index 35addf4..77a12c6 100644
 --- a/Modules/_hashopenssl.c
 +++ b/Modules/_hashopenssl.c
 @@ -131,8 +131,12 @@ static const py_hashentry_t py_hashes[] = {
@@ -403,7 +403,7 @@ index 35addf49e9..77a12c6af5 100644
  };
 
 diff --git a/Modules/_ssl.c b/Modules/_ssl.c
-index 7a28f2d37f..07740af98b 100644
+index 7a28f2d..b0d2ea1 100644
 --- a/Modules/_ssl.c
 +++ b/Modules/_ssl.c
 @@ -181,6 +181,12 @@ extern const SSL_METHOD *TLSv1_2_method(void);
@@ -463,15 +463,6 @@ index 7a28f2d37f..07740af98b 100644
      int err = SSL_verify_client_post_handshake(self->ssl);
      if (err == 0)
          return _setSSLError(get_state_sock(self), NULL, 0, __FILE__, __LINE__);
-@@ -3186,7 +3198,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
-
-     /* Set SSL_MODE_RELEASE_BUFFERS. This potentially greatly reduces memory
-        usage for no cost at all. */
--    SSL_CTX_set_mode(self->ctx, SSL_MODE_RELEASE_BUFFERS);
-+    SSL_CTX_set_mode(self->ctx, SSL_MODE_RELEASE_BUFFERS | SSL_MODE_AUTO_RETRY);
-
- #define SID_CTX "Python"
-     SSL_CTX_set_session_id_context(self->ctx, (const unsigned char *) SID_CTX,
 @@ -3199,7 +3211,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
      X509_VERIFY_PARAM_set_flags(params, X509_V_FLAG_TRUSTED_FIRST);
      X509_VERIFY_PARAM_set_hostflags(params, self->hostflags);

tests/ci/integration/python_patch/3.11/aws-lc-cpython.patch

@@ -1,5 +1,5 @@
 diff --git a/Lib/test/test_asyncio/test_events.py b/Lib/test/test_asyncio/test_events.py
-index d7871d3e53..fb65ee3b8b 100644
+index d7871d3..fb65ee3 100644
 --- a/Lib/test/test_asyncio/test_events.py
 +++ b/Lib/test/test_asyncio/test_events.py
 @@ -1103,12 +1103,12 @@ def test_create_server_ssl_match_failed(self):
@@ -20,7 +20,7 @@ index d7871d3e53..fb65ee3b8b 100644
 
          # close connection
 diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
-index 015a3d1e87..ec565ad181 100644
+index 015a3d1..ec565ad 100644
 --- a/Lib/test/test_httplib.py
 +++ b/Lib/test/test_httplib.py
 @@ -2040,7 +2040,7 @@ def test_host_port(self):
@@ -33,7 +33,7 @@ index 015a3d1e87..ec565ad181 100644
          # just check status of PHA flag
          h = client.HTTPSConnection('localhost', 443)
 diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py
-index bd0fc9c2da..3ab70f5c1f 100644
+index bd0fc9c..3ab70f5 100644
 --- a/Lib/test/test_imaplib.py
 +++ b/Lib/test/test_imaplib.py
 @@ -561,9 +561,10 @@ def test_ssl_raises(self):
@@ -66,7 +66,7 @@ index bd0fc9c2da..3ab70f5c1f 100644
                  client = self.imap_class(*server.server_address,
                                           ssl_context=ssl_context)
 diff --git a/Lib/test/test_ssl.py b/Lib/test/test_ssl.py
-index 1f881038c3..36fa1e32e8 100644
+index 1f88103..36fa1e3 100644
 --- a/Lib/test/test_ssl.py
 +++ b/Lib/test/test_ssl.py
 @@ -44,6 +44,7 @@
@@ -359,7 +359,7 @@ index 1f881038c3..36fa1e32e8 100644
  requires_keylog = unittest.skipUnless(
      HAS_KEYLOG, 'test requires OpenSSL 1.1.1 with keylog callback')
 diff --git a/Modules/Setup b/Modules/Setup
-index d3647ecb99..a0ff874b6d 100644
+index d3647ec..a0ff874 100644
 --- a/Modules/Setup
 +++ b/Modules/Setup
 @@ -216,11 +216,11 @@ PYTHONPATH=$(COREPYTHONPATH)
@@ -380,7 +380,7 @@ index d3647ecb99..a0ff874b6d 100644
  # The _tkinter module.
  #
 diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
-index 57d64bd80c..1132fa520c 100644
+index 57d64bd..1132fa5 100644
 --- a/Modules/_hashopenssl.c
 +++ b/Modules/_hashopenssl.c
 @@ -131,8 +131,12 @@ static const py_hashentry_t py_hashes[] = {
@@ -397,7 +397,7 @@ index 57d64bd80c..1132fa520c 100644
  };
 
 diff --git a/Modules/_ssl.c b/Modules/_ssl.c
-index 67ce6e97af..1132d82dd9 100644
+index 67ce6e9..6f38611 100644
 --- a/Modules/_ssl.c
 +++ b/Modules/_ssl.c
 @@ -179,6 +179,12 @@ extern const SSL_METHOD *TLSv1_2_method(void);
@@ -457,15 +457,6 @@ index 67ce6e97af..1132d82dd9 100644
      int err = SSL_verify_client_post_handshake(self->ssl);
      if (err == 0)
          return _setSSLError(get_state_sock(self), NULL, 0, __FILE__, __LINE__);
-@@ -3191,7 +3203,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
- 
-     /* Set SSL_MODE_RELEASE_BUFFERS. This potentially greatly reduces memory
-        usage for no cost at all. */
--    SSL_CTX_set_mode(self->ctx, SSL_MODE_RELEASE_BUFFERS);
-+    SSL_CTX_set_mode(self->ctx, SSL_MODE_RELEASE_BUFFERS | SSL_MODE_AUTO_RETRY);
- 
- #define SID_CTX "Python"
-     SSL_CTX_set_session_id_context(self->ctx, (const unsigned char *) SID_CTX,
 @@ -3204,7 +3216,7 @@ _ssl__SSLContext_impl(PyTypeObject *type, int proto_version)
      X509_VERIFY_PARAM_set_flags(params, X509_V_FLAG_TRUSTED_FIRST);
      X509_VERIFY_PARAM_set_hostflags(params, self->hostflags);