Fix end-point shared secret for DTLS/SCTP

When computing the end-point shared secret, don't take the
terminating NULL character into account.
Please note that this fix breaks interoperability with older
versions of OpenSSL, which are not fixed.

Fixes #7956

Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7957)

6 files changed, 225 insertions, 2 deletions

diff --git a/test/handshake_helper.c b/test/handshake_helper.c
index a9ef753d8d..a85d22e368 100644
--- a/test/handshake_helper.c
+++ b/test/handshake_helper.c
@@ -22,6 +22,10 @@
 #include "handshake_helper.h"
 #include "testutil.h"
 
+#if !defined(OPENSSL_NO_SCTP) && !defined(OPENSSL_NO_SOCK)
+#include <netinet/sctp.h>
+#endif
+
 HANDSHAKE_RESULT *HANDSHAKE_RESULT_new(void)
 {
     HANDSHAKE_RESULT *ret;
@@ -1282,13 +1286,33 @@ static int peer_pkey_type(SSL *s)
 #if !defined(OPENSSL_NO_SCTP) && !defined(OPENSSL_NO_SOCK)
 static int set_sock_as_sctp(int sock)
 {
+    struct sctp_assocparams assocparams;
+    struct sctp_rtoinfo rto_info;
+    BIO *tmpbio;
+
+    /*
+     * To allow tests to fail fast (within a second or so), reduce the
+     * retransmission timeouts and the number of retransmissions.
+     */
+    memset(&rto_info, 0, sizeof(struct sctp_rtoinfo));
+    rto_info.srto_initial = 100;
+    rto_info.srto_max = 200;
+    rto_info.srto_min = 50;
+    (void)setsockopt(sock, IPPROTO_SCTP, SCTP_RTOINFO,
+                     (const void *)&rto_info, sizeof(struct sctp_rtoinfo));
+    memset(&assocparams, 0, sizeof(struct sctp_assocparams));
+    assocparams.sasoc_asocmaxrxt = 2;
+    (void)setsockopt(sock, IPPROTO_SCTP, SCTP_ASSOCINFO,
+                     (const void *)&assocparams,
+                     sizeof(struct sctp_assocparams));
+
     /*
      * For SCTP we have to set various options on the socket prior to
      * connecting. This is done automatically by BIO_new_dgram_sctp().
      * We don't actually need the created BIO though so we free it again
      * immediately.
      */
-    BIO *tmpbio = BIO_new_dgram_sctp(sock, BIO_NOCLOSE);
+    tmpbio = BIO_new_dgram_sctp(sock, BIO_NOCLOSE);
 
     if (tmpbio == NULL)
         return 0;
@@ -1438,6 +1462,13 @@ static HANDSHAKE_RESULT *do_handshake_internal(
         return NULL;
     }
 
+#if !defined(OPENSSL_NO_SCTP) && !defined(OPENSSL_NO_SOCK)
+    if (test_ctx->enable_client_sctp_label_bug)
+        SSL_CTX_set_mode(client_ctx, SSL_MODE_DTLS_SCTP_LABEL_LENGTH_BUG);
+    if (test_ctx->enable_server_sctp_label_bug)
+        SSL_CTX_set_mode(server_ctx, SSL_MODE_DTLS_SCTP_LABEL_LENGTH_BUG);
+#endif
+
     /* Setup SSL and buffers; additional configuration happens below. */
     if (!create_peer(&server, server_ctx)) {
         TEST_note("creating server context");
diff --git a/test/recipes/80-test_ssl_new.t b/test/recipes/80-test_ssl_new.t
index ecec3c91e8..fa98f24b57 100644
--- a/test/recipes/80-test_ssl_new.t
+++ b/test/recipes/80-test_ssl_new.t
@@ -28,7 +28,7 @@ map { s/\^// } @conf_files if $^O eq "VMS";
 
 # We hard-code the number of tests to double-check that the globbing above
 # finds all files as expected.
-plan tests => 28;  # = scalar @conf_srcs
+plan tests => 29;  # = scalar @conf_srcs
 
 # Some test results depend on the configuration of enabled protocols. We only
 # verify generated sources in the default configuration.
@@ -102,6 +102,7 @@ my %skip = (
   "24-padding.conf" => disabled("tls1_3"),
   "25-cipher.conf" => disabled("ec") || disabled("tls1_2"),
   "26-tls13_client_auth.conf" => disabled("tls1_3"),
+  "29-dtls-sctp-label-bug.conf" => disabled("sctp") || disabled("sock"),
 );
 
 foreach my $conf (@conf_files) {
diff --git a/test/ssl-tests/29-dtls-sctp-label-bug.conf b/test/ssl-tests/29-dtls-sctp-label-bug.conf
new file mode 100644
index 0000000000..24f9e04f16
--- /dev/null
+++ b/test/ssl-tests/29-dtls-sctp-label-bug.conf
@@ -0,0 +1,116 @@
+# Generated with generate_ssl_tests.pl
+
+num_tests = 4
+
+test-0 = 0-SCTPLabelBug-good1
+test-1 = 1-SCTPLabelBug-good2
+test-2 = 2-SCTPLabelBug-bad1
+test-3 = 3-SCTPLabelBug-bad2
+# ===========================================================
+
+[0-SCTPLabelBug-good1]
+ssl_conf = 0-SCTPLabelBug-good1-ssl
+
+[0-SCTPLabelBug-good1-ssl]
+server = 0-SCTPLabelBug-good1-server
+client = 0-SCTPLabelBug-good1-client
+
+[0-SCTPLabelBug-good1-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[0-SCTPLabelBug-good1-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-0]
+EnableClientSCTPLabelBug = No
+EnableServerSCTPLabelBug = No
+ExpectedResult = Success
+Method = DTLS
+UseSCTP = Yes
+
+
+# ===========================================================
+
+[1-SCTPLabelBug-good2]
+ssl_conf = 1-SCTPLabelBug-good2-ssl
+
+[1-SCTPLabelBug-good2-ssl]
+server = 1-SCTPLabelBug-good2-server
+client = 1-SCTPLabelBug-good2-client
+
+[1-SCTPLabelBug-good2-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[1-SCTPLabelBug-good2-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-1]
+EnableClientSCTPLabelBug = Yes
+EnableServerSCTPLabelBug = Yes
+ExpectedResult = Success
+Method = DTLS
+UseSCTP = Yes
+
+
+# ===========================================================
+
+[2-SCTPLabelBug-bad1]
+ssl_conf = 2-SCTPLabelBug-bad1-ssl
+
+[2-SCTPLabelBug-bad1-ssl]
+server = 2-SCTPLabelBug-bad1-server
+client = 2-SCTPLabelBug-bad1-client
+
+[2-SCTPLabelBug-bad1-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[2-SCTPLabelBug-bad1-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-2]
+EnableClientSCTPLabelBug = Yes
+EnableServerSCTPLabelBug = No
+ExpectedResult = ClientFail
+Method = DTLS
+UseSCTP = Yes
+
+
+# ===========================================================
+
+[3-SCTPLabelBug-bad2]
+ssl_conf = 3-SCTPLabelBug-bad2-ssl
+
+[3-SCTPLabelBug-bad2-ssl]
+server = 3-SCTPLabelBug-bad2-server
+client = 3-SCTPLabelBug-bad2-client
+
+[3-SCTPLabelBug-bad2-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[3-SCTPLabelBug-bad2-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-3]
+EnableClientSCTPLabelBug = No
+EnableServerSCTPLabelBug = Yes
+ExpectedResult = ClientFail
+Method = DTLS
+UseSCTP = Yes
+
+
diff --git a/test/ssl-tests/29-dtls-sctp-label-bug.conf.in b/test/ssl-tests/29-dtls-sctp-label-bug.conf.in
new file mode 100644
index 0000000000..ab04d05264
--- /dev/null
+++ b/test/ssl-tests/29-dtls-sctp-label-bug.conf.in
@@ -0,0 +1,67 @@
+# -*- mode: perl; -*-
+# Copyright 2019-2019 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+
+## Test SSL_MODE_DTLS_SCTP_LABEL_LENGTH_BUG handling
+
+use strict;
+use warnings;
+
+package ssltests;
+use OpenSSL::Test::Utils;
+
+our @tests = (
+    {
+        name => "SCTPLabelBug-good1",
+        server => {},
+        client => {},
+        test => {
+            "Method" => "DTLS",
+            "UseSCTP" => "Yes",
+            "EnableClientSCTPLabelBug" => "No",
+            "EnableServerSCTPLabelBug" => "No",
+            "ExpectedResult" => "Success"
+        }
+    },
+    {
+        name => "SCTPLabelBug-good2",
+        server => {},
+        client => {},
+        test => {
+            "Method" => "DTLS",
+            "UseSCTP" => "Yes",
+            "EnableClientSCTPLabelBug" => "Yes",
+            "EnableServerSCTPLabelBug" => "Yes",
+            "ExpectedResult" => "Success"
+        }
+    },
+    {
+        name => "SCTPLabelBug-bad1",
+        server => {},
+        client => {},
+        test => {
+            "Method" => "DTLS",
+            "UseSCTP" => "Yes",
+            "EnableClientSCTPLabelBug" => "Yes",
+            "EnableServerSCTPLabelBug" => "No",
+            "ExpectedResult" => "ClientFail"
+        }
+    },
+    {
+        name => "SCTPLabelBug-bad2",
+        server => {},
+        client => {},
+        test => {
+            "Method" => "DTLS",
+            "UseSCTP" => "Yes",
+            "EnableClientSCTPLabelBug" => "No",
+            "EnableServerSCTPLabelBug" => "Yes",
+            "ExpectedResult" => "ClientFail"
+        }
+    },
+);
diff --git a/test/ssl_test_ctx.c b/test/ssl_test_ctx.c
index 3b7cc3eb5b..6a3b66bcb9 100644
--- a/test/ssl_test_ctx.c
+++ b/test/ssl_test_ctx.c
@@ -446,6 +446,8 @@ const char *ssl_ct_validation_name(ssl_ct_validation_t mode)
 IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, resumption_expected)
 IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_SERVER_CONF, server, broken_session_ticket)
 IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, use_sctp)
+IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, enable_client_sctp_label_bug)
+IMPLEMENT_SSL_TEST_BOOL_OPTION(SSL_TEST_CTX, test, enable_server_sctp_label_bug)
 
 /* CertStatus */
 
@@ -669,6 +671,8 @@ static const ssl_test_ctx_option ssl_test_ctx_options[] = {
     { "ExpectedClientSignType", &parse_expected_client_sign_type },
     { "ExpectedClientCANames", &parse_expected_client_ca_names },
     { "UseSCTP", &parse_test_use_sctp },
+    { "EnableClientSCTPLabelBug", &parse_test_enable_client_sctp_label_bug },
+    { "EnableServerSCTPLabelBug", &parse_test_enable_server_sctp_label_bug },
     { "ExpectedCipher", &parse_test_expected_cipher },
     { "ExpectedSessionTicketAppData", &parse_test_expected_session_ticket_app_data },
 };
diff --git a/test/ssl_test_ctx.h b/test/ssl_test_ctx.h
index 8fa3af0a50..3a21ac52d9 100644
--- a/test/ssl_test_ctx.h
+++ b/test/ssl_test_ctx.h
@@ -214,6 +214,10 @@ typedef struct {
     STACK_OF(X509_NAME) *expected_client_ca_names;
     /* Whether to use SCTP for the transport */
     int use_sctp;
+    /* Enable SSL_MODE_DTLS_SCTP_LABEL_LENGTH_BUG on client side */
+    int enable_client_sctp_label_bug;
+    /* Enable SSL_MODE_DTLS_SCTP_LABEL_LENGTH_BUG on server side */
+    int enable_server_sctp_label_bug;
     /* Whether to expect a session id from the server */
     ssl_session_id_t session_id_expected;
     char *expected_cipher;