Skip to content

Commit

Permalink
Implement ECDSA keys and use them by default (#563)
Browse files Browse the repository at this point in the history
* Implement ECDSA keys and use them by default

ECDSA keys can be generated an order of magnitude faster than RSA keys.
This commit adds an implementation for ECDSA, and adds parameters to
CA.__init__, CA.issue_certificate and CA.create_child_ca. The default is
ECDSA, given that OpenSSL and browsers support this for at least 10
years. This also adds an option to the cli to switch between RSA and ECDSA.

The basic test and the SSL server test have been parametrized over both
types of keys. This ensures that RSA support is not accidentally broken,
while keeping most of the tests fast.

* Drop types-cryptography in lint

Cryptography provides types since version 3.4.4 and the relevant
interfaces have been typed since version 35.0.0

* Remove check on keytype when loading from file

This would be a breaking change and it makes codecov happier. This could
be reintroduced in the future using a larger set of allowed keys, for
example using
cryptography.hazmat.primitives.asymmetric.types.CertificatePublicKeyTypes.

* Add newsfragment for ECDSA feature

* Add no cover else branch to _generate_key

There doesn't seem to be a way to tell coverage to ignore the the case
when nothing matches in a if-elif group.

---------

Co-authored-by: Quentin Pradet <quentin.pradet@gmail.com>
  • Loading branch information
VincentVanlaer and pquentin committed Apr 19, 2023
1 parent a3e352a commit 1571357
Show file tree
Hide file tree
Showing 7 changed files with 88 additions and 48 deletions.
3 changes: 3 additions & 0 deletions docs/source/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ API reference

.. automethod:: write_to_path

.. autoclass:: KeyType
:members: RSA, ECDSA
:undoc-members:

Change history
==============
Expand Down
3 changes: 1 addition & 2 deletions lint-requirements.in
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
mypy==0.910
# TODO: Switch to cryptography>=35.0.0 once it's released.
types-cryptography>=3.3.3
cryptography>=35.0.0
types-pyopenssl>=20.0.4
pytest>=6.2
idna>=3.2
14 changes: 8 additions & 6 deletions lint-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#
# This file is autogenerated by pip-compile with python 3.7
# To update, run:
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
# pip-compile lint-requirements.in
#
cffi==1.15.1
# via cryptography
cryptography==40.0.2
# via types-pyopenssl
# via
# -r lint-requirements.in
# types-pyopenssl
idna==3.4
# via -r lint-requirements.in
iniconfig==2.0.0
Expand All @@ -26,9 +28,9 @@ pytest==7.3.1
# via -r lint-requirements.in
toml==0.10.2
# via mypy
types-cryptography==3.3.23
# via -r lint-requirements.in
types-pyopenssl==23.0.0.3
tomli==2.0.1
# via pytest
types-pyopenssl==23.0.0.4
# via -r lint-requirements.in
typing-extensions==4.5.0
# via mypy
1 change: 1 addition & 0 deletions newsfragments/559.ecdsa.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for ECDSA keys in certificates. The type of key used for certificates can be controlled by the `key_type` parameter on the multiple methods that generate certificates. By default trustme will generate ECDSA certificates as they can be generated significantly faster.
58 changes: 37 additions & 21 deletions src/trustme/__init__.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
from __future__ import annotations
import datetime
import ipaddress
import os
import ssl
from enum import Enum
from base64 import urlsafe_b64encode
from contextlib import contextmanager
from tempfile import NamedTemporaryFile
from typing import Generator, List, Optional, Union
from typing import Generator, List, Optional, Union, TYPE_CHECKING

import idna

from cryptography import x509
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.asymmetric import rsa
from cryptography.hazmat.primitives.asymmetric import rsa, ec
from cryptography.hazmat.primitives.serialization import (
PrivateFormat, NoEncryption
)
Expand All @@ -21,18 +23,13 @@

from ._version import __version__

TYPE_CHECKING = False
if TYPE_CHECKING: # pragma: no cover
import OpenSSL.SSL
CERTIFICATE_PUBLIC_KEY_TYPES = Union[rsa.RSAPublicKey, ec.EllipticCurvePublicKey]
CERTIFICATE_PRIVATE_KEY_TYPES = Union[rsa.RSAPrivateKey, ec.EllipticCurvePrivateKey]

__all__ = ["CA"]

# On my laptop, making a CA + server certificate using 2048 bit keys takes ~160
# ms, and using 4096 bit keys takes ~2 seconds. We want tests to run in 160 ms,
# not 2 seconds. And we can't go lower, since Debian (and probably others)
# by default reject any keys with <2048 bits (see #45).
_KEY_SIZE = 2048

# Default certificate expiry date:
# OpenSSL on Windows fails if you try to give it a date after
# ~3001-01-19:
Expand Down Expand Up @@ -68,7 +65,7 @@ def _smells_like_pyopenssl(ctx: object) -> bool:
def _cert_builder_common(
subject: x509.Name,
issuer: x509.Name,
public_key: rsa.RSAPublicKey,
public_key: CERTIFICATE_PUBLIC_KEY_TYPES,
not_after: Optional[datetime.datetime] = None,
) -> x509.CertificateBuilder:
not_after = not_after if not_after else DEFAULT_EXPIRY
Expand Down Expand Up @@ -206,6 +203,26 @@ def tempfile(self, dir: Optional[str] = None) -> Generator[str, None, None]:
os.unlink(f.name)


class KeyType(Enum):
"""Type of the key used to generate a certificate"""

RSA = 0
ECDSA = 1

def _generate_key(self) -> CERTIFICATE_PRIVATE_KEY_TYPES:
if self is KeyType.RSA:
# key_size needs to be a least 2048 to be accepted
# on Debian and pressumably other OSes

return rsa.generate_private_key(
public_exponent=65537, key_size=2048
)
elif self is KeyType.ECDSA:
return ec.generate_private_key(ec.SECP256R1())
else: # pragma: no cover
raise ValueError("Unknown key type")


class CA:
"""A certificate authority."""
_certificate: x509.Certificate
Expand All @@ -216,12 +233,10 @@ def __init__(
path_length: int = 9,
organization_name: Optional[str] = None,
organization_unit_name: Optional[str] = None,
key_type: KeyType = KeyType.ECDSA,
) -> None:
self.parent_cert = parent_cert
self._private_key = rsa.generate_private_key(
public_exponent=65537,
key_size=_KEY_SIZE,
)
self._private_key = key_type._generate_key()
self._path_length = path_length

name = _name(
Expand Down Expand Up @@ -278,7 +293,7 @@ def private_key_pem(self) -> Blob:
)
)

