Skip to content

Commit

Permalink
TLS 1.3 Client Authentication in main handshake
Browse files Browse the repository at this point in the history
This allows for client authentication using certificates in the main handshake.
Post-handshake authentication is out of scope of this commit

Co-authored-by: René Meusel <rene.meusel@nexenio.com>
  • Loading branch information
Hannes Rantzsch and reneme committed Apr 27, 2022
1 parent eeb3230 commit 3abfc13
Show file tree
Hide file tree
Showing 22 changed files with 676 additions and 128 deletions.
17 changes: 16 additions & 1 deletion src/bogo_shim/bogo_shim.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ std::string map_to_bogo_error(const std::string& e)
{ "Application data before handshake done", ":APPLICATION_DATA_INSTEAD_OF_HANDSHAKE:" },
{ "Bad Hello_Request, has non-zero size", ":BAD_HELLO_REQUEST:" },
{ "Bad code for TLS alert level", ":UNKNOWN_ALERT_TYPE:" },
{ "Bad encoding on signature algorithms extension", ":DECODE_ERROR:" },
{ "Bad extension size", ":DECODE_ERROR:" },
{ "Bad length in hello verify request", ":DECODE_ERROR:" },
{ "Bad lengths in DTLS header", ":BAD_HANDSHAKE_RECORD:" },
Expand All @@ -99,6 +100,8 @@ std::string map_to_bogo_error(const std::string& e)
{ "Certificate key type did not match ciphersuite", ":WRONG_CERTIFICATE_TYPE:" },
{ "Certificate usage constraints do not allow this ciphersuite", ":KEY_USAGE_BIT_INCORRECT:" },
{ "Certificate: Message malformed", ":DECODE_ERROR:" },
{ "Certificate_Request context must be empty in the main handshake", ":DECODE_ERROR:" },
{ "Certificate_Request message did not provide a signature_algorithms extension", ":DECODE_ERROR:" },
{ "Channel_Impl_12::key_material_export cannot export during renegotiation", "failed to export keying material" },
{ "Client cert verify failed", ":BAD_SIGNATURE:" },
{ "Client certificate does not support signing", ":KEY_USAGE_BIT_INCORRECT:" },
Expand All @@ -123,6 +126,8 @@ std::string map_to_bogo_error(const std::string& e)
{ "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:" },
{ "Failed to agree on a signature algorithm", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
{ "Failed to negotiate a common signature algorithm for client authentication", ":NO_COMMON_SIGNATURE_ALGORITHMS:" },
{ "Finished message didn't verify", ":DIGEST_CHECK_FAILED:" },
{ "Have data remaining in buffer after ClientHello", ":EXCESS_HANDSHAKE_DATA:" },
{ "Have data remaining in buffer after Finished", ":EXCESS_HANDSHAKE_DATA:" },
Expand Down Expand Up @@ -824,7 +829,10 @@ class Shim_Policy final : public Botan::TLS::Policy
{
const Botan::TLS::Signature_Scheme scheme(pref);
if(!scheme.is_available())
{
shim_log("skipping inavailable but preferred signature scheme: " + std::to_string(pref));
continue;
}
pref_hash.push_back(scheme.hash_function_name());
}

Expand Down Expand Up @@ -876,10 +884,17 @@ class Shim_Policy final : public Botan::TLS::Policy
schemes.emplace_back(static_cast<uint16_t>(pref));
}

// BoGo gets sad if these are not included in our signature_algorithms extension
// The relevant tests (*-Sign-Negotiate-*) want to configure a preference
// for the scheme of our signing operation (-signing-prefs). However, this
// policy method (`allowed_signature_schemes`) also restricts the peer's
// signing operation. If we weren't to add a few 'common' algorithms, initial
// security parameter negotiation would fail.
// By placing the BoGo-configured scheme first we make sure our implementation
// meets BoGo's expectation when it is our turn to sign.
if(!m_args.flag_set("server"))
{
schemes.emplace_back(Botan::TLS::Signature_Scheme::RSA_PKCS1_SHA256);
schemes.emplace_back(Botan::TLS::Signature_Scheme::RSA_PSS_SHA256);
schemes.emplace_back(Botan::TLS::Signature_Scheme::ECDSA_SHA256);
}

