Skip to content

Commit

Permalink
handle cipher suites for TLS 1.3 correctly
Browse files Browse the repository at this point in the history
inspect sig schemes extensions for rfc 8448 test

Co-authored-by: René Meusel <rene.meusel@nexenio.com>
  • Loading branch information
Hannes Rantzsch and reneme committed Jan 3, 2022
1 parent 2990aa8 commit 9bf89b3
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 34 deletions.
14 changes: 13 additions & 1 deletion src/lib/tls/tls_ciphersuite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,19 @@ bool Ciphersuite::ecc_ciphersuite() const

bool Ciphersuite::usable_in_version(Protocol_Version version) const
{
BOTAN_UNUSED(version);
// RFC 8446 B.4.:
// Although TLS 1.3 uses the same cipher suite space as previous
// versions of TLS, TLS 1.3 cipher suites are defined differently, only
// specifying the symmetric ciphers, and cannot be used for TLS 1.2.
// Similarly, cipher suites for TLS 1.2 and lower cannot be used with
// TLS 1.3.
if((!version.is_datagram_protocol() && version > Protocol_Version(Protocol_Version::TLS_V12)) ||
( version.is_datagram_protocol() && version > Protocol_Version(Protocol_Version::DTLS_V12)))
{
// currently cipher suite codes {0x13,0x01} through {0x13,0x05} are
// allowed for TLS 1.3. This may change in the future.
return (ciphersuite_code() & 0xFF00) == 0x1300;
}
return true;
}

Expand Down
51 changes: 26 additions & 25 deletions src/lib/tls/tls_policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,6 @@ std::vector<std::string> Policy::allowed_key_exchange_methods() const
"ECDH",
"DH",
//"RSA",
//"IMPLICIT",
//TODO: allow TLS 1.3 ciphers with UNDEFINED kex algo
"UNDEFINED",
};
}

Expand All @@ -111,8 +108,6 @@ std::vector<std::string> Policy::allowed_signature_methods() const
"RSA",
//"DSA",
//"IMPLICIT",
//TODO: allow TLS 1.3 ciphers with UNDEFINED sig meth
"UNDEFINED",
};
}

Expand Down Expand Up @@ -452,31 +447,37 @@ std::vector<uint16_t> Policy::ciphersuite_list(Protocol_Version version) const
if(!this->acceptable_ciphersuite(suite))
continue;

if(!value_exists(kex, suite.kex_algo()))
continue; // unsupported key exchange

if(!value_exists(ciphers, suite.cipher_algo()))
continue; // unsupported cipher

if(!value_exists(macs, suite.mac_algo()))
continue; // unsupported MAC algo

if(!value_exists(sigs, suite.sig_algo()))
// these checks are irrelevant for TLS 1.3
// TODO: consider making a method for this logic
if((!version.is_datagram_protocol() && version <= Protocol_Version(Protocol_Version::TLS_V12)) ||
( version.is_datagram_protocol() && version <= Protocol_Version(Protocol_Version::DTLS_V12)))
{
// allow if it's an empty sig algo and we want to use PSK
if(suite.auth_method() != Auth_Method::IMPLICIT || !suite.psk_ciphersuite())
continue;
}
if(!value_exists(kex, suite.kex_algo()))
continue; // unsupported key exchange

/*
CECPQ1 always uses x25519 for ECDH, so treat the applications
removal of x25519 from the ECC curve list as equivalent to
saying they do not trust CECPQ1
*/
if(suite.kex_method() == Kex_Algo::CECPQ1)
{
if(value_exists(key_exchange_groups(), Group_Params::X25519) == false)
continue;
if(!value_exists(macs, suite.mac_algo()))
continue; // unsupported MAC algo

if(!value_exists(sigs, suite.sig_algo()))
{
// allow if it's an empty sig algo and we want to use PSK
if(suite.auth_method() != Auth_Method::IMPLICIT || !suite.psk_ciphersuite())
continue;
}

/*
CECPQ1 always uses x25519 for ECDH, so treat the applications
removal of x25519 from the ECC curve list as equivalent to
saying they do not trust CECPQ1
*/
if(suite.kex_method() == Kex_Algo::CECPQ1)
{
if(value_exists(key_exchange_groups(), Group_Params::X25519) == false)
continue;
}
}

// OK, consider it
Expand Down
18 changes: 17 additions & 1 deletion src/lib/tls/tls_version.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class BOTAN_PUBLIC_API(2,0) Protocol_Version final
public:
enum Version_Code {
TLS_V12 = 0x0303,
#if defined(BOTAN_HAS_TLS_13)
#if defined(BOTAN_HAS_TLS_13) // TODO consider not to ifdef this
TLS_V13 = 0x0304,
#endif
DTLS_V12 = 0xFEFD
Expand Down Expand Up @@ -131,6 +131,22 @@ class BOTAN_PUBLIC_API(2,0) Protocol_Version final
return (*this == other || *this > other);
}