def create_child_ca(self) -> "CA":
def create_child_ca(self, key_type: KeyType = KeyType.ECDSA) -> "CA":
"""Creates a child certificate authority
Returns:
Expand All @@ -291,7 +306,7 @@ def create_child_ca(self) -> "CA":
raise ValueError("Can't create child CA: path length is 0")

path_length = self._path_length - 1
return CA(parent_cert=self, path_length=path_length)
return CA(parent_cert=self, path_length=path_length, key_type=key_type)

def issue_cert(
self,
Expand All @@ -300,6 +315,7 @@ def issue_cert(
organization_name: Optional[str] = None,
organization_unit_name: Optional[str] = None,
not_after: Optional[datetime.datetime] = None,
key_type: KeyType = KeyType.ECDSA,
) -> "LeafCert":
"""Issues a certificate. The certificate can be used for either servers
or clients.
Expand Down Expand Up @@ -341,6 +357,8 @@ def issue_cert(
not_after: Set the expiry date (notAfter) of the certificate. This
argument type is `datetime.datetime`.
key_type: Set the type of key that is used for the certificate. By default this is an ECDSA based key.
Returns:
LeafCert: the newly-generated certificate.
Expand All @@ -350,10 +368,7 @@ def issue_cert(
"Must specify at least one identity or common name"
)

key = rsa.generate_private_key(
public_exponent=65537,
key_size=_KEY_SIZE,
)
key = key_type._generate_key()

ski_ext = self._certificate.extensions.get_extension_for_class(
x509.SubjectKeyIdentifier)
Expand Down Expand Up @@ -464,7 +479,8 @@ def from_pem(cls, cert_bytes: bytes, private_key_bytes: bytes) -> "CA":
ca = cls()
ca.parent_cert = None
ca._certificate = x509.load_pem_x509_certificate(cert_bytes)
ca._private_key = load_pem_private_key(private_key_bytes, password=None)
ca._private_key = load_pem_private_key(private_key_bytes, password=None) # type: ignore[assignment]

return ca


Expand Down
11 changes: 9 additions & 2 deletions src/trustme/_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,29 @@ def main(argv: Optional[List[str]] = None) -> None:
action="store_true",
help="Doesn't print out helpful information for humans.",
)
parser.add_argument(
"-k",
"--key-type",
choices=list(t.name for t in trustme.KeyType),
default="ECDSA",
)

