Add support to zeroize plaintext in S3 record layer master
authorMartin Elshuber <martin.elshuber@theobroma-systems.com>
Tue, 23 Jun 2020 10:14:41 +0000 (12:14 +0200)
committerDmitry Belyavskiy <beldmit@gmail.com>
Tue, 7 Jul 2020 09:07:47 +0000 (12:07 +0300)
Some applications want even all plaintext copies beeing
zeroized. However, currently plaintext residuals are kept in rbuf
within the s3 record layer.

This patch add the option SSL_OP_CLEANSE_PLAINTEXT to its friends to
optionally enable cleansing of decrypted plaintext data.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/12251)

CHANGES.md
doc/man3/SSL_CTX_set_options.pod
include/openssl/ssl.h
ssl/record/rec_layer_d1.c
ssl/record/rec_layer_s3.c
ssl/record/ssl3_buffer.c
test/sslapitest.c

index 2cb7398..4e0002f 100644 (file)
@@ -1100,6 +1100,14 @@ OpenSSL 3.0
 
    *Boris Pismenny*
 
+ * The SSL option SSL_OP_CLEANSE_PLAINTEXT is introduced. If that
+   option is set, openssl cleanses (zeroize) plaintext bytes from
+   internal buffers after delivering them to the application. Note,
+   the application is still responsible for cleansing other copies
+   (e.g.: data received by SSL_read(3)).
+
+   *Martin Elshuber*
+
 OpenSSL 1.1.1
 -------------
 
index 1bf19ec..adc646d 100644 (file)
@@ -265,6 +265,20 @@ functionality is not required. Those applications can turn this feature off by
 setting this option. This is a server-side opton only. It is ignored by
 clients.
 
+=item SSL_OP_CLEANSE_PLAINTEXT
+
+By default TLS connections keep a copy of received plaintext
+application data in a static buffer until it is overwritten by the
+next portion of data. When enabling SSL_OP_CLEANSE_PLAINTEXT
+deciphered application data is cleansed by calling OPENSSL_cleanse(3)
+after passing data to the application. Data is also cleansed when
+releasing the connection (eg. L<SSL_free(3)>).
+
+Since OpenSSL only cleanses internal buffers, the application is still
+responsible for cleansing all other buffers. Most notably, this
+applies to buffers passed to functions like L<SSL_read(3)>,
+L<SSL_peek(3)> but also like L<SSL_write(3)>.
+
 =back
 
 The following options no longer have any effect but their identifiers are
index f855f94..8d96f0d 100644 (file)
@@ -321,7 +321,8 @@ typedef int (*SSL_async_callback_fn)(SSL *s, void *arg);
 /* Disable Extended master secret */
 # define SSL_OP_NO_EXTENDED_MASTER_SECRET                0x00000001U
 
-/* Reserved value (until OpenSSL 3.0.0)                  0x00000002U */
+/* Cleanse plaintext copies of data delivered to the application */
+# define SSL_OP_CLEANSE_PLAINTEXT                        0x00000002U
 
 /* Allow initial connection to servers that don't support RI */
 # define SSL_OP_LEGACY_SERVER_CONNECT                    0x00000004U
