Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: RawPublicKey authentication in TLS 1.3 (RFC 7250) #3771

Merged
merged 8 commits into from
Nov 16, 2023

Conversation

reneme
Copy link
Collaborator

@reneme reneme commented Oct 24, 2023

Pull Request Dependencies

Description

This adds support for authentication with "raw public keys" to the TLS 1.3 implementation. This is useful for applications that want to avoid or don't need the complexity of a PKI. Such applications bear the responsibility to manage the trust-relationship of the public key pairs they use.

The commits build the feature from the ground up, starting with the public API, new TLS extensions, integration into Certificate/CertificateVerify messages, and the actual implementation in client and server. I hope this aids in reviewing this patch.

New user-facing APIs

  • TLS::Policy
    • ::accepted_client_certificate_types() / ::accepted_server_certificate_types() determines what trust credentials are supported for client and server authentication respectively. By default, this returns X.509. Applications that want to use raw public keys need to override or configure this accordingly.
  • Credentials_Manager
    • ::find_raw_public_key() is called when usage of raw public keys was negotiated to obtain a public key from the downstream application. This is the very same concept as the existing ::find_cert_chain()
    • ::private_key_for(Public_Key&) is called to obtain the associated private key. It is an overload of the existing ::private_key_for(X509_Certificate&) method.
  • TLS::Callbacks
    • ::tls_verify_raw_public_key() must be implemented by the downstream application to establish trust in a raw public key received from a peer. In contrast to tls_verify_cert_chain() this does not provide a best-effort default implementation; instead it rejects all raw public keys.
  • TLS::Channel
    • ::peer_raw_public_key() returns the raw public key used to authenticate this Channel's active session (or nullptr) if no raw public key was used.
  • TLS::Session_Summary
    • ::peer_raw_public_key() returns the raw public key used to authenticate the session (or nullptr) if no raw public key was used.

Test Examples

GnuTLS Client connects to Botan Server

Start a Botan Server

Create a policy.txt:

allow_tls13 = true
allow_tls12 = false
accepted_server_certificate_types = RawPublicKey

... and run the server:

./botan keygen --algo=RSA > rsa_priv.pem
./botan pkcs8 --pub-out rsa_priv.pem > rsa_pub.pem
./botan tls_server rsa_pub.pem rsa_priv.pem --port=5555 --policy=policy.txt

Connect with a GnuTLS Client

gnutls-cli --port=5555 --priority="NORMAL:+CTYPE-RAWPK" --insecure --print-cert localhost

Botan Client connects to GnuTLS Server

Start a gnutls server

./botan keygen --algo=RSA > rsa_priv.pem
./botan pkcs8 --pub-out rsa_priv.pem > rsa_pub.pem
gnutls-serv --rawpkfile=rsa_pubkey.pem --rawpkkeyfile=rsa_privkey.pem --disable-client-cert --priority="NORMAL:+CTYPE-RAWPK" --port=5555

Connect with a Botan client

Create a policy.txt:

allow_tls13 = true
allow_tls12 = false
accepted_server_certificate_types = RawPublicKey

... and run the client:

# without explicitly trusting the Raw Public Key
./botan tls_client --port=5555 --policy=policy.txt localhost

# with a trusted public key fingerprint
./botan fingerprint --algo=SHA-256 rsa_pub.pem
# -> copy the fingerprint string into the placeholder below:
./botan tls_client --port=5555 --debug --policy=rawpk_policy.txt --trusted-pubkey-sha256="ba:ad:ca:fe:..." localhost

@reneme reneme added the enhancement Enhancement or new feature label Oct 24, 2023
@reneme reneme added this to the Botan 3.3.0 milestone Oct 24, 2023
@reneme reneme self-assigned this Oct 24, 2023
@reneme reneme changed the title Feature: RawPublicKey authentication (RFC 7250) Feature: RawPublicKey authentication in TLS 1.3 (RFC 7250) Oct 25, 2023
@reneme reneme mentioned this pull request Oct 25, 2023
14 tasks
@reneme reneme force-pushed the feature/rfc7250 branch 5 times, most recently from ecc08d9 to 4c38741 Compare November 1, 2023 14:54
@reneme reneme marked this pull request as ready for review November 1, 2023 14:55
@reneme reneme requested a review from lieser November 1, 2023 14:57
@reneme
Copy link
Collaborator Author

