Merge GH #1970 Fix more DTLS BoGo tests

8 files changed, 68 insertions, 40 deletions

diff --git a/src/bogo_shim/bogo_shim.cpp b/src/bogo_shim/bogo_shim.cpp
index 29331d3cd..f7d713162 100644
--- a/src/bogo_shim/bogo_shim.cpp
+++ b/src/bogo_shim/bogo_shim.cpp
@@ -71,6 +71,8 @@ void BOTAN_NORETURN shim_exit_with_error(const std::string& s, int rc = 1)
 
 std::string map_to_bogo_error(const std::string& e)
    {
+   shim_log("Original error " + e);
+
    static const std::unordered_map<std::string, std::string> err_map
       {
          { "Application data before handshake done", ":APPLICATION_DATA_INSTEAD_OF_HANDSHAKE:" },
@@ -91,7 +93,8 @@ std::string map_to_bogo_error(const std::string& e)
          { "Channel::key_material_export cannot export during renegotiation", "failed to export keying material" },
          { "Client cert verify failed", ":BAD_SIGNATURE:" },
          { "Client did not offer NULL compression", ":INVALID_COMPRESSION_LIST:" },
-         { "Client offered version with major version under 3", ":UNSUPPORTED_PROTOCOL:" },
+         { "Client offered TLS version with major version under 3", ":UNSUPPORTED_PROTOCOL:" },
+         { "Client offered DTLS version with major version 0xFF",  ":UNSUPPORTED_PROTOCOL:" },
          { "Client policy prohibits insecure renegotiation", ":RENEGOTIATION_MISMATCH:" },
          { "Client policy prohibits renegotiation", ":NO_RENEGOTIATION:" },
          { "Client resumed extended ms session without sending extension", ":RESUMED_EMS_SESSION_WITHOUT_EMS_EXTENSION:" },
@@ -103,6 +106,7 @@ std::string map_to_bogo_error(const std::string& e)
          { "Counterparty sent inconsistent key and sig types", ":WRONG_SIGNATURE_TYPE:" },
          { "Empty ALPN protocol not allowed", ":PARSE_TLSEXT:" },
          { "Encoding error: Cannot encode PSS string, output length too small", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
+         { "Expected TLS but got a record with DTLS version", ":WRONG_VERSION_NUMBER:" },
          { "Finished message didn't verify", ":DIGEST_CHECK_FAILED:" },
          { "Inconsistent length in certificate request", ":DECODE_ERROR:" },
          { "Inconsistent values in fragmented DTLS handshake header", ":FRAGMENT_MISMATCH:" },
@@ -119,6 +123,7 @@ std::string map_to_bogo_error(const std::string& e)
          { "Message authentication failure", ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:" },
          { "OS2ECP: Unknown format type 251", ":BAD_ECPOINT:" },
          { "Policy forbids all available TLS version", ":NO_SUPPORTED_VERSIONS_ENABLED:" },
+         { "Policy forbids all available DTLS version", ":NO_SUPPORTED_VERSIONS_ENABLED:" },
          { "Policy refuses to accept signing with any hash supported by peer", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
          { "Policy requires client send a certificate, but it did not", ":PEER_DID_NOT_RETURN_A_CERTIFICATE:" },
          { "Received a record that exceeds maximum size", ":ENCRYPTED_LENGTH_TOO_LONG:" },
@@ -879,27 +884,27 @@ class Shim_Policy final : public Botan::TLS::Policy
 
       bool allow_tls10() const override
          {
-         return (!m_args.flag_set("no-tls1"));
+         return !m_args.flag_set("dtls") && !m_args.flag_set("no-tls1");
          }
 
       bool allow_tls11() const override
          {
-         return (!m_args.flag_set("no-tls11"));
+         return !m_args.flag_set("dtls") && !m_args.flag_set("no-tls11");
          }
 
       bool allow_tls12() const override
          {
-         return (!m_args.flag_set("no-tls12"));
+         return !m_args.flag_set("dtls") && !m_args.flag_set("no-tls12");
          }
 
       bool allow_dtls10() const override
          {
-         return true; // ???
+         return m_args.flag_set("dtls") && !m_args.flag_set("no-tls1");
          }
 
       bool allow_dtls12() const override
          {
-         return true; // ???
+         return m_args.flag_set("dtls") && !m_args.flag_set("no-tls12");
          }
 
       //Botan::TLS::Group_Params default_dh_group() const override;
@@ -992,7 +997,7 @@ class Shim_Policy final : public Botan::TLS::Policy
 
       size_t dtls_default_mtu() const override
          {
-         return m_args.get_int_opt_or_else("mtu", 1232);
+         return m_args.get_int_opt_or_else("mtu", 1500);
          }
 
       //size_t dtls_initial_timeout() const override;
@@ -1490,11 +1495,6 @@ int main(int /*argc*/, char* argv[])
 
       const size_t buf_size = args->get_int_opt_or_else("read-size", 18*1024);
 
-      /*
-      if(is_datagram)
-         throw Shim_Exception("No support for DTLS yet", 89);
-      */
-
       Botan::ChaCha_RNG rng(Botan::secure_vector<uint8_t>(64));
       Botan::TLS::Session_Manager_In_Memory session_manager(rng, 1024);
       Shim_Credentials creds(*args);
diff --git a/src/bogo_shim/config.json b/src/bogo_shim/config.json
index b670aa39b..7b95c9f5d 100644
--- a/src/bogo_shim/config.json
+++ b/src/bogo_shim/config.json
@@ -24,7 +24,7 @@
         "ConflictingVersionNegotiation*": "No support for 1.3 version extension",
         "VersionNegotiationExtension*": "No support for 1.3 version extension",
         "IgnoreClientVersionOrder": "No support for 1.3 version extension",
-        "NoSupportedVersions": "No support for 1.3 version extension",
+        "NoSupportedVersions*": "No support for 1.3 version extension",
 
         "DuplicateCertCompressionExt*": "No support for 1.3 cert compression extension",
 
@@ -94,6 +94,8 @@
         "ClientAuth-Verify-ECDSA-P521-SHA512-TLS12": "BoringSSL will sign SHA-1 and SHA-512 with ECDSA but not accept them.",
         "ClientAuth-Verify-ECDSA-SHA1-TLS12": "BoringSSL will sign SHA-1 and SHA-512 with ECDSA but not accept them.",
 
+        "AppDataAfterChangeCipherSpec-DTLS*": "BoringSSL DTLS drops out of order AppData, we reject",
+
         "*Renegotiate-Server-Forbidden*": "Testing some BoringSSL specific restriction",
         "Resume-Client-NoResume-TLS1-TLS11": "BoGo expects resumption attempt sends latest version",
         "Resume-Client-NoResume-TLS1-TLS12": "BoGo expects resumption attempt sends latest version",
@@ -120,25 +122,13 @@
         "RSAPSSSupport-Default-NoCerts-TLS12-Server": "Not possible to disable PSS",
 
         "SRTP-Server-IgnoreMKI-*": "Non-empty MKI is rejected",
-        "DTLS-Replay*": "Needs analysis",
+
         "DTLS-Retransmit*": "Shim needs timeout support",
 
-        "AppDataAfterChangeCipherSpec-DTLS*": "Needs investigation",
-        "BadChangeCipherSpec-DTLS-*": "Needs investigation",
         "DTLS-StrayRetransmitFinished-ClientFull": "Needs investigation",
         "DTLS-StrayRetransmitFinished-ServerResume": "Needs investigation",
-        "DisableEverything-DTLS": "Needs investigation",
-        "LargeCiphertext-DTLS": "Needs investigation",
-        "MajorVersionTolerance-DTLS": "Needs investigation",
-        "MinimumVersion-Client-TLS12-TLS1-DTLS": "Needs investigation",
-        "MinimumVersion-Server-TLS12-TLS1-DTLS": "Needs investigation",
         "MixCompleteMessageWithFragments-DTLS": "Needs investigation",
-        "NoSupportedVersions-DTLS": "Needs investigation",
         "ReorderHandshakeFragments-Small-DTLS": "Needs investigation",
-        "SendUnencryptedFinished-DTLS": "Needs investigation",
-        "VersionNegotiation-Client-TLS1-TLS12-DTLS": "Needs investigation",
-        "VersionNegotiation-Server-TLS1-TLS12-DTLS": "Needs investigation",
-        "VersionTooLow-DTLS": "Needs investigation",
 
         "Shutdown-Shim-ApplicationData*": "Needs investigation",
         "Shutdown-Shim-HelloRequest-CannotHandshake*": "Needs investigation",
@@ -148,15 +138,19 @@
 
         "MTUExceeded": "BoringSSL splits DTLS handshakes differently",
 
-        "ClientOCSPCallback-FailNoStaple-*-DTLS*": "Alert problem",
-        "MinimumVersion-Client2-TLS12-TLS1-DTLS": "Alert problem",
-        "SendBogusAlertType-DTLS": "Alert problem",
-        "TrailingMessageData-*-DTLS*": "Alert problem",
-        "WrongMessageType-*-DTLS*": "Alert problem",
+        "MinimumVersion-Client-TLS12-TLS1-DTLS": "Client sends expected alert, server doesn't receive it. Needs investigation",
+        "ClientOCSPCallback-FailNoStaple-*-DTLS*": "Client sends expected alert, server doesn't receive it. Needs investigation",
+        "MinimumVersion-Client2-TLS12-TLS1-DTLS": "Client sends expected alert, server doesn't receive it. Needs investigation",
+        "SendBogusAlertType-DTLS": "Client sends expected alert, server doesn't receive it. Needs investigation",
+        "TrailingMessageData-*-DTLS*": "Client sends expected alert, server doesn't receive it. Needs investigation",
+        "WrongMessageType-*-DTLS*": "Client sends expected alert, server doesn't receive it. Needs investigation",
 
         "Renegotiate-Client-Packed": "Packing HelloRequest with Finished loses the HelloRequest (bug)",
         "SendHalfHelloRequest*PackHandshake": "Packing HelloRequest with Finished loses the HelloRequest (bug)",
+
         "PartialClientFinishedWithClientHello": "Need to check for buffered messages when CCS (bug)",
+        "SendUnencryptedFinished-DTLS": "Need to check for buffered messages when CCS (bug)",
+
         "SendOCSPResponseOnResume-TLS12": "Not supported by Botan (bug)",
         "ECDSAKeyUsage-TLS12": "Botan ignores KeyUsage (bug)",
         "RSAKeyUsage-*": "Botan ignores KeyUsage (bug)"
diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index 7f16e5ec3..b066f649e 100644
--- a/src/lib/tls/tls_channel.cpp
+++ b/src/lib/tls/tls_channel.cpp
@@ -332,6 +332,10 @@ size_t Channel::received_data(const uint8_t input[], size_t input_size)
          BOTAN_ASSERT(input_size == 0 || needed == 0,
                       "Got a full record or consumed all input");
 
+         // Ignore invalid records in DTLS
+         if(m_is_datagram && *record.get_type() == NO_RECORD)
+            return 0;
+
          if(input_size == 0 && needed != 0)
             return needed; // need more data to complete record
 
diff --git a/src/lib/tls/tls_handshake_io.cpp b/src/lib/tls/tls_handshake_io.cpp
index afab1dd20..8834e0008 100644
--- a/src/lib/tls/tls_handshake_io.cpp
+++ b/src/lib/tls/tls_handshake_io.cpp
@@ -189,6 +189,9 @@ void Datagram_Handshake_IO::add_record(const std::vector<uint8_t>& record,
 
    if(record_type == CHANGE_CIPHER_SPEC)
       {
+      if(record.size() != 1 || record[0] != 1)
+         throw Decoding_Error("Invalid ChangeCipherSpec");
+
       // TODO: check this is otherwise empty
       m_ccs_epochs.insert(epoch);
       return;
diff --git a/src/lib/tls/tls_record.cpp b/src/lib/tls/tls_record.cpp
index a07efd306..730751855 100644
--- a/src/lib/tls/tls_record.cpp
+++ b/src/lib/tls/tls_record.cpp
@@ -343,7 +343,9 @@ size_t read_tls_record(secure_vector<uint8_t>& readbuf,
 
    *rec.get_protocol_version() = Protocol_Version(readbuf[1], readbuf[2]);
 
-   BOTAN_ASSERT(!rec.get_protocol_version()->is_datagram_protocol(), "Expected TLS");
+   if(rec.get_protocol_version()->is_datagram_protocol())
+      throw TLS_Exception(Alert::PROTOCOL_VERSION,
+                          "Expected TLS but got a record with DTLS version");
 
    const size_t record_size = make_uint16(readbuf[TLS_HEADER_SIZE-2],
                                          readbuf[TLS_HEADER_SIZE-1]);
@@ -429,19 +431,29 @@ size_t read_dtls_record(secure_vector<uint8_t>& readbuf,
 
    *rec.get_protocol_version() = Protocol_Version(readbuf[1], readbuf[2]);
 
-   BOTAN_ASSERT(rec.get_protocol_version()->is_datagram_protocol(), "Expected DTLS");
+   if(rec.get_protocol_version()->is_datagram_protocol() == false)
+      {
+      readbuf.clear();
+      *rec.get_type() = NO_RECORD;
+      return 0;
+      }
 
    const size_t record_size = make_uint16(readbuf[DTLS_HEADER_SIZE-2],
                                           readbuf[DTLS_HEADER_SIZE-1]);
 
    if(record_size > MAX_CIPHERTEXT_SIZE)
-      throw TLS_Exception(Alert::RECORD_OVERFLOW,
-                          "Got message that exceeds maximum size");
+      {
+      // Too large to be valid, ignore it
+      readbuf.clear();
+      *rec.get_type() = NO_RECORD;
+      return 0;
+      }
 
    if(fill_buffer_to(readbuf, raw_input.get_data(), raw_input.get_size(), raw_input.get_consumed(), DTLS_HEADER_SIZE + record_size))
       {
       // Truncated packet?
       readbuf.clear();
+      *rec.get_type() = NO_RECORD;
       return 0;
       }
 
@@ -458,6 +470,7 @@ size_t read_dtls_record(secure_vector<uint8_t>& readbuf,
    if(sequence_numbers && sequence_numbers->already_seen(*rec.get_sequence()))
       {
       readbuf.clear();
+      *rec.get_type() = NO_RECORD;
       return 0;
       }
 
@@ -467,6 +480,8 @@ size_t read_dtls_record(secure_vector<uint8_t>& readbuf,
       {
       rec.get_data().assign(readbuf.begin() + DTLS_HEADER_SIZE, readbuf.begin() + DTLS_HEADER_SIZE + record_size);
       readbuf.clear();
+      if(sequence_numbers)
+         sequence_numbers->read_accept(*rec.get_sequence());
       return 0; // got a full record
       }
 
diff --git a/src/lib/tls/tls_seq_numbers.h b/src/lib/tls/tls_seq_numbers.h
index 85077f5f5..34f949baa 100644
--- a/src/lib/tls/tls_seq_numbers.h
+++ b/src/lib/tls/tls_seq_numbers.h
@@ -87,12 +87,16 @@ class Datagram_Sequence_Numbers final : public Connection_Sequence_Numbers
          const size_t window_size = sizeof(m_window_bits) * 8;
 
          if(sequence > m_window_highest)
+            {
             return false;
+            }
 
          const uint64_t offset = m_window_highest - sequence;
 
          if(offset >= window_size)
+            {
             return true; // really old?
+            }
 
          return (((m_window_bits >> offset) & 1) == 1);
          }
diff --git a/src/lib/tls/tls_server.cpp b/src/lib/tls/tls_server.cpp
index 63e04a568..215cf0f04 100644
--- a/src/lib/tls/tls_server.cpp
+++ b/src/lib/tls/tls_server.cpp
@@ -468,10 +468,18 @@ void Server::process_client_hello_msg(const Handshake_State* active_state,
    pending_state.client_hello(new Client_Hello(contents));
    const Protocol_Version client_offer = pending_state.client_hello()->version();
 
-   if(client_offer.major_version() < 3)
-      throw TLS_Exception(Alert::PROTOCOL_VERSION, "Client offered version with major version under 3");
-   if(client_offer.major_version() == 3 && client_offer.minor_version() == 0)
-      throw TLS_Exception(Alert::PROTOCOL_VERSION, "SSLv3 is not supported");
+   if(client_offer.is_datagram_protocol())
+      {
+      if(client_offer.major_version() == 0xFF)
+         throw TLS_Exception(Alert::PROTOCOL_VERSION, "Client offered DTLS version with major version 0xFF");
+      }
+   else
+      {
+      if(client_offer.major_version() < 3)
+         throw TLS_Exception(Alert::PROTOCOL_VERSION, "Client offered TLS version with major version under 3");
+      if(client_offer.major_version() == 3 && client_offer.minor_version() == 0)
+         throw TLS_Exception(Alert::PROTOCOL_VERSION, "SSLv3 is not supported");
+      }
 
    const Protocol_Version negotiated_version =
       select_version(policy(), client_offer,
diff --git a/src/lib/tls/tls_version.cpp b/src/lib/tls/tls_version.cpp
index 33db02bce..ecbe94897 100644
--- a/src/lib/tls/tls_version.cpp
+++ b/src/lib/tls/tls_version.cpp
@@ -32,7 +32,7 @@ std::string Protocol_Version::to_string() const
 
 bool Protocol_Version::is_datagram_protocol() const
    {
-   return major_version() == 254;
+   return major_version() > 250;
    }
 
 bool Protocol_Version::operator>(const Protocol_Version& other) const