index 866ef18..0da012f 100644 (file)
@@ -74,6 +74,8 @@ void DTLS_RECORD_LAYER_clear(RECORD_LAYER *rl)
 
     while ((item = pqueue_pop(d->processed_rcds.q)) != NULL) {
         rdata = (DTLS1_RECORD_DATA *)item->data;
+        if (rl->s->options & SSL_OP_CLEANSE_PLAINTEXT)
+            OPENSSL_cleanse(rdata->rbuf.buf, rdata->rbuf.len);
         OPENSSL_free(rdata->rbuf.buf);
         OPENSSL_free(item->data);
         pitem_free(item);
@@ -81,6 +83,8 @@ void DTLS_RECORD_LAYER_clear(RECORD_LAYER *rl)
 
     while ((item = pqueue_pop(d->buffered_app_data.q)) != NULL) {
         rdata = (DTLS1_RECORD_DATA *)item->data;
+        if (rl->s->options & SSL_OP_CLEANSE_PLAINTEXT)
+            OPENSSL_cleanse(rdata->rbuf.buf, rdata->rbuf.len);
         OPENSSL_free(rdata->rbuf.buf);
         OPENSSL_free(item->data);
         pitem_free(item);
@@ -514,6 +518,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
             if (SSL3_RECORD_get_length(rr) == 0)
                 SSL3_RECORD_set_read(rr);
         } else {
+            if (s->options & SSL_OP_CLEANSE_PLAINTEXT)
+                OPENSSL_cleanse(&(SSL3_RECORD_get_data(rr)[SSL3_RECORD_get_off(rr)]), n);
             SSL3_RECORD_sub_length(rr, n);
             SSL3_RECORD_add_off(rr, n);
             if (SSL3_RECORD_get_length(rr) == 0) {
index 8ea1667..1d9e803 100644 (file)
@@ -1484,6 +1484,8 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
                 if (SSL3_RECORD_get_length(rr) == 0)
                     SSL3_RECORD_set_read(rr);
             } else {
+                if (s->options & SSL_OP_CLEANSE_PLAINTEXT)
+                    OPENSSL_cleanse(&(rr->data[rr->off]), n);
                 SSL3_RECORD_sub_length(rr, n);
                 SSL3_RECORD_add_off(rr, n);
                 if (SSL3_RECORD_get_length(rr) == 0) {
index 2c25099..4ebb478 100644 (file)
@@ -180,6 +180,8 @@ int ssl3_release_read_buffer(SSL *s)
     SSL3_BUFFER *b;
 
     b = RECORD_LAYER_get_rbuf(&s->rlayer);
+    if (s->options & SSL_OP_CLEANSE_PLAINTEXT)
+        OPENSSL_cleanse(b->buf, b->len);
     OPENSSL_free(b->buf);
     b->buf = NULL;
     return 1;
index 30dcae3..182984e 100644 (file)
@@ -1595,6 +1595,119 @@ static int test_large_message_dtls(void)
 }
 #endif
 
+static int execute_cleanse_plaintext(const SSL_METHOD *smeth,
+                                     const SSL_METHOD *cmeth,
+                                     int min_version, int max_version)
+{
+    size_t i;
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    int testresult = 0;
+    SSL3_RECORD *rr;
+    void *zbuf;
+
+    static unsigned char cbuf[16000];
+    static unsigned char sbuf[16000];
+
+    if (!TEST_true(create_ssl_ctx_pair(libctx,
+                                       smeth, cmeth,
+                                       min_version, max_version,
+                                       &sctx, &cctx, cert,
+                                       privkey)))
+        goto end;
+
+    if (!TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+                                      NULL, NULL)))
+        goto end;
+
+    if (!TEST_true(SSL_set_options(serverssl, SSL_OP_CLEANSE_PLAINTEXT)))
+        goto end;
+
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl,
+                                         SSL_ERROR_NONE)))
+        goto end;
+
+    for (i = 0; i < sizeof(cbuf); i++) {
+        cbuf[i] = i & 0xff;
+    }
+
+    if (!TEST_int_eq(SSL_write(clientssl, cbuf, sizeof(cbuf)), sizeof(cbuf)))
+        goto end;
+
+    if (!TEST_int_eq(SSL_peek(serverssl, &sbuf, sizeof(sbuf)), sizeof(sbuf)))
+        goto end;
+
+    if (!TEST_mem_eq(cbuf, sizeof(cbuf), sbuf, sizeof(sbuf)))
+        goto end;
+
+    /*
+     * Since we called SSL_peek(), we know the data in the record
+     * layer is a plaintext record. We can gather the pointer to check
+     * for zeroization after SSL_read().
+     */
+    rr = serverssl->rlayer.rrec;
+    zbuf = &rr->data[rr->off];
+    if (!TEST_int_eq(rr->length, sizeof(cbuf)))
+        goto end;
+
+    /*
+     * After SSL_peek() the plaintext must still be stored in the
+     * record.
+     */
+    if (!TEST_mem_eq(cbuf, sizeof(cbuf), zbuf, sizeof(cbuf)))
+        goto end;
+
+    memset(sbuf, 0, sizeof(sbuf));
+    if (!TEST_int_eq(SSL_read(serverssl, &sbuf, sizeof(sbuf)), sizeof(sbuf)))
+        goto end;
+
+    if (!TEST_mem_eq(cbuf, sizeof(cbuf), sbuf, sizeof(cbuf)))
+        goto end;
+
+    /* Check if rbuf is cleansed */
+    memset(cbuf, 0, sizeof(cbuf));
+    if (!TEST_mem_eq(cbuf, sizeof(cbuf), zbuf, sizeof(cbuf)))
+        goto end;
+
+    testresult = 1;
+ end:
+    SSL_free(serverssl);
+    SSL_free(clientssl);
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+
+    return testresult;
+}
+
+static int test_cleanse_plaintext(void)
+{
+#if !defined(OPENSSL_NO_TLS1_2)
+    if (!TEST_true(execute_cleanse_plaintext(TLS_server_method(),
+                                             TLS_client_method(),
+                                             TLS1_2_VERSION,
+                                             TLS1_2_VERSION)))
+        return 0;
+
+#endif
+
+#if !defined(OPENSSL_NO_TLS1_3)
+    if (!TEST_true(execute_cleanse_plaintext(TLS_server_method(),
+                                             TLS_client_method(),
+                                             TLS1_3_VERSION,
+                                             TLS1_3_VERSION)))
+        return 0;
+#endif
+
+#if !defined(OPENSSL_NO_DTLS)
+    if (!TEST_true(execute_cleanse_plaintext(DTLS_server_method(),
+                                             DTLS_client_method(),
+                                             DTLS1_VERSION,
+                                             0)))
+        return 0;
+#endif
+    return 1;
+}
+
 #ifndef OPENSSL_NO_OCSP
 static int ocsp_server_cb(SSL *s, void *arg)
 {
@@ -8324,6 +8437,7 @@ int setup_tests(void)
 #ifndef OPENSSL_NO_DTLS
     ADD_TEST(test_large_message_dtls);
 #endif
+    ADD_TEST(test_cleanse_plaintext);
 #ifndef OPENSSL_NO_OCSP
     ADD_TEST(test_tlsext_status_type);
 #endif