args = parser.parse_args(argv)
cert_dir = args.dir
identities = [str(identity) for identity in args.identities]
common_name = str(args.common_name[0]) if args.common_name else None
expires_on = None if args.expires_on is None else datetime.strptime(args.expires_on, DATE_FORMAT)
quiet = args.quiet
key_type = trustme.KeyType[args.key_type]

if not os.path.isdir(cert_dir):
raise ValueError(f"--dir={cert_dir} is not a directory")
if len(identities) < 1:
raise ValueError("Must include at least one identity")

# Generate the CA certificate
ca = trustme.CA()
cert = ca.issue_cert(*identities, common_name=common_name, not_after=expires_on)
ca = trustme.CA(key_type=key_type)
cert = ca.issue_cert(*identities, common_name=common_name, not_after=expires_on, key_type=key_type)

# Write the certificate and private key the server should use
server_key = os.path.join(cert_dir, "server.key")
Expand Down
46 changes: 29 additions & 17 deletions tests/test_trustme.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import service_identity.pyopenssl # type: ignore[import]

import trustme
from trustme import CA, LeafCert
from trustme import CA, LeafCert, KeyType


SslSocket = Union[ssl.SSLSocket, OpenSSL.SSL.Connection]
Expand Down Expand Up @@ -63,12 +63,17 @@ def assert_is_leaf(leaf_cert: x509.Certificate) -> None:
assert eku.critical is True


def test_basics() -> None:
ca = CA()
@pytest.mark.parametrize(
"key_type,expected_key_header", [(KeyType.RSA, b"RSA"), (KeyType.ECDSA, b"EC")]
)
def test_basics(key_type: KeyType, expected_key_header: bytes) -> None:
ca = CA(key_type=key_type)

today = datetime.datetime.today()

assert b"BEGIN RSA PRIVATE KEY" in ca.private_key_pem.bytes()
assert (
b"BEGIN " + expected_key_header + b" PRIVATE KEY" in ca.private_key_pem.bytes()
)
assert b"BEGIN CERTIFICATE" in ca.cert_pem.bytes()

private_key = load_pem_private_key(ca.private_key_pem.bytes(), password=None)
Expand All @@ -77,9 +82,11 @@ def test_basics() -> None:
assert ca_cert.not_valid_before <= today <= ca_cert.not_valid_after

public_key1 = private_key.public_key().public_bytes(
Encoding.PEM, PublicFormat.PKCS1)
Encoding.PEM, PublicFormat.SubjectPublicKeyInfo
)
public_key2 = ca_cert.public_key().public_bytes(
Encoding.PEM, PublicFormat.PKCS1)
Encoding.PEM, PublicFormat.SubjectPublicKeyInfo
)
assert public_key1 == public_key2