/**
* @return if this version is earlier to other
*/
bool operator<(const Protocol_Version& other) const
{
return !(*this >= other);
}

/**
* @return if this version is earlier than or equal to other
*/
bool operator<=(const Protocol_Version& other) const
{
return (*this == other || *this < other);
}

private:
uint16_t m_version;
};
Expand Down
5 changes: 2 additions & 3 deletions src/tests/data/tls-policy/rfc8448.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ allow_dtls10 = false
allow_dtls12 = false
ciphers = AES-128/GCM ChaCha20Poly1305 AES-256/GCM
macs = AEAD
signature_hashes = SHA-256 SHA-384
signature_methods = UNDEFINED
key_exchange_methods = UNDEFINED
signature_hashes = SHA-512 SHA-384 SHA-256
signature_methods = ECDSA RSA
key_exchange_groups = x25519 secp256r1 secp384r1 secp521r1 ffdhe/ietf/2048 ffdhe/ietf/3072 ffdhe/ietf/4096 ffdhe/ietf/6144 ffdhe/ietf/8192
key_exchange_groups_to_offer = x25519
allow_insecure_renegotiation = false
Expand Down
39 changes: 35 additions & 4 deletions src/tests/test_tls_rfc8448.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include <botan/tls_version.h>

#include <botan/assert.h>
#include <botan/internal/stl_util.h>

#include <botan/internal/loadstor.h>
#endif
Expand Down Expand Up @@ -111,7 +112,34 @@ decltype(auto) parse_extensions(const std::vector<uint8_t> &exts_buffer, Test::R
}

return exts;
};
}

void compare_signature_scheme_extensions(const std::vector<uint8_t> &produced_schemes, const std::vector<uint8_t> &expected_schemes, Test::Result &result)
{
auto preader = Botan::TLS::TLS_Data_Reader("Produced Signature_Algorithms", produced_schemes);
auto ps = Botan::TLS::Signature_Algorithms(preader, produced_schemes.size()).supported_schemes();

auto ereader = Botan::TLS::TLS_Data_Reader("Expected Signature_Algorithms", expected_schemes);
auto es = Botan::TLS::Signature_Algorithms(ereader, expected_schemes.size()).supported_schemes();

for (const auto& scheme : es)
{
if (!Botan::TLS::signature_scheme_is_known(scheme))
// do not check for schemes Botan doesn't support
continue;

if (!result.confirm("expected scheme is present", Botan::value_exists(ps, scheme)))
result.test_note(std::string("did not produce expected signature scheme: ") + Botan::TLS::sig_scheme_to_string(scheme));
}

for (const auto& scheme : ps)
{
if (!result.confirm("produced scheme was expected", Botan::value_exists(es, scheme)))
result.test_note(std::string("produced unexpected signature scheme: ") + Botan::TLS::sig_scheme_to_string(scheme));
}

// TODO: the order of schemes is not checked
}

void compare_extensions(const std::vector<uint8_t> &exts_buffer, const std::vector<uint8_t> &exp_exts_buffer, Test::Result &result)
{
Expand All @@ -121,20 +149,23 @@ void compare_extensions(const std::vector<uint8_t> &exts_buffer, const std::vect
for (const auto &eext : exp)
{
const auto &pext = prod.find(eext.first);
if (!result.confirm("extension was missing", pext != prod.end())) {
if (!result.confirm("expected extension is present", pext != prod.end())) {
result.test_note(std::string("expected to produce TLS extension: ") + std::to_string(eext.first));
}
}

for (const auto &pext : prod)
{
const auto &eext = exp.find(pext.first);
if (!result.confirm("extension was expected", eext != exp.end())) {
if (!result.confirm("produced extension was expected", eext != exp.end())) {
result.test_note(std::string("did not expect to produce TLS extension: ") + std::to_string(pext.first));
continue;
}

result.test_eq(std::string("content of extension type ") + std::to_string(pext.first), pext.second, eext->second);
if (pext.first == Botan::TLS::Handshake_Extension_Type::TLSEXT_SIGNATURE_ALGORITHMS)
compare_signature_scheme_extensions(pext.second, eext->second, result);
else
result.test_eq(std::string("content of extension type ") + std::to_string(pext.first), pext.second, eext->second);
}
}

Expand Down

0 comments on commit 9bf89b3

Please sign in to comment.