Expand Down
21 changes: 3 additions & 18 deletions src/bogo_shim/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"PartialFinishedWithServerHelloDone": "Unexpected record vs excess handshake data",
"HelloRetryRequest-DuplicateCurve-TLS13": "expects 'illegal parameter' but we want to stick with 'decode error'",
"HelloRetryRequest-DuplicateCookie-TLS13": "expects 'illegal parameter' but we want to stick with 'decode error'",
"EncryptedExtensionsWithKeyShare-TLS13": "expects 'unsupported extension' but RFC requires 'illegal parameter'"
"EncryptedExtensionsWithKeyShare-TLS13": "expects 'unsupported extension' but RFC requires 'illegal parameter'",
"ClientSkipCertificateVerify-TLS13": "would require ambiguous error mapping"
},

"DisabledTests": {
Expand Down Expand Up @@ -113,23 +114,7 @@
"*EarlyData*": "No TLS 1.3 Early Data, yet",
"TLS13-1RTT-Client-*": "No TLS 1.3 Early Data, yet",

"FailCertCallback-Client-TLS13": "No client auth in TLS 1.3, yet",
"Client-Sign*-TLS13": "No client auth in TLS 1.3, yet",
"TLS13-Client-ClientAuth-": "No client auth in TLS 1.3, yet",
"ClientAuth-*-TLS13": "No client auth in TLS 1.3, yet",
"TLS13-Client-ClientAuth-*": "No client auth in TLS 1.3, yet",
"NoClientCertificate-TLS13": "No client auth in TLS 1.3, yet",
"NoCommonAlgorithms-TLS13": "No client auth in TLS 1.3, yet",
"ClientAuth-*-TLS13-*": "No client auth in TLS 1.3, yet",
"TrailingMessageData-TLS13-CertificateRequest-TLS": "No client auth in TLS 1.3, yet",
"RequestContextInHandshake-TLS13": "No client auth in TLS 1.3, yet",
"UnknownExtensionInCertificateRequest-TLS13": "No client auth in TLS 1.3, yet",
"MissingSignatureAlgorithmsInCertificateRequest-TLS13": "No client auth in TLS 1.3, yet",
"ClientSkipCertificateVerify-TLS13": "No client auth in TLS 1.3, yet",
"SendReceiveIntermediate-Client-TLS13": "No client auth in TLS 1.3, yet",
"TLS13-Client-CertReq-CA-List": "No client auth in TLS 1.3, yet",
"SendNoClientCertificateExtensions-TLS13": "No client auth in TLS 1.3, yet",

"SendNoClientCertificateExtensions-TLS13": "-signed-cert-timestamps currently not supported in the shim",
"KeyUpdate-RequestACK-UnfinishedWrite": "-read-with-unfinished-write currently not supported in the shim",

"*Binder*": "No TLS 1.3",
Expand Down
14 changes: 7 additions & 7 deletions src/lib/tls/msg_cert_req.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

namespace Botan::TLS {

Handshake_Type Certificate_Req::type() const
Handshake_Type Certificate_Request_12::type() const
{
return CERTIFICATE_REQUEST;
}
Expand Down Expand Up @@ -56,7 +56,7 @@ uint8_t cert_type_name_to_code(const std::string& name)
/**
* Create a new Certificate Request message
*/
Certificate_Req::Certificate_Req(Handshake_IO& io,
Certificate_Request_12::Certificate_Request_12(Handshake_IO& io,
Handshake_Hash& hash,
const Policy& policy,
const std::vector<X509_DN>& ca_certs) :
Expand All @@ -70,7 +70,7 @@ Certificate_Req::Certificate_Req(Handshake_IO& io,
/**
* Deserialize a Certificate Request message
*/
Certificate_Req::Certificate_Req(const std::vector<uint8_t>& buf)
Certificate_Request_12::Certificate_Request_12(const std::vector<uint8_t>& buf)
{
if(buf.size() < 4)
throw Decoding_Error("Certificate_Req: Bad certificate request");
Expand Down Expand Up @@ -115,25 +115,25 @@ Certificate_Req::Certificate_Req(const std::vector<uint8_t>& buf)
}
}

const std::vector<std::string>& Certificate_Req::acceptable_cert_types() const
const std::vector<std::string>& Certificate_Request_12::acceptable_cert_types() const
{
return m_cert_key_types;
}

const std::vector<X509_DN>& Certificate_Req::acceptable_CAs() const
const std::vector<X509_DN>& Certificate_Request_12::acceptable_CAs() const
{
return m_names;
}

const std::vector<Signature_Scheme>& Certificate_Req::signature_schemes() const
const std::vector<Signature_Scheme>& Certificate_Request_12::signature_schemes() const
{
return m_schemes;
}

/**
* Serialize a Certificate Request message
*/
std::vector<uint8_t> Certificate_Req::serialize() const
std::vector<uint8_t> Certificate_Request_12::serialize() const
{
std::vector<uint8_t> buf;

Expand Down
94 changes: 74 additions & 20 deletions src/lib/tls/msg_cert_verify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Botan is released under the Simplified BSD License (see license.txt)
*/

#include "botan/internal/stl_util.h"
#include <botan/tls_messages.h>

#include <botan/internal/tls_handshake_io.h>
Expand All @@ -19,14 +20,52 @@

namespace Botan::TLS {

namespace {

std::vector<uint8_t> message(Connection_Side side, const Transcript_Hash& hash)
{
std::vector<uint8_t> msg(64, 0x20);
msg.reserve(64 + 32 + 1 + hash.size());

const std::string context_string = (side == Botan::TLS::Connection_Side::SERVER)
? "TLS 1.3, server CertificateVerify"
: "TLS 1.3, client CertificateVerify";

msg.insert(msg.end(), context_string.cbegin(), context_string.cend());
msg.push_back(0x00);

msg.insert(msg.end(), hash.cbegin(), hash.cend());
return msg;
}

Signature_Scheme choose_signature_scheme(
const Private_Key& key,
const std::vector<Signature_Scheme>& allowed_schemes,
const std::vector<Signature_Scheme>& peer_allowed_schemes)
{
for(Signature_Scheme scheme : allowed_schemes)
{
if(scheme.is_available()
&& scheme.is_suitable_for(key)
&& value_exists(peer_allowed_schemes, scheme))
{
return scheme;
}
}

throw TLS_Exception(Alert::HANDSHAKE_FAILURE, "Failed to agree on a signature algorithm");
}

}

/*
* Create a new Certificate Verify message
* Create a new Certificate Verify message for TLS 1.2
*/
Certificate_Verify::Certificate_Verify(Handshake_IO& io,
Handshake_State& state,
const Policy& policy,
RandomNumberGenerator& rng,
const Private_Key* priv_key)
Certificate_Verify_12::Certificate_Verify_12(Handshake_IO& io,
Handshake_State& state,
const Policy& policy,
RandomNumberGenerator& rng,
const Private_Key* priv_key)
{
BOTAN_ASSERT_NONNULL(priv_key);

Expand Down Expand Up @@ -106,8 +145,35 @@ bool Certificate_Verify_12::verify(const X509_Certificate& cert,

#if defined(BOTAN_HAS_TLS_13)

/*
* Create a new Certificate Verify message for TLS 1.3
*/
Certificate_Verify_13::Certificate_Verify_13(
const std::vector<Signature_Scheme>& peer_allowed_schemes,
Connection_Side whoami,
const Private_Key& key,
const Policy& policy,
const Transcript_Hash& hash,
Callbacks& callbacks,
RandomNumberGenerator& rng)
: m_side(whoami)
{
m_scheme = choose_signature_scheme(key, policy.allowed_signature_schemes(), peer_allowed_schemes);
BOTAN_ASSERT_NOMSG(m_scheme.is_available());

// we need to verify that the provided private key is strong enough for TLS 1.3

m_signature =
callbacks.tls_sign_message(key,
rng,
m_scheme.padding_string(),
m_scheme.format().value(),
message(m_side, hash));
}


Certificate_Verify_13::Certificate_Verify_13(const std::vector<uint8_t>& buf,
const Connection_Side side)
const Connection_Side side)
: Certificate_Verify(buf)
, m_side(side)
{
Expand All @@ -133,24 +199,12 @@ bool Certificate_Verify_13::verify(const X509_Certificate& cert,
if(m_scheme.algorithm_identifier() != cert.subject_public_key_algo())
{ throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Signature algorithm does not match certificate's public key"); }

std::vector<uint8_t> msg(64, 0x20);
msg.reserve(64 + 32 + 1 + transcript_hash.size());

const std::string context_string = (m_side == Botan::TLS::Connection_Side::SERVER)
? "TLS 1.3, server CertificateVerify"
: "TLS 1.3, client CertificateVerify";

msg.insert(msg.end(), context_string.cbegin(), context_string.cend());
msg.push_back(0x00);

msg.insert(msg.end(), transcript_hash.cbegin(), transcript_hash.cend());

const auto key = cert.load_subject_public_key();
const bool signature_valid =
callbacks.tls_verify_message(*key,
m_scheme.padding_string(),
m_scheme.format().value(),
msg,
message(m_side, transcript_hash),
m_signature);

#if defined(BOTAN_UNSAFE_FUZZER_MODE)
Expand Down
35 changes: 32 additions & 3 deletions src/lib/tls/msg_certificate_13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ void Certificate_13::validate_extensions(const std::set<Handshake_Extension_Type
{ throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Certificate Entry contained an extension that was not offered"); }
}

const X509_Certificate& Certificate_13::leaf() const
{
BOTAN_STATE_CHECK(!empty());
return m_entries.front().certificate;
}

void Certificate_13::verify(Callbacks& callbacks,
const Policy& policy,
Credentials_Manager& creds,
Expand Down Expand Up @@ -90,6 +96,17 @@ void Certificate_13::verify(Callbacks& callbacks,
callbacks.tls_verify_cert_chain(certs, ocsp_responses, trusted_CAs, usage, hostname, policy);
}

Certificate_13::Certificate_13(const std::vector<X509_Certificate>& certs,
const Connection_Side side,
const std::vector<uint8_t> &request_context)
: m_request_context(request_context)
, m_side(side)
{
// TODO: proper extensions
for (const auto& c : certs)
{ m_entries.emplace_back(Certificate_Entry{c, Extensions()}); }
}

/**
* Deserialize a Certificate message
*/
Expand Down Expand Up @@ -155,7 +172,7 @@ Certificate_13::Certificate_13(const std::vector<uint8_t>& buf,
// message as well.
if(entry.extensions.contains_implemented_extensions_other_than({
TLSEXT_CERT_STATUS_REQUEST,
// SIGNED_CERTIFICATE_TIMESTAMP
// TLSEXT_SIGNED_CERTIFICATE_TIMESTAMP
}))
{
throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Certificate Entry contained an extension that is not allowed");
Expand Down Expand Up @@ -196,8 +213,20 @@ Certificate_13::Certificate_13(const std::vector<uint8_t>& buf,
*/
std::vector<uint8_t> Certificate_13::serialize() const
{
// Needed only for server implementation or client authentication
throw Not_Implemented("NYI");
std::vector<uint8_t> buf;

append_tls_length_value(buf, m_request_context, 1);

std::vector<uint8_t> entries;
for(const auto& entry : m_entries)
{
append_tls_length_value(entries, entry.certificate.BER_encode(), 3);
append_tls_length_value(entries, entry.extensions.serialize(m_side), 2);
}

append_tls_length_value(buf, entries, 3);

return buf;
}

}
Loading

0 comments on commit 3abfc13

Please sign in to comment.