assert ca_cert.issuer == ca_cert.subject
Expand All @@ -88,7 +95,9 @@ def test_basics() -> None:
with pytest.raises(ValueError):
ca.issue_cert()

server = ca.issue_cert("test-1.example.org", "test-2.example.org")
server = ca.issue_cert(
"test-1.example.org", "test-2.example.org", key_type=key_type
)

assert b"PRIVATE KEY" in server.private_key_pem.bytes()
assert b"BEGIN CERTIFICATE" in server.cert_chain_pems[0].bytes()
Expand Down Expand Up @@ -256,6 +265,7 @@ def test_ca_from_pem(tmp_path: Path) -> None:
def check_connection_end_to_end(
wrap_client: Callable[[CA, socket.socket, str], SslSocket],
wrap_server: Callable[[LeafCert, socket.socket], SslSocket],
key_type: KeyType,
) -> None:
# Client side
def fake_ssl_client(ca: CA, raw_client_sock: socket.socket, hostname: str) -> None:
Expand Down Expand Up @@ -301,31 +311,32 @@ def doit(ca: CA, hostname: str, server_cert: LeafCert) -> None:
f1.result()
f2.result()

ca = CA()
intermediate_ca = ca.create_child_ca()
ca = CA(key_type=key_type)
intermediate_ca = ca.create_child_ca(key_type=key_type)
hostname = "my-test-host.example.org"

# Should work
doit(ca, hostname, ca.issue_cert(hostname))
doit(ca, hostname, ca.issue_cert(hostname, key_type=key_type))

# Should work
doit(ca, hostname, intermediate_ca.issue_cert(hostname))
doit(ca, hostname, intermediate_ca.issue_cert(hostname, key_type=key_type))

# To make sure that the above success actually required that the
# CA and cert logic is all working, make sure that the same code
# fails if the certs or CA aren't right:

# Bad hostname fails
with pytest.raises(Exception):
doit(ca, "asdf.example.org", ca.issue_cert(hostname))
doit(ca, "asdf.example.org", ca.issue_cert(hostname, key_type=key_type))

# Bad CA fails
bad_ca = CA()
with pytest.raises(Exception):
doit(bad_ca, hostname, ca.issue_cert(hostname))
doit(bad_ca, hostname, ca.issue_cert(hostname, key_type=key_type))


def test_stdlib_end_to_end() -> None:
@pytest.mark.parametrize("key_type", [KeyType.RSA, KeyType.ECDSA])
def test_stdlib_end_to_end(key_type: KeyType) -> None:
def wrap_client(ca: CA, raw_client_sock: socket.socket, hostname: str) -> ssl.SSLSocket:
ctx = ssl.create_default_context()
ca.configure_trust(ctx)
Expand All @@ -346,10 +357,11 @@ def wrap_server(server_cert: LeafCert, raw_server_sock: socket.socket) -> ssl.SS
print("server encrypted with:", wrapped_server_sock.cipher())
return wrapped_server_sock

check_connection_end_to_end(wrap_client, wrap_server)
check_connection_end_to_end(wrap_client, wrap_server, key_type)


def test_pyopenssl_end_to_end() -> None:
@pytest.mark.parametrize("key_type", [KeyType.RSA, KeyType.ECDSA])
def test_pyopenssl_end_to_end(key_type: KeyType) -> None:
def wrap_client(ca: CA, raw_client_sock: socket.socket, hostname: str) -> OpenSSL.SSL.Connection:
# Cribbed from example at
# https://service-identity.readthedocs.io/en/stable/api.html#service_identity.pyopenssl.verify_hostname
Expand All @@ -372,7 +384,7 @@ def wrap_server(server_cert: LeafCert, raw_server_sock: socket.socket) -> OpenSS
conn.do_handshake()
return conn

check_connection_end_to_end(wrap_client, wrap_server)
check_connection_end_to_end(wrap_client, wrap_server, key_type)


def test_identity_variants() -> None:
Expand Down

0 comments on commit 1571357

Please sign in to comment.