reneme commented Nov 1, 2023

This should be ready for a review now.

Two things:

  1. That's the second feature I implemented for TLS 1.3 only (after RecordSizeLimit). I feel that is something we should track somewhere.
  2. It feels inconsistent that we use std::shared_ptr or std::unique_ptr for asymmetric keys. Having those classes explicitly allow/prohibit to be copied and/or moved would be more modern, in my opinion (like X509_Certificate). That might be something to consider for Botan 4.0 when splitting the Public_Key and Private_Key base classes. Where would I note that, so that we don't forget it?

@coveralls
Copy link

coveralls commented Nov 1, 2023

Coverage Status

coverage: 92.061% (-0.008%) from 92.069%
when pulling ba11760 on Rohde-Schwarz:feature/rfc7250
into 201cfe5 on randombit:master.

Copy link
Collaborator

@FAlbertDev FAlbertDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job. Looks very clean! I only left some comments and suggestions.

Do we also want to update doc/cli.rst?

src/lib/tls/tls13/msg_certificate_13.cpp Outdated Show resolved Hide resolved
src/lib/tls/tls_extensions.h Outdated Show resolved Hide resolved
Comment on lines 207 to 208
} else if(cert_type == Certificate_Type::RawPublicKey) {
auto raw_public_key = credentials_manager.find_raw_public_key(key_types, op_type, hostname);

Copy link
Collaborator

@FAlbertDev FAlbertDev Nov 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some check that prevents servers from asking for a RawPublicKey in the server's ClientCertTypeExtension even if the client does not list it in its initial ClientCertTypeExtension? If not, a malicious server could force raw public key client authentication even if the client's policy forbids it (of course, find_raw_public_key must be implemented for this scenario to work).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No there wasn't! Very important catch. Thanks.

src/lib/tls/tls_callbacks.h Outdated Show resolved Hide resolved
src/lib/tls/tls_extensions.cpp Show resolved Hide resolved
src/lib/tls/tls_messages.h Outdated Show resolved Hide resolved
src/cli/tls_helpers.h Show resolved Hide resolved
src/lib/tls/credentials_manager.h Show resolved Hide resolved
src/lib/tls/credentials_manager.h Show resolved Hide resolved
src/lib/tls/tls12/tls_channel_impl_12.h Show resolved Hide resolved
src/lib/tls/tls13/tls_client_impl_13.cpp Outdated Show resolved Hide resolved
src/lib/tls/tls_extensions.cpp Show resolved Hide resolved
src/lib/tls/tls_messages.h Show resolved Hide resolved
src/lib/tls/tls_messages.h Show resolved Hide resolved
src/lib/tls/tls_policy.cpp Show resolved Hide resolved
src/lib/tls/tls_policy.h Show resolved Hide resolved
@reneme
Copy link
Collaborator Author

reneme commented Nov 3, 2023

Thanks for the reviews. I had a quick skim-through. Lot's of valuable remarks, I believe. I'm hoping to get back to this on Monday, but it might slip to Thursday.

reneme and others added 7 commits November 10, 2023 10:14
Co-Authored-By: Fabian Albert <fabian.albert@rohde-schwarz.com>
Co-Authored-By: Fabian Albert <fabian.albert@rohde-schwarz.com>
Co-Authored-By: Fabian Albert <fabian.albert@rohde-schwarz.com>
This allows for both client and server authentication using raw
public keys instead of X.509 certificates.

Co-Authored-By: Fabian Albert <fabian.albert@rohde-schwarz.com>
@reneme
Copy link
Collaborator Author

reneme commented Nov 10, 2023

@FAlbertDev @lieser I addressed your comments. Thanks a lot. You've both found very relevant issues.

@reneme reneme merged commit 20279c6 into randombit:master Nov 16, 2023
38 checks passed
@reneme reneme deleted the feature/rfc7250 branch November 16, 2023 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants