From 406c57f09eac849c10807b74c8e7ba051a6a5c2c Mon Sep 17 00:00:00 2001 From: Jack Lloyd Date: Fri, 13 Nov 2015 12:52:20 -0500 Subject: Add TLS_PSK tests Fix a bug which rejected any short server key exchanges. These can occur with a plain PSK with short or empty identity hints. Disable SHA-224 by default. Remove some vestigal RC4 cruft. Push more on the TLS corruption tests. --- doc/manual/tls.rst | 3 +- doc/news.rst | 8 ++ src/lib/tls/msg_server_kex.cpp | 5 +- src/lib/tls/tls_policy.cpp | 7 +- src/lib/tls/tls_suite_info.cpp | 7 +- src/scripts/tls_suite_info.py | 33 +++--- src/tests/tests.cpp | 4 +- src/tests/unit_tls.cpp | 242 ++++++++++++++++++++++++++++++++--------- 8 files changed, 220 insertions(+), 89 deletions(-) diff --git a/doc/manual/tls.rst b/doc/manual/tls.rst index f96f27620..1926d5c08 100644 --- a/doc/manual/tls.rst +++ b/doc/manual/tls.rst @@ -560,8 +560,9 @@ policy settings from a file. Returns the list of algorithms we are willing to use for public key signatures, in order of preference. - Default: "SHA-512", "SHA-384", "SHA-256", "SHA-224" + Default: "SHA-512", "SHA-384", "SHA-256" + Also allowed: "SHA-224" Also allowed (although **not recommended**): "MD5", "SHA-1" .. note:: diff --git a/doc/news.rst b/doc/news.rst index 240d5e67d..f45de0bae 100644 --- a/doc/news.rst +++ b/doc/news.rst @@ -17,6 +17,14 @@ Version 1.11.25, Not Yet Released * Fixed the signature of botan_pubkey_destroy which took the wrong type and was not usable. +* The TLS client would erronously reject any server key exchange + packet smaller than 6 bytes. This prevented negotiating a plain PSK + TLS ciphersuite with an empty identity hint. ECDHE_PSK and DHE_PSK + suites were not affected. + +* Support for negotiating use of SHA-224 in TLS has been disabled in the + default policy. + Version 1.11.24, 2015-11-04 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/lib/tls/msg_server_kex.cpp b/src/lib/tls/msg_server_kex.cpp index 3fcdb5ab2..50caf3288 100644 --- a/src/lib/tls/msg_server_kex.cpp +++ b/src/lib/tls/msg_server_kex.cpp @@ -1,6 +1,6 @@ /* * Server Key Exchange Message -* (C) 2004-2010,2012 Jack Lloyd +* (C) 2004-2010,2012,2015 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -145,9 +145,6 @@ Server_Key_Exchange::Server_Key_Exchange(const std::vector& buf, Protocol_Version version) : m_kex_key(nullptr), m_srp_params(nullptr) { - if(buf.size() < 6) - throw Decoding_Error("Server_Key_Exchange: Packet corrupted"); - TLS_Data_Reader reader("ServerKeyExchange", buf); /* diff --git a/src/lib/tls/tls_policy.cpp b/src/lib/tls/tls_policy.cpp index d8dd2c828..7d1af71ef 100644 --- a/src/lib/tls/tls_policy.cpp +++ b/src/lib/tls/tls_policy.cpp @@ -1,6 +1,6 @@ /* * Policies for TLS -* (C) 2004-2010,2012 Jack Lloyd +* (C) 2004-2010,2012,2015 Jack Lloyd * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -44,7 +44,7 @@ std::vector Policy::allowed_signature_hashes() const "SHA-512", "SHA-384", "SHA-256", - "SHA-224", + //"SHA-224", //"SHA-1", //"MD5", }; @@ -282,9 +282,6 @@ std::vector Policy::ciphersuite_list(Protocol_Version version, if(!have_srp && suite.kex_algo() == "SRP_SHA") continue; - if(version.is_datagram_protocol() && suite.cipher_algo() == "RC4") - continue; - if(!version.supports_aead_modes() && suite.mac_algo() == "AEAD") continue; diff --git a/src/lib/tls/tls_suite_info.cpp b/src/lib/tls/tls_suite_info.cpp index cb5c1d4c5..5aff035b9 100644 --- a/src/lib/tls/tls_suite_info.cpp +++ b/src/lib/tls/tls_suite_info.cpp @@ -2,8 +2,8 @@ * TLS cipher suite information * * This file was automatically generated from the IANA assignments -* (tls-parameters.txt hash 4bc98b6f75ad5b63952b5f457fa7adbfef60f095) -* by ./src/scripts/tls_suite_info.py on 2015-05-11 +* (tls-parameters.txt hash 6a934405ed41aa4d6113dad17f815867741430ac) +* by ./src/scripts/tls_suite_info.py on 2015-11-13 * * Botan is released under the Simplified BSD License (see license.txt) */ @@ -57,9 +57,6 @@ Ciphersuite Ciphersuite::by_id(u16bit suite) case 0xC081: // DHE_DSS_WITH_CAMELLIA_256_GCM_SHA384 return Ciphersuite(0xC081, "DSA", "DH", "Camellia-256/GCM", 32, 4, 8, "AEAD", 0, "SHA-384"); - case 0x0066: // DHE_DSS_WITH_RC4_128_SHA - return Ciphersuite(0x0066, "DSA", "DH", "RC4", 16, 0, 0, "SHA-1", 20); - case 0x0099: // DHE_DSS_WITH_SEED_CBC_SHA return Ciphersuite(0x0099, "DSA", "DH", "SEED", 16, 16, 0, "SHA-1", 20); diff --git a/src/scripts/tls_suite_info.py b/src/scripts/tls_suite_info.py index 8589ddeec..dc0468c88 100755 --- a/src/scripts/tls_suite_info.py +++ b/src/scripts/tls_suite_info.py @@ -3,7 +3,7 @@ """ Used to generate lib/tls/tls_suite_info.cpp from IANA params -(C) 2011, 2012, 2013, 2014 Jack Lloyd +(C) 2011, 2012, 2013, 2014, 2015 Jack Lloyd Botan is released under the Simplified BSD License (see license.txt) """ @@ -53,7 +53,6 @@ def to_ciphersuite_info(code, name): mac_algo = 'SHA256' cipher_info = { - 'RC4': ('RC4',None), 'CHACHA20': ('ChaCha',32), 'IDEA': ('IDEA',16), 'DES': ('DES',8), @@ -72,7 +71,6 @@ def to_ciphersuite_info(code, name): 'SHA384': 'SHA-384', 'SHA512': 'SHA-512', - 'RC4': 'RC4', 'CHACHA': 'ChaCha', '3DES': 'TripleDES', @@ -122,28 +120,26 @@ def to_ciphersuite_info(code, name): return 'Ciphersuite(0x%s, "%s", "%s", "%s", %d, %d, %d, "AEAD", %d, "%s")' % ( code, sig_algo, kex_algo, "ChaCha20Poly1305", cipher_keylen, iv_len, 0, 0, mac_algo) - stream_ciphers = ['RC4'] + mode = cipher[-1] + if mode not in ['CBC', 'GCM', 'CCM(8)', 'CCM', 'OCB']: + print "#warning Unknown mode %s" % (' '.join(cipher)) - if cipher_algo not in stream_ciphers: - mode = cipher[-1] - if mode not in ['CBC', 'GCM', 'CCM(8)', 'CCM', 'OCB']: - print "#warning Unknown mode %s" % (' '.join(cipher)) + ivlen = 8 if cipher_algo == '3DES' else 16 - ivlen = 8 if cipher_algo == '3DES' else 16 - - if mode != 'CBC': - if mode == 'OCB': - cipher_algo += '/OCB(12)' - else: - cipher_algo += '/' + mode + if mode != 'CBC': + if mode == 'OCB': + cipher_algo += '/OCB(12)' + else: + cipher_algo += '/' + mode - if cipher_algo in stream_ciphers or mode == 'CBC': + if mode == 'CBC': return 'Ciphersuite(0x%s, "%s", "%s", "%s", %d, %d, 0, "%s", %d)' % ( code, sig_algo, kex_algo, cipher_algo, cipher_keylen, ivlen, mac_algo, mac_keylen[mac_algo]) - elif mode == 'OCB': + elif mode == 'OCB': return 'Ciphersuite(0x%s, "%s", "%s", "%s", %d, %d, %d, "AEAD", %d, "%s")' % ( code, sig_algo, kex_algo, cipher_algo, cipher_keylen, 4, 0, 0, mac_algo) + else: iv_bytes_from_hs = 4 iv_bytes_from_rec = 8 @@ -242,9 +238,6 @@ def main(args = None): def define_custom_ciphersuite(name, code): suites[name] = (code, to_ciphersuite_info(code, name)) - # From http://tools.ietf.org/html/draft-ietf-tls-56-bit-ciphersuites-01 - define_custom_ciphersuite('DHE_DSS_WITH_RC4_128_SHA', '0066') - if options.with_chacha: # Google servers - draft-agl-tls-chacha20poly1305-04 define_custom_ciphersuite('ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256', 'CC13') diff --git a/src/tests/tests.cpp b/src/tests/tests.cpp index 55c9e3824..294ef303b 100644 --- a/src/tests/tests.cpp +++ b/src/tests/tests.cpp @@ -163,8 +163,8 @@ bool Test::Result::test_eq(const char* producer, const char* what, bits_different += Botan::hamming_weight(xor_diff[i]); } - err << "Produced: " << Botan::hex_encode(produced, produced_size) << "\n" - << "Expected: " << Botan::hex_encode(expected, expected_size); + err << "\nProduced: " << Botan::hex_encode(produced, produced_size) + << "\nExpected: " << Botan::hex_encode(expected, expected_size); if(bits_different > 0) { diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp index 8502352fc..a5ec9e0d5 100644 --- a/src/tests/unit_tls.cpp +++ b/src/tests/unit_tls.cpp @@ -99,7 +99,14 @@ class Credentials_Manager_Test : public Botan::Credentials_Manager { if(type == "tls-server" && context == "session-ticket") return Botan::SymmetricKey("AABBCCDDEEFF012345678012345678"); - throw std::runtime_error("No PSK set for " + context); + + if(context == "server.example.com" && type == "tls-client") + return Botan::SymmetricKey("20B602D1475F2DF888FCB60D2AE03AFD"); + + if(context == "server.example.com" && type == "tls-server") + return Botan::SymmetricKey("20B602D1475F2DF888FCB60D2AE03AFD"); + + throw std::runtime_error("No PSK set for " + type + "/" + context); } public: @@ -167,7 +174,7 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, Botan::TLS::Session_Manager_In_Memory server_sessions(rng); Botan::TLS::Session_Manager_In_Memory client_sessions(rng); - Test::Result result("TLS handshake"); + Test::Result result(offer_version.to_string()); for(size_t r = 1; r <= 4; ++r) { @@ -178,6 +185,7 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, auto handshake_complete = [&](const Botan::TLS::Session& session) -> bool { handshake_done = true; + result.test_note("Session established " + session.version().to_string() + " " + session.ciphersuite().to_string() + " " + Botan::hex_encode(session.session_id())); @@ -192,9 +200,12 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, }; auto next_protocol_chooser = [&](std::vector protos) { - result.test_eq("protocol count", protos.size(), 2); - result.test_eq("protocol[0]", protos[0], "test/1"); - result.test_eq("protocol[1]", protos[1], "test/2"); + if(r <= 2) + { + result.test_eq("protocol count", protos.size(), 2); + result.test_eq("protocol[0]", protos[0], "test/1"); + result.test_eq("protocol[1]", protos[1], "test/2"); + } return "test/3"; }; @@ -235,7 +246,8 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, if(rounds > 25) { - result.test_failure("Still here after many rounds, deadlock?"); + if(r <= 2) + result.test_failure("Still here after many rounds, deadlock?"); break; } @@ -261,10 +273,10 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, server.send(server_sent); } - const bool corrupt_client_data = (r == 3 && c2s_traffic.size() && rng.next_byte() % 3 == 0 && rounds > 1); - const bool corrupt_server_data = (r == 4 && s2c_traffic.size() && rng.next_byte() % 3 == 0 && rounds > 1); + const bool corrupt_client_data = (r == 3 && (rng.next_byte() <= 128 || rounds > 2)); + const bool corrupt_server_data = (r == 4 && (rng.next_byte() <= 128 || rounds > 2)); - try + if(c2s_traffic.size() > 0) { /* * Use this as a temp value to hold the queues as otherwise they @@ -274,43 +286,66 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, std::vector input; std::swap(c2s_traffic, input); - if(corrupt_server_data) - input = Test::mutate_vec(input); - - server.received_data(input.data(), input.size()); - } - catch(std::exception& e) - { if(corrupt_server_data) { - result.test_note("corruption caused server exception"); + try + { + input = Test::mutate_vec(input, true); + size_t needed = server.received_data(input.data(), input.size()); + + if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024)) + { + input.resize(needed); + Test::rng().randomize(input.data(), input.size()); + needed = server.received_data(input.data(), input.size()); + result.test_eq("no more data needed now", needed, 0); + } + } + catch(std::exception& e) + { + result.test_note("corruption caused server exception"); + } } else { - result.test_failure("server error", e.what()); + size_t needed = server.received_data(input.data(), input.size()); + result.test_eq("full packet received", needed, 0); } + continue; } - try + if(s2c_traffic.size() > 0) { std::vector input; std::swap(s2c_traffic, input); - if(corrupt_client_data) - input = Test::mutate_vec(input); - client.received_data(input.data(), input.size()); - } - catch(std::exception& e) - { if(corrupt_client_data) { - result.test_note("corruption caused client exception"); + try + { + input = Test::mutate_vec(input, true); + size_t needed = client.received_data(input.data(), input.size()); + + if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024)) + { + input.resize(needed); + Test::rng().randomize(input.data(), input.size()); + needed = client.received_data(input.data(), input.size()); + result.test_eq("no more data needed now", needed, 0); + } + } + catch(std::exception& e) + { + result.test_note("corruption caused client exception"); + } } else { - result.test_failure("client error", e.what()); + size_t needed = client.received_data(input.data(), input.size()); + result.test_eq("full packet received", needed, 0); } + continue; } @@ -324,6 +359,14 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, result.test_eq("server recv", server_recv, client_sent); } + if(r > 2) + { + if(client_recv.size() && server_recv.size()) + { + result.test_failure("Negotiated in the face of data corruption " + std::to_string(r)); + } + } + if(client.is_closed() && server.is_closed()) break; @@ -343,7 +386,14 @@ Test::Result test_tls_handshake(Botan::TLS::Protocol_Version offer_version, } catch(std::exception& e) { - result.test_failure("TLS client", e.what()); + if(r > 2) + { + result.test_note("Corruption caused exception"); + } + else + { + result.test_failure("TLS client", e.what()); + } } } @@ -361,7 +411,7 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version, Botan::TLS::Session_Manager_In_Memory server_sessions(rng); Botan::TLS::Session_Manager_In_Memory client_sessions(rng); - Test::Result result("DTLS handshake"); + Test::Result result(offer_version.to_string()); for(size_t r = 1; r <= 2; ++r) { @@ -379,9 +429,12 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version, }; auto next_protocol_chooser = [&](std::vector protos) { - result.test_eq("protocol count", protos.size(), 2); - result.test_eq("protocol[0]", protos[0], "test/1"); - result.test_eq("protocol[1]", protos[1], "test/2"); + if(r <= 2) + { + result.test_eq("protocol count", protos.size(), 2); + result.test_eq("protocol[0]", protos[0], "test/1"); + result.test_eq("protocol[1]", protos[1], "test/2"); + } return "test/3"; }; @@ -450,10 +503,10 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version, server.send(server_sent); } - const bool corrupt_client_data = (r == 3 && c2s_traffic.size() && rng.next_byte() % 3 == 0 && rounds < 10); - const bool corrupt_server_data = (r == 4 && s2c_traffic.size() && rng.next_byte() % 3 == 0 && rounds < 10); + const bool corrupt_client_data = (r == 3 && rng.next_byte() % 3 <= 1 && rounds < 10); + const bool corrupt_server_data = (r == 4 && rng.next_byte() % 3 <= 1 && rounds < 10); - try + if(c2s_traffic.size() > 0) { /* * Use this as a temp value to hold the queues as otherwise they @@ -463,30 +516,80 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version, std::vector input; std::swap(c2s_traffic, input); - if(corrupt_client_data) - input = Test::mutate_vec(input); + if(corrupt_server_data) + { + try + { + input = Test::mutate_vec(input, true); + size_t needed = server.received_data(input.data(), input.size()); + + if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024)) + { + input.resize(needed); + Test::rng().randomize(input.data(), input.size()); + needed = client.received_data(input.data(), input.size()); + result.test_eq("no more data needed now", needed, 0); + } + } + catch(std::exception& e) + { + result.test_note("corruption caused server exception"); + } + } + else + { + try + { + size_t needed = server.received_data(input.data(), input.size()); + result.test_eq("full packet received", needed, 0); + } + catch(std::exception& e) + { + result.test_failure("server error", e.what()); + } + } - server.received_data(input.data(), input.size()); - } - catch(std::exception& e) - { - result.test_failure("server error", e.what()); continue; } - try + if(s2c_traffic.size() > 0) { std::vector input; std::swap(s2c_traffic, input); - if(corrupt_server_data) - input = Test::mutate_vec(input); + if(corrupt_client_data) + { + try + { + input = Test::mutate_vec(input, true); + size_t needed = client.received_data(input.data(), input.size()); + + if(needed > 0 && result.test_lt("Never requesting more than max protocol len", needed, 18*1024)) + { + input.resize(needed); + Test::rng().randomize(input.data(), input.size()); + needed = client.received_data(input.data(), input.size()); + result.test_eq("no more data needed now", needed, 0); + } + } + catch(std::exception& e) + { + result.test_note("corruption caused client exception"); + } + } + else + { + try + { + size_t needed = client.received_data(input.data(), input.size()); + result.test_eq("full packet received", needed, 0); + } + catch(std::exception& e) + { + result.test_failure("client error", e.what()); + } + } - client.received_data(input.data(), input.size()); - } - catch(std::exception& e) - { - result.test_failure("client error", e.what()); continue; } @@ -525,7 +628,14 @@ Test::Result test_dtls_handshake(Botan::TLS::Protocol_Version offer_version, } catch(std::exception& e) { - result.test_failure("DTLS handshake", e.what()); + if(r > 2) + { + result.test_note("Corruption caused failure"); + } + else + { + result.test_failure("DTLS handshake", e.what()); + } } } @@ -543,6 +653,7 @@ class Test_Policy : public Botan::TLS::Text_Policy size_t dtls_maximum_timeout() const override { return 8; } }; + class TLS_Unit_Tests : public Test { public: @@ -553,6 +664,7 @@ class TLS_Unit_Tests : public Test Test_Policy policy; results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy)); + results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy)); results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy)); @@ -568,6 +680,7 @@ class TLS_Unit_Tests : public Test policy.set("key_exchange_methods", "DH"); results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V10, *basic_creds, policy)); results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V11, *basic_creds, policy)); + policy.set("key_exchange_methods", "ECDH"); results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy)); @@ -580,14 +693,39 @@ class TLS_Unit_Tests : public Test results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V10, *basic_creds, policy)); results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); +#if defined(BOTAN_HAS_AEAD_OCB) + policy.set("ciphers", "AES-128/OCB(12)"); + results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); + results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); +#endif + +#if defined(BOTAN_HAS_AEAD_CHACHA20_POLY1305) policy.set("ciphers", "ChaCha20Poly1305"); results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); +#endif + + policy.set("ciphers", "AES-128/GCM"); + policy.set("key_exchange_methods", "PSK"); + results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); + results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); + + // For whatever reason no (EC)DHE_PSK GCM ciphersuites are defined + policy.set("ciphers", "AES-128"); + policy.set("key_exchange_methods", "ECDHE_PSK"); + results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); + results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); + + policy.set("key_exchange_methods", "DHE_PSK"); + results.push_back(test_tls_handshake(Botan::TLS::Protocol_Version::TLS_V12, *basic_creds, policy)); + results.push_back(test_dtls_handshake(Botan::TLS::Protocol_Version::DTLS_V12, *basic_creds, policy)); + return results; } + }; -BOTAN_REGISTER_TEST("unit_tls", TLS_Unit_Tests); +BOTAN_REGISTER_TEST("tls", TLS_Unit_Tests); #endif -- cgit v1.2.3