Repository: randombit/botan
Pull request #1975 (2019-05-24 11:08:59)

Add a script for running TLS Fuzzer
Can't use this in CI as it has too many sharp edges (assumes SSLv3 support, etc).


Patch:
diff --git a/src/cli/tls_proxy.cpp b/src/cli/tls_proxy.cpp
index e2613f95d..2770a7c48 100644
--- a/src/cli/tls_proxy.cpp
+++ b/src/cli/tls_proxy.cpp
@@ -57,11 +57,13 @@ inline void log_error(const char* where, const boost::system::error_code& error)
 
 inline void log_binary_message(const char* where, const uint8_t buf[], size_t buf_len)
    {
+   BOTAN_UNUSED(where, buf, buf_len);
    //std::cout << where << ' ' << Botan::hex_encode(buf, buf_len) << std::endl;
    }
 
 void log_text_message(const char* where,  const uint8_t buf[], size_t buf_len)
    {
+   BOTAN_UNUSED(where, buf, buf_len);
    //const char* c = reinterpret_cast<const char*>(buf);
    //std::cout << where << ' ' << std::string(c, c + buf_len)  << std::endl;
    }
diff --git a/src/lib/math/numbertheory/pow_mod.cpp b/src/lib/math/numbertheory/pow_mod.cpp
index 00917a5f9..02434fe68 100644
--- a/src/lib/math/numbertheory/pow_mod.cpp
+++ b/src/lib/math/numbertheory/pow_mod.cpp
@@ -65,8 +65,8 @@ void Power_Mod::set_modulus(const BigInt& n, Usage_Hints hints, bool disable_mon
 */
 void Power_Mod::set_base(const BigInt& b) const
    {
-   if(b.is_zero() || b.is_negative())
-      throw Invalid_Argument("Power_Mod::set_base: arg must be > 0");
+   if(b.is_negative())
+      throw Invalid_Argument("Power_Mod::set_base: arg must be non-negative");
 
    if(!m_core)
       throw Internal_Error("Power_Mod::set_base: m_core was NULL");
diff --git a/src/lib/tls/msg_client_kex.cpp b/src/lib/tls/msg_client_kex.cpp
index f55568c8e..9cb92447b 100644
--- a/src/lib/tls/msg_client_kex.cpp
+++ b/src/lib/tls/msg_client_kex.cpp
@@ -377,6 +377,10 @@ Client_Key_Exchange::Client_Key_Exchange(const std::vector<uint8_t>& contents,
             else
                m_pre_master = shared_secret;
             }
+         catch(Invalid_Argument& e)
+            {
+            throw TLS_Exception(Alert::ILLEGAL_PARAMETER, e.what());
+            }
          catch(std::exception &)
             {
             /*
diff --git a/src/lib/tls/tls_callbacks.cpp b/src/lib/tls/tls_callbacks.cpp
index 28884c1e2..bcd3aa39b 100644
--- a/src/lib/tls/tls_callbacks.cpp
+++ b/src/lib/tls/tls_callbacks.cpp
@@ -119,7 +119,7 @@ std::pair<secure_vector<uint8_t>, std::vector<uint8_t>> TLS::Callbacks::tls_dh_a
     * advantage to bogus keys anyway.
     */
    if(Y <= 1 || Y >= p - 1)
-      throw TLS_Exception(Alert::INSUFFICIENT_SECURITY,
+      throw TLS_Exception(Alert::ILLEGAL_PARAMETER,
                           "Server sent bad DH key for DHE exchange");
 
    DL_Group group(p, g);
diff --git a/src/lib/tls/tls_channel.cpp b/src/lib/tls/tls_channel.cpp
index b066f649e..9bff836f2 100644
--- a/src/lib/tls/tls_channel.cpp
+++ b/src/lib/tls/tls_channel.cpp
@@ -441,7 +441,8 @@ void Channel::process_handshake_ccs(const secure_vector<uint8_t>& record,
             else if(epoch == sequence_numbers().current_read_epoch() - 1)
                {
                BOTAN_ASSERT(m_active_state, "Have active state here");
-               m_active_state->handshake_io().add_record(unlock(record),
+               m_active_state->handshake_io().add_record(record.data(),
+                                                         record.size(),
                                                          record_type,
                                                          record_sequence);
                }
@@ -460,7 +461,8 @@ void Channel::process_handshake_ccs(const secure_vector<uint8_t>& record,
    // May have been created in above conditional
    if(m_pending_state)
       {
-      m_pending_state->handshake_io().add_record(unlock(record),
+      m_pending_state->handshake_io().add_record(record.data(),
+                                                 record.size(),
                                                  record_type,
                                                  record_sequence);
 
diff --git a/src/lib/tls/tls_handshake_io.cpp b/src/lib/tls/tls_handshake_io.cpp
index 8834e0008..7ac868612 100644
--- a/src/lib/tls/tls_handshake_io.cpp
+++ b/src/lib/tls/tls_handshake_io.cpp
@@ -46,16 +46,17 @@ Protocol_Version Stream_Handshake_IO::initial_record_version() const
    return Protocol_Version::TLS_V10;
    }
 
-void Stream_Handshake_IO::add_record(const std::vector<uint8_t>& record,
+void Stream_Handshake_IO::add_record(const uint8_t record[],
+                                     size_t record_len,
                                      Record_Type record_type, uint64_t)
    {
    if(record_type == HANDSHAKE)
       {
-      m_queue.insert(m_queue.end(), record.begin(), record.end());
+      m_queue.insert(m_queue.end(), record, record + record_len);
       }
    else if(record_type == CHANGE_CIPHER_SPEC)
       {
-      if(record.size() != 1 || record[0] != 1)
+      if(record_len != 1 || record[0] != 1)
          throw Decoding_Error("Invalid ChangeCipherSpec");
 
       // Pretend it's a regular handshake message of zero length
@@ -77,6 +78,9 @@ Stream_Handshake_IO::get_next_record(bool)
          {
          Handshake_Type type = static_cast<Handshake_Type>(m_queue[0]);
 
+         if(type == HANDSHAKE_NONE)
+            throw Decoding_Error("Invalid handshake message type");
+
          std::vector<uint8_t> contents(m_queue.begin() + 4,
                                        m_queue.begin() + length);
 
@@ -181,7 +185,8 @@ bool Datagram_Handshake_IO::timeout_check()
    return true;
    }
 
-void Datagram_Handshake_IO::add_record(const std::vector<uint8_t>& record,
+void Datagram_Handshake_IO::add_record(const uint8_t record[],
+                                       size_t record_len,
                                        Record_Type record_type,
                                        uint64_t record_sequence)
    {
@@ -189,7 +194,7 @@ 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)
+      if(record_len != 1 || record[0] != 1)
          throw Decoding_Error("Invalid ChangeCipherSpec");
 
       // TODO: check this is otherwise empty
@@ -199,28 +204,25 @@ void Datagram_Handshake_IO::add_record(const std::vector<uint8_t>& record,
 
    const size_t DTLS_HANDSHAKE_HEADER_LEN = 12;
 
-   const uint8_t* record_bits = record.data();
-   size_t record_size = record.size();
-
-   while(record_size)
+   while(record_len)
       {
-      if(record_size < DTLS_HANDSHAKE_HEADER_LEN)
+      if(record_len < DTLS_HANDSHAKE_HEADER_LEN)
          return; // completely bogus? at least degenerate/weird
 
-      const uint8_t msg_type = record_bits[0];
-      const size_t msg_len = load_be24(&record_bits[1]);
-      const uint16_t message_seq = load_be<uint16_t>(&record_bits[4], 0);
-      const size_t fragment_offset = load_be24(&record_bits[6]);
-      const size_t fragment_length = load_be24(&record_bits[9]);
+      const uint8_t msg_type = record[0];
+      const size_t msg_len = load_be24(&record[1]);
+      const uint16_t message_seq = load_be<uint16_t>(&record[4], 0);
+      const size_t fragment_offset = load_be24(&record[6]);
+      const size_t fragment_length = load_be24(&record[9]);
 
       const size_t total_size = DTLS_HANDSHAKE_HEADER_LEN + fragment_length;
 
-      if(record_size < total_size)
+      if(record_len < total_size)
          throw Decoding_Error("Bad lengths in DTLS header");
 
       if(message_seq >= m_in_message_seq)
          {
-         m_messages[message_seq].add_fragment(&record_bits[DTLS_HANDSHAKE_HEADER_LEN],
+         m_messages[message_seq].add_fragment(&record[DTLS_HANDSHAKE_HEADER_LEN],
                                               fragment_length,
                                               fragment_offset,
                                               epoch,
@@ -232,8 +234,8 @@ void Datagram_Handshake_IO::add_record(const std::vector<uint8_t>& record,
          // TODO: detect retransmitted flight
          }
 
-      record_bits += total_size;
-      record_size -= total_size;
+      record += total_size;
+      record_len -= total_size;
       }
    }
 
diff --git a/src/lib/tls/tls_handshake_io.h b/src/lib/tls/tls_handshake_io.h
index 8e1a0eca7..66579459d 100644
--- a/src/lib/tls/tls_handshake_io.h
+++ b/src/lib/tls/tls_handshake_io.h
@@ -39,7 +39,8 @@ class Handshake_IO
          const std::vector<uint8_t>& handshake_msg,
          Handshake_Type handshake_type) const = 0;
 
-      virtual void add_record(const std::vector<uint8_t>& record,
+      virtual void add_record(const uint8_t record[],
+                              size_t record_len,
                               Record_Type type,
                               uint64_t sequence_number) = 0;
 
@@ -78,7 +79,8 @@ class Stream_Handshake_IO final : public Handshake_IO
          const std::vector<uint8_t>& handshake_msg,
          Handshake_Type handshake_type) const override;
 
-      void add_record(const std::vector<uint8_t>& record,
+      void add_record(const uint8_t record[],
+                      size_t record_len,
                       Record_Type type,
                       uint64_t sequence_number) override;
 
@@ -118,7 +120,8 @@ class Datagram_Handshake_IO final : public Handshake_IO
          const std::vector<uint8_t>& handshake_msg,
          Handshake_Type handshake_type) const override;
 
-      void add_record(const std::vector<uint8_t>& record,
+      void add_record(const uint8_t record[],
+                      size_t record_len,
                       Record_Type type,
                       uint64_t sequence_number) override;
 
diff --git a/src/scripts/run_tls_fuzzer.py b/src/scripts/run_tls_fuzzer.py
new file mode 100755
index 000000000..4754480a6
--- /dev/null
+++ b/src/scripts/run_tls_fuzzer.py
@@ -0,0 +1,97 @@
+#!/usr/bin/python
+
+import argparse
+import subprocess
+import logging
+import sys
+import os
+import time
+
+def script_is_disabled(script_name):
+    if script_name.find('tls13') >= 0:
+        return True
+    if script_name.find('sslv2') >= 0:
+        return True
+
+    disabled = {
+        'test-SSLv3-padding.py',
+        'test-serverhello-random.py', # assumes support for SSLv2 hello
+        'test-x25519.py', # assumes support for X448 (!)
+    }
+
+    if script_name in disabled:
+        return True
+
+    slow = {
+        'test-bleichenbacher-workaround.py',
+        'test-client-compatibility.py',
+        'test-dhe-key-share-random.py',
+        'test-dhe-no-shared-secret-padding.py',
+        'test-ecdhe-padded-shared-secret.py',
+        'test-ecdhe-rsa-key-share-random.py',
+        'test-fuzzed-plaintext.py',
+        'test-invalid-client-hello-w-record-overflow.py',
+        'test-invalid-client-hello.py',
+        'test-large-hello.py',
+    }
+    if script_name in slow:
+        return True
+
+    return False
+
+def main(args = None):
+    if args is None:
+        args = sys.argv[1:]
+
+    parser = argparse.ArgumentParser()
+
+    # TODO generate key and spawn the server on some random port in tmp dir
+    # TODO support running tls_server binary under valgrind
+
+    parser.add_argument('--verbose', action='store_true', default=False)
+    parser.add_argument('tls-fuzzer-dir')
+
+    args = vars(parser.parse_args(args))
+
+    tlsfuzzer_dir = args['tls-fuzzer-dir']
+
+    if not os.access(tlsfuzzer_dir, os.X_OK):
+        raise Exception("Unable to read TLS fuzzer dir")
+
+    tls_scripts_dir = os.path.join(tlsfuzzer_dir, 'scripts')
+    if not os.access(tlsfuzzer_dir, os.X_OK):
+        raise Exception("Unable to read TLS fuzzer scripts dir")
+
+    scripts = sorted(os.listdir(tls_scripts_dir))
+
+    procs = {}
+
+    for script in scripts:
+        if script_is_disabled(script):
+            logging.debug('Skipping %s' % (script))
+            continue
+
+        procs[script] = subprocess.Popen([sys.executable, os.path.join(tls_scripts_dir, script)],
+                                         cwd=tlsfuzzer_dir,
+                                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+    results = {}
+
+    while len(results) != len(procs):
+        time.sleep(.5)
+        for (script, proc) in procs.items():
+
+            if script in results:
+                continue
+
+            if proc.poll() != None:
+                rv = proc.returncode
+                results[script] = rv
+                if rv == 0:
+                    print("PASS %s" % (script))
+                else:
+                    print("FAIL %s" % (script))
+                sys.stdout.flush()
+
+if __name__ == '__main__':
+    sys.exit(main())


Test patch:
diff --git a/src/tests/data/tls-policy/compat.txt b/src/tests/data/tls-policy/compat.txt
index 2fb831f44..4de9a60a6 100644
--- a/src/tests/data/tls-policy/compat.txt
+++ b/src/tests/data/tls-policy/compat.txt
@@ -2,7 +2,8 @@
 # for interop testing and fuzz testing
 
 # It is based on the default policy, but allows 3DES, SHA-1 signatures,
-# static RSA, and reduces the ephemeral key sizes
+# static RSA, reduces the ephemeral key sizes, and removes some
+# non-standard ciphersuites
 
 allow_tls10 = true
 allow_tls11 = true
@@ -13,8 +14,8 @@ ciphers = ChaCha20Poly1305 AES-256/GCM AES-128/GCM AES-256 AES-128 3DES
 macs = AEAD SHA-256 SHA-384 SHA-1
 signature_hashes = SHA-512 SHA-384 SHA-256 SHA-1
 signature_methods = ECDSA RSA IMPLICIT
-key_exchange_methods = CECPQ1 ECDH DH RSA
-key_exchange_groups = x25519 secp256r1 secp521r1 secp384r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048
+key_exchange_methods = ECDH DH RSA
+key_exchange_groups = x25519 secp256r1 secp521r1 secp384r1 brainpool256r1 brainpool384r1 brainpool512r1 ffdhe/ietf/2048 ffdhe/ietf/3072 ffdhe/ietf/4096 ffdhe/ietf/6144 ffdhe/ietf/8192
 allow_insecure_renegotiation = false
 include_time_in_hello_random = true
 allow_client_initiated_renegotiation = true