From 97dac0e46e37c3d221086413f7e9513324f6eddf Mon Sep 17 00:00:00 2001 From: Martin Maney Date: Mon, 31 Aug 2020 19:54:38 -0500 Subject: [PATCH 1/3] Crypto refactoring, with the addition of ECDSA keys, including cli options. Incompatible changes in Client.__init__ interface and some internals! --- .circleci/config.yml | 2 + Makefile | 16 +++ README.md | 16 +++ docs/crypto.md | 59 +++++++++ docs/sewer-as-a-library.md | 58 +++++---- setup.py | 2 +- sewer/cli.py | 63 ++++++--- sewer/client.py | 161 +++++------------------ sewer/crypto.py | 209 ++++++++++++++++++++++++++++++ sewer/providers/tests/__init__.py | 0 sewer/tests/test_Client.py | 60 ++++----- sewer/tests/test_catalog.py | 2 +- sewer/tests/test_crypto.py | 50 +++++++ 13 files changed, 489 insertions(+), 209 deletions(-) create mode 100644 docs/crypto.md create mode 100644 sewer/crypto.py create mode 100644 sewer/providers/tests/__init__.py create mode 100644 sewer/tests/test_crypto.py diff --git a/.circleci/config.yml b/.circleci/config.yml index 2e174a89..3b864ffd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -28,6 +28,8 @@ jobs: - run: name: run tests command: | + mkdir test + make testdata find . -type f -name *.pyc -delete | echo coverage erase coverage run --omit="*tests*,*.virtualenvs/*,*.venv/*,*__init__*,*/usr/local/lib/python2.7/dist-packages*" -m unittest discover && bash <(curl -s https://codecov.io/bash) diff --git a/Makefile b/Makefile index ad82799f..f4e35dde 100644 --- a/Makefile +++ b/Makefile @@ -60,6 +60,9 @@ uploadprod_tag: # you can run single testcase as; # 1. python -m unittest sewer.tests.test_Client.TestClient.test_something # 2. python -m unittest discover -k test_find_dns_zone_id + +.PHONY: test testdata rsatestkeys secptestkeys + test: @printf "\n removing pyc files::\n" && find . -type f -name *.pyc -delete | echo @printf "\n coverage erase::\n" && ${coverage} erase @@ -67,3 +70,16 @@ test: @printf "\n coverage report::\n" && ${coverage} report --show-missing --fail-under=85 @printf "\n run black::\n" && ${black} --line-length=100 --py36 . @printf "\n run pylint::\n" && ${pylint} --enable=E --disable=W,R,C --unsafe-load-any-extension=y sewer/ + + +testdata: rsatestkeys secptestkeys + +rsatestkeys: + openssl genrsa -out test/rsa2048.pem 2048 + openssl genrsa -out test/rsa3072.pem 3072 + openssl genrsa -out test/rsa4096.pem 4096 + +secptestkeys: + openssl ecparam -out test/secp256r1.pem -name secp256r1 -genkey + openssl ecparam -out test/secp384r1.pem -name secp384r1 -genkey + openssl ecparam -out test/secp521r1.pem -name secp521r1 -genkey diff --git a/README.md b/README.md index 600bc2e4..d6cf089f 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,21 @@ ## Sewer +**Cryptography rework branch.** + +The story so far: +- the mixed openssl/cryptography libraries using code has all been ripped +out of client.py +- AcmeKey and AcmeCsr, in crypto.py, replace and extend (ECDSA keys) old code +- Client NO LONGER GENERATES KEYS (account or certificate) +- Client argument changes: + + acct_key:AcmeKey in place of account_key: str + + cert_key:AcmeKey in plaxe of certificate_key: str + + bits: int removed + + is_new_account:bool [False] if key requires registration +- CLI options have changed a bit, mostly to add new features (ECDSA keys!) + + acct_key/cert_key added, preferred over account_key/certificate_key + + acct_key_type/cert_key_type allow selection of RSA or EC key generation + [![Codacy Badge](https://api.codacy.com/project/badge/Grade/ccf655afb3974e9698025cbb65949aa2)](https://www.codacy.com/app/komuW/sewer?utm_source=github.com&utm_medium=referral&utm_content=komuW/sewer&utm_campaign=Badge_Grade) [![CircleCI](https://circleci.com/gh/komuw/sewer.svg?style=svg)](https://circleci.com/gh/komuw/sewer) [![codecov](https://codecov.io/gh/komuW/sewer/branch/master/graph/badge.svg)](https://codecov.io/gh/komuW/sewer) diff --git a/docs/crypto.md b/docs/crypto.md new file mode 100644 index 00000000..1f91d863 --- /dev/null +++ b/docs/crypto.md @@ -0,0 +1,59 @@ +## A crypto module for ACME + +There wer several motivations behind the creation of `crypto.py`: + +- a desire to convert the OpenSSL (Python code) usage to the preferred + cryptography package. + +- breaking the Client global mess up into more cohesive components + +- . Oh, and @jfb's work on adding ECDSA certificate keys +was the spark that triggered the whole thing into [code] motion. + +### AcmeKey + +`AcmeKey` is a base class for key-type specific classes, but it also carries +shared code that both of the current type-specific classes, `AcmeRsaKey` and +`AcmeEcKey` share the use of. Much of this uses type-specific types or +values which are defined on the subclasses. + +#### Attributes + +- pk, the private key in the form of cryptography's key class + +- kid, the _key identifier_ that comes from registering pk with ACME. + +#### Methods + +- `set_kid(self, kid: str) -> None` Hook for ACME register_account or its + caller to use to attach the registered key's kid to the AcmeKey object. + +- `private_bytes(self) -> bytes` Returns all the key's info in PEM format + +- `sign_message(self, message: bytes) -> bytes` Calculate the signature for + the given message. (uses SHA256 only because that's what ACME specifies) + +- `jwk(self) -> Dict[str, str]` Returns the JSON Web Key as a dictionary + (with binary values base64 encoded). (value is cached) + +#### Other AcmeKey module-level bits + +Two factory methods for AcmeKey (these might become class methods of +AcmeKey?): + +- `new_ACME_key(key_type: str) -> AcmeKey` + +- `load_ACME_key(pem_data: bytes) -> AcmeKey` + +### AcmeCsr + +This is currently a minimal replacement for the OpenSSL-based create_csr +method. Which might be all an ACME client requires, so perhaps the current +design will be more or less how it comes out? There will be an additional +flag for setting the "must be stapled" certificate extension, but there's +really not much else to add, based on a review of the de-facto standard of +cert-bot's options. + +One thing to note: yes, the choice of DER format is intentional and +necessary, as the protocol requires that, base64-url encoded, _without_ the +starting and ending text lines that PEM wraps that same encoding in. diff --git a/docs/sewer-as-a-library.md b/docs/sewer-as-a-library.md index 23cb207c..22c31a36 100644 --- a/docs/sewer-as-a-library.md +++ b/docs/sewer-as-a-library.md @@ -4,6 +4,9 @@ ```python import sewer.client +from sewer.crypto import AcmeKey + +# [[ change this to load using the catalog! ]] import sewer.dns_providers.cloudflare dns_class = sewer.dns_providers.cloudflare.CloudFlareDns( @@ -11,47 +14,51 @@ dns_class = sewer.dns_providers.cloudflare.CloudFlareDns( CLOUDFLARE_API_KEY='nsa-grade-api-key' ) -# 1. to create a new certificate: +# 1. to create a new certificate (new account and certificate keys) + client = sewer.client.Client( domain_name='example.com', - dns_class=dns_class + dns_class=dns_class, + acct_key=AcmeKey.create("rsa2048"), + cert_key=AcmeKey.create("rsa2048") ) certificate = client.cert() -certificate_key = client.certificate_key -account_key = client.account_key -print("your certificate is:", certificate) -print("your certificate's key is:", certificate_key) -print("your letsencrypt.org account key is:", account_key) -# NB: your certificate_key and account_key should be SECRET. -# keep them very safe. +# NB: new crypto keeps keys & certs in python objects. They intentionally +# do not convert to printable form automatically (__str__, etc.) -# you can write these out to individual files, eg:: +print("your certificate is:", certificate.private_bytes()) +print("your certificate's key is:", cert_key.private_bytes()) +print("your letsencrypt.org account key is:", acct_key.private_bytes()) -with open('certificate.crt', 'w') as certificate_file: - certificate_file.write(certificate) -with open('certificate.key', 'w') as certificate_key_file: - certificate_key_file.write(certificate_key) -with open('account_key.key', 'w') as account_key_file: - account_key_file.write(account_key) +# NB: your certificate_key and account_key should be SECRET. +# You can write these out to individual files, eg:: + +with open("certificate.crt", "wb"') as f: + f.write(certificate.private_bytes()) +with open('certificate.key', 'wb') as f: + f.write(certkey.private_bytes()) +with open('account.key', 'w') f: + f.write(acctkey.private_bytes()) +# the acct_key also contains ACME's "kid" identifier if you're interested # 2. to renew a certificate: -import sewer.client -import sewer.dns_providers.cloudflare dns_class = sewer.dns_providers.cloudflare.CloudFlareDns( CLOUDFLARE_EMAIL='example@example.com', CLOUDFLARE_API_KEY='nsa-grade-api-key' ) -with open('account_key.key', 'r') as account_key_file: - account_key = account_key_file.read() +# load saved keys or create new as you prefer +acct_key = AcmeKey.from_file("account.key") +cert_key = AcmeKey.from_file("certificate_key") client = sewer.client.Client( domain_name='example.com', dns_class=dns_class, - account_key=account_key + acct_key=acct_key, + cert_key=cert_key, ) certificate = client.renew() certificate_key = client.certificate_key @@ -62,8 +69,6 @@ with open('certificate.key', 'w') as certificate_key_file: certificate_key_file.write(certificate_key) # 3. You can also request/renew wildcard certificates: -import sewer.client -import sewer.dns_providers.cloudflare dns_class = sewer.dns_providers.cloudflare.CloudFlareDns( CLOUDFLARE_EMAIL='example@example.com', @@ -71,11 +76,12 @@ dns_class = sewer.dns_providers.cloudflare.CloudFlareDns( ) client = sewer.client.Client( domain_name='*.example.com', - dns_class=dns_class + dns_class=dns_class, + # load or create keys ) certificate = client.cert() -certificate_key = client.certificate_key -account_key = client.account_key +cert_key = client.cert_key +acct_key = client.acct_key ``` ### Bring your own HTTP provider diff --git a/setup.py b/setup.py index 99bd973f..a33da867 100644 --- a/setup.py +++ b/setup.py @@ -47,7 +47,7 @@ "Programming Language :: Python :: 3.8", ], packages=find_packages(exclude=["docs", "*tests*"]), - install_requires=["requests", "pyopenssl", "cryptography"], + install_requires=["requests", "cryptography"], extras_require=dict( provider_deps_map, dev=["coverage", "twine", "wheel"], diff --git a/sewer/cli.py b/sewer/cli.py index b6cc1601..0755c7af 100644 --- a/sewer/cli.py +++ b/sewer/cli.py @@ -1,8 +1,10 @@ import argparse, os -from .catalog import ProviderCatalog from . import client, config, lib +from .catalog import ProviderCatalog +from .crypto import AcmeKey, key_type_choices + def setup_parser(catalog): """ @@ -35,34 +37,55 @@ def setup_parser(catalog): ### ACME account options parser.add_argument( + "--acct_key", "--account_key", - type=argparse.FileType("r"), + dest="acct_key_file", + type=argparse.FileType("rb"), help="Filepath to read to get registered ACME account. Default is to create one.", ) parser.add_argument("--email", help="Email to be used for registration of an ACME account.") + parser.add_argument( + "--acct_key_type", + choices=key_type_choices, + default="rsa2048", + help="Select type of account key to generate if not loading per --acct_key. Default rsa2048.", + ), + ### certificate options parser.add_argument( + "--cert_key", "--certificate_key", - type=argparse.FileType("r"), + dest="cert_key_file", + type=argparse.FileType("rb"), help="Filepath to read to get certificate key. Default is to create one.", ) + parser.add_argument( + "--cert_key_type", + choices=key_type_choices, + default="rsa2048", + help="Select type of certificate key to generate if not loading per --cert_key. Default rsa2048.", + ), + parser.add_argument( "--domain", required=True, help="The DNS identity which will be the certificate's Common Name. May be a wildcard.", ) + parser.add_argument( "--alt_domains", default=[], nargs="*", help="Optional alternate (SAN) identities to be added to the CN on this certificate.", ) + parser.add_argument( "--bundle_name", help="The basename for the output files. Default is the CN given by --domain.", ) + parser.add_argument( "--out_dir", default=os.getcwd(), @@ -301,8 +324,6 @@ def main(): alt_domains = args.alt_domains if args.action != "none": logger.warning("DEPRECATION WARNING: --action option is obsolete and will be dropped soon") - account_key = args.account_key - certificate_key = args.certificate_key bundle_name = args.bundle_name endpoint = args.endpoint email = args.email @@ -321,14 +342,23 @@ def main(): if not os.access(out_dir, os.W_OK): raise OSError("The dir '{0}' is not writable".format(out_dir)) - if account_key: - account_key = account_key.read() - if certificate_key: - certificate_key = certificate_key.read() + if args.acct_key_file: + acct_key = AcmeKey.from_bytes(args.acct_key_file.read()) + is_new_acct = False + else: + acct_key = AcmeKey.create(args.acct_key_type) + is_new_acct = True + + if args.cert_key_file: + cert_key = AcmeKey.from_bytes(args.cert_key.read()) + else: + cert_key = AcmeKey.create(args.cert_key_type) + if bundle_name: file_name = bundle_name else: file_name = "{0}".format(domain) + if endpoint == "staging": ACME_DIRECTORY_URL = config.ACME_DIRECTORY_URL_STAGING else: @@ -341,14 +371,13 @@ def main(): domain_name=domain, domain_alt_names=alt_domains, contact_email=email, - account_key=account_key, - certificate_key=certificate_key, + acct_key=acct_key, + is_new_acct=is_new_acct, + cert_key=cert_key, ACME_DIRECTORY_URL=ACME_DIRECTORY_URL, LOG_LEVEL=loglevel, ACME_REQUEST_TIMEOUT=args.acme_timeout, ) - certificate_key = acme_client.certificate_key - account_key = acme_client.account_key # prepare file path account_key_file_path = os.path.join(out_dir, "{0}.account.key".format(file_name)) @@ -356,8 +385,7 @@ def main(): crt_key_file_path = os.path.join(out_dir, "{0}.key".format(file_name)) # write out account_key in out_dir directory - with open(account_key_file_path, "w") as account_file: - account_file.write(account_key) + acct_key.to_file(account_key_file_path) logger.info("account key succesfully written to {0}.".format(account_key_file_path)) certificate = acme_client.cert() @@ -365,8 +393,7 @@ def main(): # write out certificate and certificate key in out_dir directory with open(crt_file_path, "w") as certificate_file: certificate_file.write(certificate) - with open(crt_key_file_path, "w") as certificate_key_file: - certificate_key_file.write(certificate_key) - logger.info("certificate succesfully written to {0}.".format(crt_file_path)) + + cert_key.to_file(crt_key_file_path) logger.info("certificate key succesfully written to {0}.".format(crt_key_file_path)) diff --git a/sewer/client.py b/sewer/client.py index 7b047606..8a6734fd 100644 --- a/sewer/client.py +++ b/sewer/client.py @@ -2,14 +2,11 @@ from hashlib import sha256 from typing import Dict, Sequence, Tuple, Union -# used to just import cryptography, which worked only because other modules did more :-( -import cryptography.hazmat.primitives.serialization -import cryptography.hazmat.backends -import OpenSSL.crypto import requests from .auth import ChalListType, ErrataListType, ProviderBase from .config import ACME_DIRECTORY_URL_PRODUCTION +from .crypto import AcmeCsr, AcmeKey from .lib import create_logger, log_response, safe_base64, sewer_meta @@ -20,13 +17,14 @@ class Client: def __init__( self, + *, domain_name: str, + acct_key: AcmeKey, + cert_key: AcmeKey, + is_new_acct=False, dns_class: ProviderBase = None, domain_alt_names: Sequence[str] = None, contact_email: str = None, - account_key: str = None, - certificate_key: str = None, - bits: int = 2048, digest: str = "sha256", provider: ProviderBase = None, ACME_REQUEST_TIMEOUT: int = 7, @@ -53,20 +51,6 @@ def __init__( type(contact_email) ) ) - elif not isinstance(account_key, (type(None), str)): - raise ValueError( - """account_key should be of type:: None or str. You entered {0}. - More specifically, account_key should be the result of reading an ssl account certificate""".format( - type(account_key) - ) - ) - elif not isinstance(certificate_key, (type(None), str)): - raise ValueError( - """certificate_key should be of type:: None or str. You entered {0}. - More specifically, certificate_key should be the result of reading an ssl certificate""".format( - type(certificate_key) - ) - ) elif LOG_LEVEL.upper() not in ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]: raise ValueError( """LOG_LEVEL should be one of; 'DEBUG', 'INFO', 'WARNING', 'ERROR' or 'CRITICAL'. not {0}""".format( @@ -92,7 +76,6 @@ def __init__( domain_alt_names = [] self.domain_alt_names = list(set(domain_alt_names)) self.contact_email = contact_email - self.bits = bits self.digest = digest self.ACME_REQUEST_TIMEOUT = ACME_REQUEST_TIMEOUT self.ACME_AUTH_STATUS_WAIT_PERIOD = ACME_AUTH_STATUS_WAIT_PERIOD @@ -101,6 +84,10 @@ def __init__( self.ACME_VERIFY = ACME_VERIFY self.LOG_LEVEL = LOG_LEVEL.upper() + self.acct_key = acct_key + self.cert_key = cert_key + self.is_new_acct = is_new_acct + self.logger = create_logger(__name__, LOG_LEVEL) try: @@ -114,19 +101,7 @@ def __init__( self.ACME_NEW_ORDER_URL = acme_endpoints["newOrder"] self.ACME_REVOKE_CERT_URL = acme_endpoints["revokeCert"] - # unique account identifier - # https://tools.ietf.org/html/draft-ietf-acme-acme#section-6.2 - self.kid = None - - self.certificate_key = certificate_key or self.create_certificate_key() - self.csr = self.create_csr() - - if not account_key: - self.account_key = self.create_account_key() - self.PRIOR_REGISTERED = False - else: - self.account_key = account_key - self.PRIOR_REGISTERED = True + self.acme_csr = AcmeCsr(cn=domain_name, san=domain_alt_names, key=self.cert_key) if dns_class is not None: self.logger.warning( @@ -229,71 +204,16 @@ def get_acme_endpoints(self): ) return get_acme_endpoints - def create_certificate_key(self): - self.logger.debug("create_certificate_key") - return self.create_key().decode() - - def create_account_key(self): - self.logger.debug("create_account_key") - return self.create_key().decode() - - def create_key(self, key_type=OpenSSL.crypto.TYPE_RSA): - key = OpenSSL.crypto.PKey() - key.generate_key(key_type, self.bits) - private_key = OpenSSL.crypto.dump_privatekey(OpenSSL.crypto.FILETYPE_PEM, key) - return private_key - - def create_csr(self): - """ - https://tools.ietf.org/html/draft-ietf-acme-acme#section-7.4 - The CSR is sent in the base64url-encoded version of the DER format. (NB: this - field uses base64url, and does not include headers, it is different from PEM.) - """ - self.logger.debug("create_csr") - X509Req = OpenSSL.crypto.X509Req() - X509Req.get_subject().CN = self.domain_name - - if self.domain_alt_names: - SAN = "DNS:{0}, ".format(self.domain_name).encode("utf8") + ", ".join( - "DNS:" + i for i in self.domain_alt_names - ).encode("utf8") - else: - SAN = "DNS:{0}".format(self.domain_name).encode("utf8") - - X509Req.add_extensions( - [ - OpenSSL.crypto.X509Extension( - "subjectAltName".encode("utf8"), critical=False, value=SAN - ), - # OpenSSL.crypto.X509Extension(b"tlsfeature", critical=False, value=b"status_request") - ] - ) - pk = OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, self.certificate_key.encode() - ) - X509Req.set_pubkey(pk) - X509Req.set_version(2) - X509Req.sign(pk, self.digest) - return OpenSSL.crypto.dump_certificate_request(OpenSSL.crypto.FILETYPE_ASN1, X509Req) - def acme_register(self): """ - https://tools.ietf.org/html/draft-ietf-acme-acme#section-7.3 - The server creates an account and stores the public key used to - verify the JWS (i.e., the "jwk" element of the JWS header) to - authenticate future requests from the account. - The server returns this account object in a 201 (Created) response, with the account URL - in a Location header field. - This account URL will be used in subsequest requests to ACME, as the "kid" value in the acme header. - If the server already has an account registered with the provided - account key, then it MUST return a response with a 200 (OK) status - code and provide the URL of that account in the Location header field. - If there is an existing account with the new key - provided, then the server SHOULD use status code 409 (Conflict) and - provide the URL of that account in the Location header field + RFC8555 has some changes in behavior. + + For now,leaving the behavior unchanged except replacing "self.PRIOR_REGISTERED" + with "not self.is_new_acct". But further work is needed - I don't think a + 409 result is part of the protocol any more, for one thing. """ self.logger.info("acme_register (newAccount)") - if self.PRIOR_REGISTERED: + if not self.is_new_acct: payload = {"onlyReturnExisting": True} elif self.contact_email: payload = { @@ -321,8 +241,7 @@ def acme_register(self): ) ) - kid = acme_register_response.headers["Location"] - setattr(self, "kid", kid) + self.acct_key.set_kid(acme_register_response.headers["Location"]) self.logger.info("acme_register_success") return acme_register_response @@ -430,7 +349,9 @@ def get_identifier_authorization(self, auth_url: str) -> Dict[str, str]: def get_keyauthorization(self, token): self.logger.debug("get_keyauthorization") - acme_header_jwk_json = json.dumps(self.get_jwk(), sort_keys=True, separators=(",", ":")) + acme_header_jwk_json = json.dumps( + self.acct_key.jwk(), sort_keys=True, separators=(",", ":") + ) acme_thumbprint = safe_base64(sha256(acme_header_jwk_json.encode("utf8")).digest()) acme_keyauthorization = "{0}.{1}".format(token, acme_thumbprint) @@ -525,7 +446,7 @@ def send_csr(self, finalize_url): GET request to the order resource to obtain its current state. """ self.logger.info("send_csr") - payload = {"csr": safe_base64(self.csr)} + payload = {"csr": safe_base64(self.acme_csr.public_bytes())} send_csr_response = self.make_signed_acme_request( url=finalize_url, payload=json.dumps(payload) ) @@ -567,11 +488,6 @@ def download_certificate(self, certificate_url: str) -> str: self.logger.info("download_certificate_success") return pem_certificate - def sign_message(self, message): - self.logger.debug("sign_message") - pk = OpenSSL.crypto.load_privatekey(OpenSSL.crypto.FILETYPE_PEM, self.account_key.encode()) - return OpenSSL.crypto.sign(pk, message.encode("utf8"), self.digest) - def get_nonce(self): """ https://tools.ietf.org/html/draft-ietf-acme-acme#section-6.4 @@ -583,28 +499,6 @@ def get_nonce(self): nonce = response.headers["Replay-Nonce"] return nonce - def get_jwk(self) -> Dict[str, str]: - """ - calculate the JSON Web Key (jwk) from self.account_key - """ - - def val_to_bin(val): - numbytes = (val.bit_length() + 7) // 8 - return val.to_bytes(numbytes, "big") - - private_key = cryptography.hazmat.primitives.serialization.load_pem_private_key( - self.account_key.encode(), - password=None, - backend=cryptography.hazmat.backends.default_backend(), - ) - pubnums = private_key.public_key().public_numbers() - jwk = { - "kty": "RSA", - "e": safe_base64(val_to_bin(pubnums.e)), - "n": safe_base64(val_to_bin(pubnums.n)), - } - return jwk - def get_acme_header(self, url, needs_jwk=False): """ https://tools.ietf.org/html/draft-ietf-acme-acme#section-6.2 @@ -619,9 +513,9 @@ def get_acme_header(self, url, needs_jwk=False): header = {"alg": "RS256", "nonce": self.get_nonce(), "url": url} if needs_jwk: - header["jwk"] = self.get_jwk() + header["jwk"] = self.acct_key.jwk() else: - header["kid"] = self.kid + header["kid"] = self.acct_key.kid return header def make_signed_acme_request(self, url, payload, needs_jwk=False): @@ -630,8 +524,10 @@ def make_signed_acme_request(self, url, payload, needs_jwk=False): payload64 = safe_base64(payload) protected = self.get_acme_header(url, needs_jwk) protected64 = safe_base64(json.dumps(protected)) - signature = self.sign_message(message="{0}.{1}".format(protected64, payload64)) # bytes - signature64 = safe_base64(signature) # str + message = ("%s.%s" % (protected64, payload64)).encode("utf-8") + # signature = self.sign_message(message="{0}.{1}".format(protected64, payload64)) # bytes + # signature64 = safe_base64(signature) # str + signature64 = safe_base64(self.acct_key.sign_message(message)) data = json.dumps( {"protected": protected64, "payload": payload64, "signature": signature64} ) @@ -684,6 +580,9 @@ def get_certificate(self): ### TO DO ### this is the obfuscated timeout loop. Clean this mess up! ### # # # ### it also keeps trying even when the auth is failed :-( + ### FIX? ### shouldn't this be checking the ORDER's status for completion? + # that is at least the most frugal of queries approach... + for chal in challenges: # Before sending a CSR, we need to make sure the server has completed the # validation for all the authorizations diff --git a/sewer/crypto.py b/sewer/crypto.py new file mode 100644 index 00000000..7016e48a --- /dev/null +++ b/sewer/crypto.py @@ -0,0 +1,209 @@ +from cryptography import x509 +from cryptography.x509.oid import NameOID +from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives.asymmetric import ec, padding, rsa +from cryptography.hazmat.primitives.serialization import ( + load_pem_private_key, + Encoding, + NoEncryption, + PrivateFormat, +) +from cryptography.hazmat.backends import default_backend, openssl + +from typing import Any, Dict, List, Optional, Tuple, Union + +from .lib import safe_base64 + + +### types for things defined here + +### FIX ME ### what can we use for XxxKeyType? + +# RsaKeyType = openssl.rsa._RSAPrivateKey +# EcKeyType = openssl.ec._EllipticCurvePrivateKey +# AcmeKeyType = Union[RsaKeyType, EcKeyType] + +# and why does this [seem] to work? +AcmeKeyType = Union[openssl.rsa._RSAPrivateKey, openssl.ec._EllipticCurvePrivateKey] + + +class AcmeKey: + """ + AcmeKey is the base class for the actual type-specific classes. It implements + some common methods, usually in terms of class values or methods that only + exist in the specific subclasses. + + NB: these are all wrappers and do NOT include key creation or loading. + See new_ACME_key and load_ACME_key factory functions below. + """ + + pk: AcmeKeyType + + private_format: Any = None + jwk_const: Dict[str, str] + jwk_attr: Dict[str, str] + + def __init__(self, pk: AcmeKeyType) -> None: + self.pk = pk + self.kid: Optional[str] = None + self._jwk: Optional[Dict[str, str]] = None + + @staticmethod + def create(key_type: str) -> AcmeKeyType: + """ + Factory method to create a new key of key_type, returned as an AcmeKey. + """ + + if key_type not in key_type_map: + raise ValueError("AcmeKey.create: unrecognized key_type: %s" % key_type) + Cls, arg = key_type_map[key_type] + + # ? # I suppose this could be inverted to keep the key type specifics in Cls + + if Cls is AcmeRsaKey: + return Cls(rsa.generate_private_key(65537, arg, default_backend())) + if Cls is AcmeEcKey: + return Cls(ec.generate_private_key(arg, default_backend())) + + raise ValueError("AcmeKey.create: got bad class from type_map") + + @staticmethod + def from_bytes(pem_data: bytes) -> AcmeKeyType: + """ + load a key from the PEM-format bytes, return an AcmeKey + + NB: since it's not stored in the PEM, the kid is empty (None) + """ + + pk = load_pem_private_key(pem_data, None, default_backend()) + + if isinstance(pk, rsa.RSAPrivateKey): + return AcmeRsaKey(pk) + if isinstance(pk, ec.EllipticCurvePrivateKey): + return AcmeEcKey(pk) + + raise ValueError("AcmeKey.from_bytes: unrecognized key type: %s" % pk) + + @staticmethod + def from_file(filename: str) -> AcmeKeyType: + "convenience method to load a PEM-format key; returns the AcmeKey" + + with open(filename, "rb") as f: + return AcmeKey.from_bytes(f.read()) + + def set_kid(self, kid: str) -> None: + """ + The kid is received when registering the key with the ACME service + """ + self.kid = kid + + def private_bytes(self) -> bytes: + """ + return pk's serialized (PEM) form + + USES ConcreteClass.private_format + """ + + pem_data = self.pk.private_bytes( + encoding=Encoding.PEM, format=self.private_format, encryption_algorithm=NoEncryption() + ) + return pem_data + + def to_file(self, filename: str) -> None: + "convenience method to write out the key in PEM form" + + with open(filename, "wb") as f: + f.write(self.private_bytes()) + + def sign_message(self, message: bytes) -> bytes: + raise NotImplementedError("subclass must implement sign_message") + + def jwk(self) -> Dict[str, str]: + """ + Returns the key's JWK as a dictionary + + CACHES result in _jwk + + USES ConcreteClass.{jwk_const, jwk_attr} + """ + + if not self._jwk: + pubnums = self.pk.public_key().public_numbers() + jwk = dict(self.jwk_const) # pylint: disable=E1101 + for name, attr_name in self.jwk_attr.items(): # pylint: disable=E1101 + val = getattr(pubnums, attr_name) + numbytes = (val.bit_length() + 7) // 8 + jwk[name] = safe_base64(val.to_bytes(numbytes, "big")) + self._jwk = jwk + return self._jwk + + +### Concrete AcmeXxxKey classes + +# NB: when adding a new key type, or extending one, remember to update key_type_map +# to match (below) + + +class AcmeRsaKey(AcmeKey): + + # maybe TraditionalOpenSSL to come out the same as testdata keys? + private_format = PrivateFormat.PKCS8 + jwk_const = {"kty": "RSA"} + jwk_attr = {"e": "e", "n": "n"} + + def sign_message(self, message: bytes) -> bytes: + signature = self.pk.sign(message, padding.PKCS1v15(), hashes.SHA256()) + return signature + + +class AcmeEcKey(AcmeKey): + + private_format = PrivateFormat.TraditionalOpenSSL + # jwk_const has to be set based on the key size + jwk_attr = {"x": "x", "y": "y"} + + def __init__(self, pk: AcmeKeyType) -> None: + self.jwk_const = {"kty": "EC", "crv": "P-%s" % pk.curve.key_size} + super().__init__(pk) + + def sign_message(self, message: bytes) -> bytes: + signature = self.pk.sign(message, ec.ECDSA(hashes.SHA256())) + return signature + + +# Key Type Registry +# +# key_type_map - maps key type name to the AcmeKey subclass that handles it and +# some magic values that are arguments needed for creating a new key. + +key_type_map: Dict[str, Tuple[Any, ...]] = { + "rsa2048": (AcmeRsaKey, 2048), + "rsa3072": (AcmeRsaKey, 3072), + "rsa4096": (AcmeRsaKey, 4096), + "secp256r1": (AcmeEcKey, ec.SECP256R1), + "secp384r1": (AcmeEcKey, ec.SECP384R1), + "secp521r1": (AcmeEcKey, ec.SECP521R1), +} + +# extract just the names for option choice lists, etc. +key_type_choices = list(key_type_map) + + +### We also need to generate Certificate Signing Requests + + +class AcmeCsr: + def __init__(self, *, cn: str, san: List[str], key: AcmeKey) -> None: + """ + temporary "just like Client.create_csr", more or less + """ + + csrb = x509.CertificateSigningRequestBuilder() + csrb = csrb.subject_name(x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)])) + all_names = list(set([cn] + san)) + SAN: List = [x509.DNSName(name) for name in all_names] + csrb = csrb.add_extension(x509.SubjectAlternativeName(SAN), critical=False) + self.csr = csrb.sign(key.pk, hashes.SHA256(), default_backend()) + + def public_bytes(self) -> bytes: + return self.csr.public_bytes(Encoding.DER) diff --git a/sewer/providers/tests/__init__.py b/sewer/providers/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/sewer/tests/test_Client.py b/sewer/tests/test_Client.py index 83f99cc8..cf160d0c 100644 --- a/sewer/tests/test_Client.py +++ b/sewer/tests/test_Client.py @@ -2,6 +2,16 @@ # not to pollute the global namespace. # see: https://python-packaging.readthedocs.io/en/latest/testing.html +# Have had to sprinkle the pylint pragma "disable=E1125" in the TestCase +# classes because pylint just won't shut up about missing required keywords +# that are passed as **kwargs. If I had time to piss away on it the pragma +# could be added to individual calls. Not! + +# Also, you can't write out the pragma in a comment like the above without +# having pylint notice it - and in this case give an error for it at module +# scope. So reminiscent of the bad side of ol' lint. + + from unittest import mock import cryptography from unittest import TestCase @@ -10,15 +20,19 @@ from sewer.config import ACME_DIRECTORY_URL_STAGING from . import test_utils +from ..crypto import AcmeKey LOG_LEVEL = "CRITICAL" +keys_for_ACME = {"acct_key": AcmeKey.create("rsa2048"), "cert_key": AcmeKey.create("rsa2048")} + usual_ACME = { "ACME_REQUEST_TIMEOUT": 1, "ACME_AUTH_STATUS_WAIT_PERIOD": 0, "ACME_DIRECTORY_URL": ACME_DIRECTORY_URL_STAGING, "LOG_LEVEL": LOG_LEVEL, } +usual_ACME.update(keys_for_ACME) class TestClient(TestCase): @@ -33,6 +47,8 @@ class TestClient(TestCase): should be in different testClasses """ + # pylint: disable=E1125 + def setUp(self): self.domain_name = "example.com" with mock.patch("requests.post", return_value=test_utils.MockResponse()), mock.patch( @@ -58,6 +74,7 @@ def mock_create_acme_client(): provider=test_utils.ExmpleHttpProvider(), ACME_DIRECTORY_URL=ACME_DIRECTORY_URL_STAGING, LOG_LEVEL=LOG_LEVEL, + **keys_for_ACME, ) self.assertRaises(ValueError, mock_create_acme_client) @@ -73,38 +90,6 @@ def test_user_agent_is_generated(self): for i in ["python-requests", "sewer", "https://github.com/komuw/sewer"]: self.assertIn(i, self.client.User_Agent) - def test_certificate_key_is_generated(self): - with mock.patch("requests.post", return_value=test_utils.MockResponse()), mock.patch( - "requests.get", return_value=test_utils.MockResponse() - ): - - certificate_key = self.client.certificate_key - - certificate_key_private_key = cryptography.hazmat.primitives.serialization.load_pem_private_key( - certificate_key.encode(), - password=None, - backend=cryptography.hazmat.backends.default_backend(), - ) - self.assertIsInstance( - certificate_key_private_key, cryptography.hazmat.backends.openssl.rsa._RSAPrivateKey - ) - - def test_account_key_is_generated(self): - with mock.patch("requests.post", return_value=test_utils.MockResponse()), mock.patch( - "requests.get", return_value=test_utils.MockResponse() - ): - - account_key = self.client.account_key - - account_key_private_key = cryptography.hazmat.primitives.serialization.load_pem_private_key( - account_key.encode(), - password=None, - backend=cryptography.hazmat.backends.default_backend(), - ) - self.assertIsInstance( - account_key_private_key, cryptography.hazmat.backends.openssl.rsa._RSAPrivateKey - ) - def test_acme_registration_is_done(self): with mock.patch("requests.post", return_value=test_utils.MockResponse()), mock.patch( "requests.get", return_value=test_utils.MockResponse() @@ -273,6 +258,8 @@ class TestClientForSAN(TestClient): Test Acme client for SAN certificates. """ + # pylint: disable=E1125 + def setUp(self): self.domain_alt_names = [ "blog.exampleSAN.com", @@ -300,6 +287,8 @@ class TestClientForWildcard(TestClient): Test Acme client for wildard certificates. """ + # pylint: disable=E1125 + def setUp(self): self.domain_alt_names = [ "blog.exampleSAN.com", @@ -328,6 +317,8 @@ class TestClientDnsApiCompatibility(TestCase): Test Acme client support with the deprecated dns_class parameter. """ + # pylint: disable=E1125 + def setUp(self): self.domain_name = "example.com" with mock.patch("requests.post") as mock_requests_post, mock.patch( @@ -352,6 +343,7 @@ def mock_create_acme_client(): dns_class=test_utils.ExmpleDnsProvider(), # NOTE: dns_class used here ACME_DIRECTORY_URL=ACME_DIRECTORY_URL_STAGING, LOG_LEVEL=LOG_LEVEL, + **keys_for_ACME, ) self.assertRaises(ValueError, mock_create_acme_client) @@ -398,9 +390,13 @@ def mock_instantiate_client(): class TestClientUnits(TestCase): + + # pylint: disable=E1125 + def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.mock_args = {"domain_name": "example.com", "LOG_LEVEL": LOG_LEVEL} + self.mock_args.update(keys_for_ACME) self.mock_challenges = [{"ident_value": "example.com", "key_auth": "abcdefgh12345678"}] def mock_sewer(self, provider): diff --git a/sewer/tests/test_catalog.py b/sewer/tests/test_catalog.py index 54143206..0dff2eda 100644 --- a/sewer/tests/test_catalog.py +++ b/sewer/tests/test_catalog.py @@ -3,7 +3,7 @@ from .. import auth, catalog -class TestLib(unittest.TestCase): +class TestCatalog(unittest.TestCase): def test01_ProviderCatalog_create(self): cat = catalog.ProviderCatalog() self.assertIsInstance(cat, catalog.ProviderCatalog) diff --git a/sewer/tests/test_crypto.py b/sewer/tests/test_crypto.py new file mode 100644 index 00000000..6f4a1c34 --- /dev/null +++ b/sewer/tests/test_crypto.py @@ -0,0 +1,50 @@ +import unittest + +from typing import Sequence, Tuple + +from ..crypto import AcmeKey, AcmeCsr + + +KeyType = Tuple[str, int] + + +class Test_AcmeKey(unittest.TestCase): + + rsa_key_types = (("rsa2048", 2048), ("rsa3072", 3072), ("rsa4096", 4096)) + secp_key_types = (("secp256r1", 256), ("secp384r1", 384), ("secp521r1", 521)) + + def fromfile_privbytes_frombytes_sign_key(self, key_type: KeyType) -> None: + type_name, key_size = key_type + filename = "test/%s.pem" % type_name + # from_file + loaded_key = AcmeKey.from_file(filename) + self.assertTrue(loaded_key.pk.key_size == key_size) + # private_bytes + loaded_pb = loaded_key.private_bytes() + # from_bytes + reloaded_key = AcmeKey.from_bytes(loaded_pb) + self.assertTrue(loaded_key.pk.private_numbers() == reloaded_key.pk.private_numbers()) + # sign_message + self.assertTrue(len(loaded_key.sign_message(b"Taketh uth to thine leaderth"))) + + def test11_rsa_privbytes_sign(self): + for kt in self.rsa_key_types: + self.fromfile_privbytes_frombytes_sign_key(kt) + + def test11_secp_privbytes_sign(self): + for kt in self.secp_key_types: + self.fromfile_privbytes_frombytes_sign_key(kt) + + def generate_test(self, key_types: Sequence[KeyType]) -> None: + for type_name, key_size in key_types: + key = AcmeKey.create(type_name) + self.assertTrue(key.pk.key_size == key_size) + + def test21_generate_rsa_keys(self): + self.generate_test(self.rsa_key_types) + + def test22_generate_ec_keys(self): + self.generate_test(self.secp_key_types) + + +### TODO ### CSR tests From d710e0af4a6fbefbdb3a03e3e96b921315c61530 Mon Sep 17 00:00:00 2001 From: Martin Maney Date: Sun, 6 Sep 2020 22:19:25 -0500 Subject: [PATCH 2/3] Crypto fixes, clarifications, lots of documentation updates, and no, LE doesn't P-521 :-( --- Makefile | 15 ++--- README.md | 3 +- docs/ACME.md | 6 +- docs/CHANGELOG.md | 32 +++++++++++ docs/DNS-Propagation.md | 23 ++++---- docs/UnifiedProvider.md | 24 ++++---- docs/catalog.md | 14 ++--- docs/crypto.md | 76 ++++++++++++++++++------ docs/dns-01.md | 115 ++++++++++++++----------------------- docs/http-01.md | 9 ++- docs/sewer-as-a-library.md | 63 +++++++++++++++++--- docs/sewer-cli.md | 79 ++++++++++++++++++------- docs/unpropagated.md | 10 ++-- docs/wildcards.md | 4 +- sewer/auth.py | 2 +- sewer/catalog.py | 2 +- sewer/cli.py | 37 ++++++++---- sewer/client.py | 35 ++++++----- sewer/crypto.py | 110 ++++++++++++++++++++++------------- sewer/tests/test_Client.py | 8 +-- sewer/tests/test_crypto.py | 17 +++++- 21 files changed, 439 insertions(+), 245 deletions(-) diff --git a/Makefile b/Makefile index f4e35dde..fd5fcc79 100644 --- a/Makefile +++ b/Makefile @@ -68,18 +68,19 @@ test: @printf "\n coverage erase::\n" && ${coverage} erase @printf "\n coverage run::\n" && ${coverage} run --omit="*tests*,*.virtualenvs/*,*.venv/*,*__init__*" -m unittest discover @printf "\n coverage report::\n" && ${coverage} report --show-missing --fail-under=85 - @printf "\n run black::\n" && ${black} --line-length=100 --py36 . + @printf "\n run black::\n" && ${black} --line-length=100 --target-version py35 . @printf "\n run pylint::\n" && ${pylint} --enable=E --disable=W,R,C --unsafe-load-any-extension=y sewer/ testdata: rsatestkeys secptestkeys rsatestkeys: - openssl genrsa -out test/rsa2048.pem 2048 - openssl genrsa -out test/rsa3072.pem 3072 - openssl genrsa -out test/rsa4096.pem 4096 + openssl genpkey -out test/rsa2048.pem -algorithm RSA -pkeyopt rsa_keygen_bits:2048 + openssl genpkey -out test/rsa3072.pem -algorithm RSA -pkeyopt rsa_keygen_bits:3072 + openssl genpkey -out test/rsa4096.pem -algorithm RSA -pkeyopt rsa_keygen_bits:4096 secptestkeys: - openssl ecparam -out test/secp256r1.pem -name secp256r1 -genkey - openssl ecparam -out test/secp384r1.pem -name secp384r1 -genkey - openssl ecparam -out test/secp521r1.pem -name secp521r1 -genkey + openssl genpkey -out test/secp256r1.pem -algorithm EC -pkeyopt ec_paramgen_curve:P-256 + openssl genpkey -out test/secp384r1.pem -algorithm EC -pkeyopt ec_paramgen_curve:P-384 +# not actually useful with LE at this time +# openssl genpkey -out test/secp521r1.pem -algorithm EC -pkeyopt ec_paramgen_curve:P-521 diff --git a/README.md b/README.md index d6cf089f..ed5a8875 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ out of client.py - CLI options have changed a bit, mostly to add new features (ECDSA keys!) + acct_key/cert_key added, preferred over account_key/certificate_key + acct_key_type/cert_key_type allow selection of RSA or EC key generation + + is_new_account added to support registering your own account key [![Codacy Badge](https://api.codacy.com/project/badge/Grade/ccf655afb3974e9698025cbb65949aa2)](https://www.codacy.com/app/komuW/sewer?utm_source=github.com&utm_medium=referral&utm_content=komuW/sewer&utm_campaign=Badge_Grade) [![CircleCI](https://circleci.com/gh/komuw/sewer.svg?style=svg)](https://circleci.com/gh/komuw/sewer) @@ -24,7 +25,7 @@ out of client.py Sewer is a Let's Encrypt(ACME) client. It's name is derived from Kenyan hip hop artiste, Kitu Sewer. -- This is the trunk, moving towards a 0.8.4 release. No notes yet. +- This is crypto work, intended for the 0.8.4 release. No notes yet. - The stable release is [0.8.3](https://komuw.github.io/sewer/notes/0.8.3-notes). - More history in the [CHANGELOG](https://komuw.github.io/sewer/CHANGELOG). diff --git a/docs/ACME.md b/docs/ACME.md index fc122304..2360d3aa 100644 --- a/docs/ACME.md +++ b/docs/ACME.md @@ -1,4 +1,4 @@ -## ACME, RFCs, and confusion, oh my! +# ACME, RFCs, and confusion, oh my! ACME grew out of early, ad-hoc procedures designed to let CAs issue large numbers of certificates with low overhead. As described in RFC855, these @@ -40,9 +40,9 @@ make out, often-shifting schedule for various partial transitions, but I'm not going to try to make sense of them. As of the beginning of 2020, the only immediate effect on sewer was that one could no longer run it against the *staging* server. The next big change is when that same restriction is -rolled out on LE's *production* server late in the year. Since sewer +rolled out on LE's *production* server later in the year. Since sewer v0.8.2, which implemented the final RFC8555 protocol at least well enough to work with LE's server implementation, our tl;dr is just this: > If you get a failure running an older version of sewer, get v0.8.2 or -> later. This is a known problem: v0.8.2 is the fix. + later. This is a known problem: v0.8.2 or later is the fix. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b7dabc5d..d8db5a81 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -1,6 +1,38 @@ ## `sewer` changelog: most recent version is listed first. +## **version:** 0.8.4 [unreleased] + +- add support for ECDSA keys + +CLI changes: + +- `--account_key` & `--certificate_key` supported only for compatibility: + use `--acct_key` and `--cert_key` to avoid future deprecation. + +- add `--acct_key_type` & `--cert_key_type` to allow choice of RSA or EC + keys and sizes. + +- changed default for generated keys to 3072 bit RSA (had been 2048 bit) + +- add `--is_new_key` to allow for first-time registration of your own + account key (using `--acct_key`) generated outside of sewer. + +Internal changes: + +- Client methods cert() and renew() are deprecated; just call + get_certificate() directly instead. + +- crytographic refactoring + + - AcmeKey & AcmeCsr in crypto.py; uses only cryptography library + + - dropped `account_key` and `certificate_key` optional arguments + + - added `acct_key` and `cert_key` REQUIRED arguments taking AcmeKey objects + + - dropped `bits` argument because Client no longer generates keys! + ## **version:** 0.8.3 Features and Improvements: diff --git a/docs/DNS-Propagation.md b/docs/DNS-Propagation.md index 5fbcc05c..8f4a3e78 100644 --- a/docs/DNS-Propagation.md +++ b/docs/DNS-Propagation.md @@ -1,4 +1,4 @@ -## Waiting for Mr. DNS or Someone Like Him +# Waiting for Mr. DNS or Someone Like Him Q: How long does it take after you've setup the challenge response TXT records until they're actually accessible to the ACME server? @@ -8,14 +8,17 @@ A: Good Question! According to [Let's Encrypt](https://letsencrypt.org/docs/challenge-types/#dns-01-challenge) it can take up to an hour. Depends on the DNS service. Some provide a way to check that your changes are fully propagated to all their servers. With -many, however, you just have to wait. +many, however, you just have to wait. But be sure you wait long enough, +because Let's Encrypt DOES NOT implement automatic or triggered retry of a +failed authorization - you have to restart the [same] order or else start +all over again. Sewer provides a flexible _delay until actually published_ mechanism through three optional driver parameters, `prop_delay`, `prop_timeout`, `prop_sleep_times`, and the [`unpropagated` method](unpropagated). Let's see how they're used in various circumstances. -### No API support, no reliable way to check: just delay +## No API support, no reliable way to check: just delay If you can't check that the TXT records are fully published, then all you can do is delay for a while. Perhaps the DNS service will suggest a safe @@ -27,9 +30,9 @@ and sewer's engine will add that many seconds of delay after the challenge setup returns before it signals the ACME server to validate those challenges. -**CLI option --p_opt prop_delay=... is available for all providers as of 0.8.3** +**CLI option --p_opt prop_delay=... is available for all drivers since 0.8.3** -### API support or can check: use a timeout +## API support or can check: use a timeout If the DNS service gives you a way to check that the propagation is complete, or if there are not too many authoritative servers (viz., not an @@ -43,9 +46,9 @@ before it starts checking. And there's a delay between checks that has a hopefully sensible default, but which you can adjust if necessary through the `prop_sleep_times` parameter. -**legacy providers do not implement `unpropagated` as of 0.8.3** +**no drivers implement `unpropagated` as of 0.8.3** -### You probably don't need to change `prop_sleep_times` +## You probably don't need to change `prop_sleep_times` Unless you do, but if it's not obvious, just leave it. @@ -77,7 +80,7 @@ ready. So the timeout isn't a hard maximum time, but it's bounded to be no more than one sleep interval (plus actual time to run `unpropagated`, of course) over `prop_timeout`. -### Other Notes and Advanced Use +## Other Notes and Advanced Use These values are setup through the Provider on the reasonable assumption that they will vary most directly with the choice of service provider, so @@ -95,10 +98,10 @@ on the Provider instance. This is solidly in the categories of don't do it unless you're sure you need to, and be prepared to own both the pieces if you break it! -### Could this be used with non-DNS Providers? +## Could this be used with non-DNS drivers? Yes! I have no experience with http-01 in any setting where such a delay might be needed, but the mechanism is implemented in sewer's engine, and all that needs be done is to setup the parameters (and implement unpropagated in the driver if using more than just `prop_delay`) as described above and -there you are! +there you go! diff --git a/docs/UnifiedProvider.md b/docs/UnifiedProvider.md index 7c152286..e686ac67 100644 --- a/docs/UnifiedProvider.md +++ b/docs/UnifiedProvider.md @@ -1,8 +1,8 @@ -## DNS and HTTP challenges unified +# DNS and HTTP challenges unified _There's still a draft when the wind is blowing, but it's getting less._ -### Dedication +## Dedication It is indisputable that this is, in the first instance, Alec Troemel's fault, since he added support for http-01 challenges. @@ -11,7 +11,7 @@ made in the process of unifying the two types of challenges, while influenced by Alec's code and our discussions, are entirely my fault. Alec cannot be blamed for my choices! -### A few words about words +## A few words about words Because the word "provider" is so overloaded, I'm going to refer to the service-specific implementations as "drivers", except when I forget, or @@ -19,7 +19,7 @@ missed changing an old use. "Provider" is still used in the class names. And then there are the "service providers", viz., DNS services or web hosts, etc. -### Overview (tl;dr) +## Overview (tl;dr) `ProviderBase` described here defines the interface the ACME engine uses with new-model drivers (all http-01 drivers, as there are no old ones). New @@ -30,7 +30,7 @@ drivers normally should inherit from the `DNSProviderBase` or individual drivers need to be created (or modified) to support it at this time. _unbound_ssh is a quirky but working example that supports aliasing.` -### ProviderBase interface for ACME engine +## ProviderBase interface for ACME engine The interface between the ACME protocol code and any driver implementation consists of three methods, `setup`, `unpropagated` and `clear`. The first @@ -75,11 +75,11 @@ This is the pattern which all three methods use: accept a list of challenges subset which have problems or are not ready. So in all cases an empty list returned means that all went well. -### `ProviderBase` +## `ProviderBase` Abstract base class for driver implementations ultimately inherit from. -#### ProviderBase __init__ +### ProviderBase __init__ __init__(self, *, @@ -127,7 +127,7 @@ It is allowed to add, change, or even remove items from kwargs if necessary --- (see where for args documentation? DNS-Alias and DNS-Propagation & ???) -#### `setup(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` +### `setup(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` The `setup` method is called to publish one or more challenges. Each item in the list describes one challenge. @@ -171,7 +171,7 @@ defined values: | "skipped" | setup | may skip challenges after one has a hard failure | | "unready" | unpropagated | soft fail: record not deployed to authoritative server(s). If a non-recoverable error is detected, then use _failed_. | -#### `unpropagated(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` +### `unpropagated(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` This method is expected to be needed mostly for DNS challenges, but it should be used whenever a service provider has a relatively slow or @@ -180,7 +180,7 @@ challenge data being visible to the world. When there's no expectation of such lag, or no way to reliably check that the challenge has propagated, this may as well just return an empty list, and we'll all hope for the best. -#### `clear(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` +### `clear(self, challenges: Sequence[Dict[str, str]]) -> Sequence[Tuple[str, str, Dict[str, str]]]` `clear`, unlike `setup`, SHOULD NOT stop processing challenges after hitting an error. It's possible that any reported errors will be treated as @@ -189,7 +189,7 @@ challenges). _? should have a status word for "this one's hard failed, forget about it"?_ -### `DNSProviderBase` +## `DNSProviderBase` The driver *interface* is the same for everything except legacy DNS drivers, but there are some differences which it makes no sense to push into @@ -209,7 +209,7 @@ handling both the aliasing and non-aliasing case. `cname_domain` forms the DNS name for the CNAME that should exist in the aliasing case and returns it for the use of a hypothetical sanity check, or None when not aliasing. -### `HTTPProviderBase` +## `HTTPProviderBase` This intermediate base class stands ready to handle any HTTP-specific options or helper methods. No additions are expected until sewer has had diff --git a/docs/catalog.md b/docs/catalog.md index 5dc65a33..b40290f0 100644 --- a/docs/catalog.md +++ b/docs/catalog.md @@ -1,4 +1,4 @@ -## Driver Catalog +# The Catalog of Drivers The driver catalog, `sewer/catalog.json`, replaces scattered facilities that were used to stitch things together. The import farms in `sewer.__init__` @@ -15,7 +15,7 @@ facilities - it's all lists & dicts (see eg. setup.py which loads the catalog this way to avoid potential issues with trying to call into the package's code before it's installed). -### Catalog structure +## Catalog structure The catalog resides in a JSON file that loads as an array of dictionaries, one element for each registered driver. The per-driver record contains the @@ -40,7 +40,7 @@ following items (some optional): - **memo** Additional text/comments about the driver, the descriptor, etc. - **deps** list of additional projects this driver requires (for setup) -### args - parameter desciptors +## args - parameter desciptors This is a bit of a mess due to legacy drivers that ignored the established conventions. To be fair to them, those conventions weren't clearly @@ -102,7 +102,7 @@ so both the parameter and envvar name must be given explicitly. There is also an optional parameter that has never had an associated envvar that the implementation used. -### driver parameter and environment variable names +## driver parameter and environment variable names The convention is that the envvar name (if any) SHOULD be formed from the driver name and the individual args' names (see the first envvar rule @@ -117,12 +117,12 @@ Obviously the drivers and envvar names are not so consistent among the legacy DNS drivers. Therefore the descriptor has both `param` and `envvar` values, along with a set of rules for resolving the names to be used. -#### parameter name rules +### parameter name rules 1. `descriptor.args[n].name` is the "modern" name for the nth parameter 2. if `param` is given, it overrides the "modern" name -#### environment name rules +### environment name rules 1. f"{descriptor.name}_{descriptor.args[n].name}".upper() is the default 2. if `envvar` is given, it overrides the default @@ -136,7 +136,7 @@ Two guidelines for the use of envvars: 2. If `envvar` is set to the empty string, then catalog using code will not look for a matching envvar at all. -### catalog representation in Python +## catalog representation in Python For now, see the brief implementation in sewer/catalog.py for the way the JSON structure is mapped into a ProviderDescriptor instance. diff --git a/docs/crypto.md b/docs/crypto.md index 1f91d863..011bcf9b 100644 --- a/docs/crypto.md +++ b/docs/crypto.md @@ -1,21 +1,51 @@ ## A crypto module for ACME -There wer several motivations behind the creation of `crypto.py`: +There were several motivations behind the creation of `crypto.py`: - a desire to convert the OpenSSL (Python code) usage to the preferred cryptography package. - breaking the Client global mess up into more cohesive components -- . Oh, and @jfb's work on adding ECDSA certificate keys -was the spark that triggered the whole thing into [code] motion. +- Oh, and @jfb's PR adding ECDSA certificate keys was the spark that +triggered the whole thing into [code] motion. + +### Preliminaries + +I use the term `private key` quite a bit, as it is the term widely used for +some representation of both the public and private parts of an asymmetric +key. ### AcmeKey `AcmeKey` is a base class for key-type specific classes, but it also carries -shared code that both of the current type-specific classes, `AcmeRsaKey` and -`AcmeEcKey` share the use of. Much of this uses type-specific types or -values which are defined on the subclasses. +shared code that implements most of the methods on the concrete classes. +These methods use type-specific types or values which are defined on the +concrete classes. + +> In fact, it turns out that aside from the CSR, everything sewer needs to +do with crypto can be done after importing just AcmeKey (factory methods and +the methods on the concrete classes the factories hand out). + +So now Client and the cli code can stop schlepping around a string that +holds the key in PEM format. That's part of the reason this work +intentionally broke the names in Client (eg., acct_key in place of account +key) when it switched to an AcmeKey object. + +#### Factory methods + +Two essential factories: + +- `create(key_type: str) -> AcmeKeyType` generates a new private key + of the named kind. + +- `from_bytes(pem_data: bytes) -> AcmeKeyType` returns an AcmeKey object + containing the key serialized in the PEM bytes, assuming it is one of the + types of keys that are known (viz., implemented in a subclass of AcmeKey). + +And an inessential convenience: + +- `from_file(filename: str) -> AcmeKeyType` #### Attributes @@ -23,27 +53,39 @@ values which are defined on the subclasses. - kid, the _key identifier_ that comes from registering pk with ACME. -#### Methods +But sewer's code never needs to touch the pk directly, and kid is needed +only for constructing the signed ACME requests. -- `set_kid(self, kid: str) -> None` Hook for ACME register_account or its - caller to use to attach the registered key's kid to the AcmeKey object. +#### Methods - `private_bytes(self) -> bytes` Returns all the key's info in PEM format +- `to_file(self, filename:str) -> None` Writes private key to file as PEM + +private_bytes is the only method that all ACME clients must use, and that +will often be done indirectly through to_file. + +> Note: AcmeKey uses PKCS8 format for private_bytes rather than the +traditional OpenSSL format (PKCS1 for RSA). This shouldn't be an issue for +anything less artificial than some tests in test_crypto.py . + - `sign_message(self, message: bytes) -> bytes` Calculate the signature for the given message. (uses SHA256 only because that's what ACME specifies) - `jwk(self) -> Dict[str, str]` Returns the JSON Web Key as a dictionary (with binary values base64 encoded). (value is cached) -#### Other AcmeKey module-level bits - -Two factory methods for AcmeKey (these might become class methods of -AcmeKey?): +Sign_message and jwk are methods which an ACME client might find reason to +call, especially if they were implementing a part of the ACME protocol that +sewer currently omits, or a third party extension. -- `new_ACME_key(key_type: str) -> AcmeKey` +- `set_kid(self, kid: str) -> None` Hook for ACME register_account or its + caller to use to attach the registered key's kid to the AcmeKey object. -- `load_ACME_key(pem_data: bytes) -> AcmeKey` +set_kid is pure implementation detail, a stash for the account's registered +URL on a specific ACME server. The only use that comes to mind offhand +would be if n ACME client wanted to save and restore this across runs, as it +is not part of the private key PEM file. ### AcmeCsr @@ -55,5 +97,5 @@ really not much else to add, based on a review of the de-facto standard of cert-bot's options. One thing to note: yes, the choice of DER format is intentional and -necessary, as the protocol requires that, base64-url encoded, _without_ the -starting and ending text lines that PEM wraps that same encoding in. +necessary, as the ACME protocol requires that format, base64-url encoded, +_without_ the starting and ending text lines that PEM adds. diff --git a/docs/dns-01.md b/docs/dns-01.md index 0ce49a81..7cc47c6c 100644 --- a/docs/dns-01.md +++ b/docs/dns-01.md @@ -1,29 +1,45 @@ -## sewer's DNS service support - -ACME's dns-01 authorization was sewer's original target. -There are a number of DNS services supported in-tree, -and implementations for other services are difficult to write only if the -service's API is difficult. - -### DNS services supported - -- [acme-dns](https://github.com/joohoi/acme-dns) -- [Aliyun](https://help.aliyun.com/document_detail/29739.html) -- [Aurora](https://www.pcextreme.com/aurora/dns) -- [AWS Route 53](https://aws.amazon.com/route53/) -- [Cloudflare](https://www.cloudflare.com/dns) -- [ClouDNS](https://www.cloudns.net) -- [DNSPod](https://www.dnspod.cn/) -- [DuckDNS](https://www.duckdns.org/) -- [Gandi](https://doc.livedns.gandi.net/) -- [Hurricane Electric DNS](https://dns.he.net/) -- [PowerDNS](https://doc.powerdns.com/authoritative/http-api/index.html) -- [Rackspace](https://www.rackspace.com/cloud/dns) -- [unbound_ssh] - -_ToDo: combine this list with the features table below, I think._ - -### Add a driver for your DNS service +# DNS service drivers + +ACME's dns-01 authorization was sewer's original target. There are a number +of DNS services supported in-tree, and implementations for other services +are difficult to write only if the service's API is difficult. + +## DNS services supported + +Currently, these are all _legacy_ drivers, built on the original DNS-only +interface. That's okay, there's no plan to drop them (just a hope that +interested users will step up to get them updated), but that does mean that +support for some features varies. + +| service | driver name | wc+ | alias | prop | notes | +| --- | --- | :-: | :-: | :-: | --- | +| [acme-dns](https://github.com/joohoi/acme-dns) | acmedns | ? | no | no | | +| [Aliyun](https://help.aliyun.com/document_detail/29739.html) | aliyun | ? | no | no | | +| [Aurora](https://www.pcextreme.com/aurora/dns) | aurora | ? | no | no | | +| [Cloudflare](https://www.cloudflare.com/dns) | cloudflare | ? | no | no | patch in #123, needs confirmation? | +| [ClouDNS](https://www.cloudns.net) | cloudns | ? | no | no | test coverage 75% | +| [DNSPod](https://www.dnspod.cn/) | dnspod | ? | no | no | | +| [DuckDNS](https://www.duckdns.org/) | duckdns | ? | no | no | | +| [Gandi](https://doc.livedns.gandi.net/) | gandi | OK | no | no | wildcard & other fixes in 0.8.3 | +| [Hurricane Electric](https://dns.he.net/) | hurricane | ? | no | no | test coverage 70% | +| [PowerDNS](https://doc.powerdns.com/authoritative/http-api/index.html) | powerdns | NO | no | no | apparently not in 0.8.2; bug #195 | +| [Rackspace](https://www.rackspace.com/cloud/dns) | rackspace | ? | no | no | test coverage 69% | +| [Route 53 (AWS)](https://aws.amazon.com/route53/) | route53 (1) | OK | no | no | wc+ in 0.8.2; not in CLI | +| Unbound | unbound_ssh | OK | yes | no | Working demonstrator model for local unbound server | + +- _wc+_ (wilcard plus) is specifically about a single certificate that has + at least two registered names: `domain.tld` and `*.domain.tld`. This + specific combination has issues with some service providers/s.p.'s + API/drivers. So far it's been possible to make it work by changing the + drivers, but it has to be done one by one. + +- _alias_ publishing challenges in a different [sub]domain than the + identities being authorized. See [Aliasing](aliasing). + +- _prop_ support for the [unpropagated](unpropagated) interface. Can be + added to any driver but may only be worthwhile with service API support? + +## Add a driver for your DNS service Most (?) of the DNS drivers came about because someone wanted to use sewer with their DNS service provider, but there wasn't a driver to use with the @@ -38,7 +54,7 @@ _after they have all migrated to the new interface_. New DNS drivers should use DNSProviderBase from the start, of course, and will be placed in sewer/providers/ if added to the project. - # sketch of dns-01 provider, including alias support [which is NOT in 0.8.2] + # sketch of simple dns-01 provider, including alias support from .. import auth from .. import lib @@ -80,48 +96,3 @@ which the new-model interface provides - that can be a big win for large-SAN certificates if you have to grovel the service's API (or web pages) to guide the construction of your commands to them. It does show the use of target_domain to support [DNS aliasing](Aliasing). - -### Legacy DNS drivers vs. $FEATURES - -Three features that have varying support in the Legacy drivers. - -| driver name | wildcard | alias | prop | notes | -| --- | :-: | :-: | :-: | ---| -| acmedns | ? | no | no | | -| aliyun | ? | no | no | | -| aurora | ? | no | no | | -| cloudns | ? | no | no | test coverage 75% | -| cloudflare | ? | no | no | patch in #123, needs confirmation? | -| dnspod | ? | no | no | | -| duckdns | ? | no | no | | -| gandi | OK | no | no | wildcard & other fixes in 0.8.3 | -| hurricane | ? | no | no | test coverage 70% | -| powerdns | NO | no | no | apparently not in 0.8.2; bug #195 | -| rackspace | ? | no | no | test coverage 69% | -| route53 (1) | OK | no | no | wildcard since pre-0.8.2 | -| unbound_ssh | OK | yes | no | Working demonstrator model | - -> (1) route53 was never setup to be used from `sewer-cli`. That will change, -maybe for 0.8.3, but does anyone care? No complaints have been heard... - -_wildcard_ is NOT the older issue - since 0.8.2, all drivers should be able -to support creating certificates for simple `*.domain.tld` requests. -There is a deeper problem when one wants a wildcard that _also_ covers plain -`domain.tld`, which is sometimes desired. -Because of the way ACME and DNS work, such a certificate must post two -different challenge responses on the same DNS name (domain.tld). -DNS inherently supports this, and all the DNS services that have been -examined so far support it, but their APIs don't always make it easy. -And that's why some drivers cannot yet handle these _wildcard-plus_ -requests. _I'm sure some of those "unknowns" are "OK", but I can't find -evidence at this time._ - -_alias_ is the new (0.8.3) `--p_opts alias=...` feature, which none of the legacy -drivers initially supported. - -_prop_ is support for the "check & wait" propagation wait (engine support in 0.8.2), -which none of the legacy providers support (yet?). Note that `prop_delay`, -although passed through the driver, is working without driver support aside -from **kwargs. The other `prop_*` parameters are what this column reports. - -_updated pre-0.8.3 release_ diff --git a/docs/http-01.md b/docs/http-01.md index ba354476..40484145 100644 --- a/docs/http-01.md +++ b/docs/http-01.md @@ -1,3 +1,8 @@ -## HTTP challenge providers +# HTTP challenge providers -**placeholder** +There are no http-01 drivers in sewer yet. + +## Bring your own HTTP provider + +**To be rewritten.** For now, see `providers/demo.py` for some hints, and +[UnifiedProviders](UnifiedProviders) for doumentation of the interface. diff --git a/docs/sewer-as-a-library.md b/docs/sewer-as-a-library.md index 22c31a36..6309bc13 100644 --- a/docs/sewer-as-a-library.md +++ b/docs/sewer-as-a-library.md @@ -1,6 +1,58 @@ -## USAGE - sewer as a Python library - -### Basic usage example +# Sewer as a Python Library + +>`sewer-the-library` is in a period of heavy change (summer 2020 - ?). I'll +try to keep the examples (below) and other docs up to date, but I'm sure +things will lag sometimes. + +This document is neither a "cookbook" nor in any way a substitute for the +documentation of sewer's parts and internals, such as they are. Let's try +to list the existing docs: + +- [Cryptographic library](crypto) this is in decent shape because it's been + created and kept up to date in sync with crypto.py - both too new to have + bit rot yet. + +- [ACME protocol](ACME) was a piece I started writing while learning the + quirks of the ACMEprotocol. Quite incomplete, the main thing it brings to + the table is the link to RFC8555, which is the protocol's definition. Of + course there are other foundational RFCs to be read... + +- The [driver catalog](catalog) is another new part that isn't yet being + used to its fullest. It glues the drivers and the CLI program together, + and stands ready to help your bespoke front end likewise unless your + target is so specific you can just manually import the only driver you + need. + +- Drivers! So much of this is about those intermediaries between sewer and + the diverse services that actually publish our challenge responses. + + + [Unified provider](UnifiedProvider) began as a technical essay when I + was starting to sort through the problems and possibilities Alec's + original http-01 driver support introduced. It's an uneven blend of + design philosophy and code documentation, with plenty of ToDo in it. + + + [Wildcard certificates](wildcards) are one of the things the dns-01 + challenge type brought to the table. Some notes on how they work and + what issues remain. + + + [Aliasing](Aliasing) can be a handy technique to manage dns-01 + challenges without needing to deal with a primary DNS provider whose + support for fast-propagating, short-lived TXT records leaves something + to be desired. + + Speaking of DNS Propagation, we find [DNS propagation](DNS-Propagation), + which talks about what it is and why you might need it, and offers what + documentation there is on the parameters to pass to the drivers to + control it. And for driver writers, mostly, + [unpropagated](unpropagated) discusses what's needed to add a probe & + wait timeout loop. + +_more to do _ + +## Usage examples + +Keep in mind that these are untested code intended to demonstrate how the +major features are used. Supporting details may not be repeated for each +similar example, or may not be present in any of them. ```python import sewer.client @@ -83,8 +135,3 @@ certificate = client.cert() cert_key = client.cert_key acct_key = client.acct_key ``` - -### Bring your own HTTP provider - -**To be rewritten.** For now, see `providers/demo.py` for some hints, and -[UnifiedProviders](UnifiedProviders) for doumentation of the interface. diff --git a/docs/sewer-cli.md b/docs/sewer-cli.md index eee7b6c8..ef863123 100644 --- a/docs/sewer-cli.md +++ b/docs/sewer-cli.md @@ -1,4 +1,4 @@ -# Sewer's user command (so many --options!) +## Sewer's user command (so many --options!) Sewer's command line interface, historically named just "sewer" or "sewer-cli", and implemented in `sewer/cli.py`, is now also available using @@ -14,7 +14,36 @@ output from running `sewer-cli --help`, but that can be rather terse. Here we will discuss what the options are and why they are needed, especially some recently [or soon-to-be!] added options. -## sewer-cli General Options +### _key_type_ values + +This is new with the crypto overhaul (pre-0.8.4). You can now choose the +type and size of both the account and certificate keys to be generated by +sewer (if you don't pass it existing keys for one or both). +| _key_type_ | key & size | notes | +| --- | :-: | --- | +| rsa2048 | RSA 2048 bits | old sewer default | +| rsa3072 | RSA 3072 bits | NEW sewer default | +| rsa4096 | RSA 4096 bits | | +| secp256r1 | ECDSA 256 bits | | +| secp384r1 | ECDSA 384 bits | | +| secp521r1 | ECDSA 521 bits | **not accepted for certificate key** | + +> NOTE that the default generated key has changed from 2048 bit RSA to 3072 +bit RSA. This is in keeping with current NIST reccomendations. Unless you +have a need to continue to use RSA account keys (existing scripts assume +RSA, perhaps), one of the ECDSA types is suggested. + +The choice of key_type would be easy if not for external factors: ECDSA is +widely preferred on most grounds, but RSA may be required for backwards +compatibility with old software or appliances. Some new applications and +devices, OTOH, are dropping RSA due to its resource demands (CPU time and +memory). + +The 521 bit EC key is still valid in the specs, but currently most (?) browsers +don't support it, as LE has chosen to reject certificates using that key +type and size. + +### sewer-cli General Options `--version` `--known_providers` @@ -35,14 +64,14 @@ server key (the only thing that CAN be reused) depends only on whether If you need to increase this timeout you'll know it . _added by #188 in pre-0.8.3; reworked from #154 from @menduo_ -## ACME options +### ACME options `--endpoint` "production"|"staging" > Default is "production", viz., issue a legitimate certificate. Use "staging" for testing! _protocol changes enforced since late 2019 for staging are fixed in 0.8.2_ -## Account options +### Account options To an ACME server, an account is a key pair which has been registered with that server. Oh, there's other information that MAY be attached when it is @@ -54,22 +83,29 @@ By default, `sewer-cli` will create a new, unique key pair each time it's run. And this is okay, because it will also save the key alongside the certificate and the key that's attested to by the certificate. But if you don't want every cert to be issued to a new identity, you'll need to use -`--account_key` to provide the already-registered one. **NB:** sewer does not -currently provide any way to register, for the first time, a provided key. -_(this will change in the 0.9 overhaul)_ +`--acct_key` to provide the already-registered one. **New in 0.8.4:** you +can use a new, unregistered account key if you also use the --is_new_acct +option (and email, of course). After the certificate has been created and downloaded, the account key `sewer-cli` used will be saved alongside the certificate and certificate key. After this,the new account key is registered with the ACME endpoint -and may be used for future certificate requests using `--account_key`. +and may be used for future certificate requests using `--acct_key`. -`--account_key` _filepath_ +`--acct_key` _filepath_ > Filepath to existing, already registered ACME account key. Default is to -create a new key and register it. +create a new key and register it. Preferred over `--account_key` now. + +`acct_key_type` _key_type_ +> Type of key to generate if `--acct_key` not given. Default is rsa3087. + +`--is_new_acct` +> Used with `--acct_key`, allows a key you created outside of sewer to be +registered as an account key the first time it's used. `--email` _email_address_ -## Challenge publisher options +### Challenge publisher options `--provider`|`--dns` **name** > Name of the [DNS] provider to use. @@ -80,7 +116,7 @@ As of 0.8.3 this still only supports the legacy DNS providers. in accordance with argparse's good advice that "users expect options to be optional"._ -### Driver parameters +#### Driver parameters During the pre-0.8.3 work, several long options were added to `sewer-cli` for individual new driver parameters. These single-use options will be @@ -92,7 +128,7 @@ introduction of `--p_opts`. parameters into the drivers when 0.8.3 releases. Like `--alt_domains`, there can be any number of named parameters following `--p_opts`. -#### Propagation management parameters +##### Propagation management parameters There are two sorts of things for which we have to wait: the ACME server (see `--acme_timeout`) and the service provider (especially DNS propagation @@ -125,7 +161,7 @@ first, second, ... _not all ready_ response from the driver's `unpropagated` method. The last value is re-used after the list has been used up. Default is "1,2,4,8". _to be added when there's driver support_ -#### DNS driver parameters +##### DNS driver parameters `--p_opts alias_domain=` > Configure an alternate DNS domain in which the challenge responses will be @@ -134,7 +170,7 @@ providers accept this, but require further modification to actually apply the aliasing that's supported by their parent classes.** _This was `--alias_domain ` during 0.8.3 development.` -## Certificate info +### Certificate info `--domain` **CN-name** > The primary identity for the certificate. REQUIRED, no default. CN-name @@ -151,11 +187,14 @@ Default is an empty list. > Set directory where the certificate and key files will be stored. Default is to use the current working directory where sewer was run. -`--certificate_key` _filepath_ -> Full path to your [existing] certificate key. As with the account_key, if -this is not specified, a new key will be created and used (in the -certificate). Has a similar effect to certbot's `--reuse-key` (sp?) if it -points to the key file from the previous run. +`--cert_key` _filepath_ +> File path to your existing certificate key. Preferred over +`--certificate_key`. As with `acct_key`, if this is not specified, a new +key will be created. Has a similar effect to certbot's `--reuse-key` (sp?) +if it points to the key file from the previous run. + +`--cert_key_type` key_type +> Type of key to generate if `--cert_key` not given. Default is rsa3087. --bundle_name _basename_ > Base name to use for output file, eg., out_dir/basename.{account.key,crt,key} diff --git a/docs/unpropagated.md b/docs/unpropagated.md index 09578de7..e4dd2ba8 100644 --- a/docs/unpropagated.md +++ b/docs/unpropagated.md @@ -1,4 +1,4 @@ -## Waiting for the Challenge to Propagate +# Waiting for the Challenge to Propagate When you use a service provider's API to setup a challenge response, how long does it take before the ACME server can reliably get that answer? @@ -6,7 +6,7 @@ Especially with global anycast DNS services, it can take a while! This delay between _posted to API_ and _actually online everywhere that matters_ is the propagation delay we're talking about here. -### How shall we wait, let me count the ways +## How shall we wait, let me count the ways sewer provides two kinds of delay that can be used to deal with propagation within the service provider's systems. Although this was designed mostly @@ -15,7 +15,7 @@ service provider. The two kinds of delay are (1) an unconditional sleep and (2) an iterative probe/sleep delay loop that has a timeout to keep it from waiting _too long_. -#### To sleep, perchance t'will be enough +### To sleep, perchance t'will be enough The unconditional sleep is implemented in the sewer core logic and is available to any driver which can pass `prop_delay=seconds_to_sleep` along @@ -25,7 +25,7 @@ all been setup. --- Available in drivers in 0.8.3. -#### Check twice, respond once +### Check twice, respond once The iterative probe is a more active sort of delay: it repeatedly calls the driver's `unpropagated` method to test whether the challenges are all in @@ -47,7 +47,7 @@ This does also require an implementaion of the `unpropagated` method in the driver. The only sane default, used in the legacy drivers' shim class, is to always return success without any actual checking. -### Advice to driver authors and users +## Advice to driver authors and users Authors: If the service gives you a way to do a meaningful check and it's needed, please implement `unpropagated`, and mention that in the driver's diff --git a/docs/wildcards.md b/docs/wildcards.md index 10d55d70..7e7ee0be 100644 --- a/docs/wildcards.md +++ b/docs/wildcards.md @@ -1,4 +1,4 @@ -## Wildcard Certificates +# Wildcard Certificates Since 0.8.2, sewer should be able to request and receive simple wildcard certificates using any of the DNS drivers. In earlier versions there was an @@ -6,7 +6,7 @@ eccentric re-naming of wildcard targets in the core logic which the drivers would, sometimes unreliably, remove. _tl;dr: before 0.8.2 it depended on the driver._ -### One issue remains in 0.8.3 +## One issue remains in 0.8.3 Certificates with a wildcard CN name, eg., `domain=*.example.com`, are valid for all and only the immediate sub domains of example.com. They do NOT diff --git a/sewer/auth.py b/sewer/auth.py index 0192838c..594013bb 100644 --- a/sewer/auth.py +++ b/sewer/auth.py @@ -20,7 +20,7 @@ def __init__( LOG_LEVEL: str = "INFO", prop_delay: int = 0, prop_timeout: int = 0, - prop_sleep_times: Union[Sequence[int], int] = (1, 2, 4, 8) + prop_sleep_times: Union[Sequence[int], int] = (1, 2, 4, 8), ) -> None: # TypeError if missing, still check that it's a sequencey value; non-str vals, meh diff --git a/sewer/catalog.py b/sewer/catalog.py index f00357f1..794fc804 100644 --- a/sewer/catalog.py +++ b/sewer/catalog.py @@ -16,7 +16,7 @@ def __init__( path: str = None, cls: str = None, features: Sequence[str] = None, - memo: str = None + memo: str = None, ) -> None: "initialize a driver descriptor from one item in the catalog" diff --git a/sewer/cli.py b/sewer/cli.py index 0755c7af..90e13d8b 100644 --- a/sewer/cli.py +++ b/sewer/cli.py @@ -6,6 +6,9 @@ from .crypto import AcmeKey, key_type_choices +DEFAULT_KEY_TYPE = "rsa3072" + + def setup_parser(catalog): """ return configured ArgumentParser - catalog-driven list of providers @@ -41,15 +44,25 @@ def setup_parser(catalog): "--account_key", dest="acct_key_file", type=argparse.FileType("rb"), - help="Filepath to read to get registered ACME account. Default is to create one.", + help="File to load registered ACME account key from. Default is to create one.", ) - parser.add_argument("--email", help="Email to be used for registration of an ACME account.") parser.add_argument( "--acct_key_type", choices=key_type_choices, - default="rsa2048", - help="Select type of account key to generate if not loading per --acct_key. Default rsa2048.", + default=DEFAULT_KEY_TYPE, + help=( + "Type of acct key to generate if not loaded by --acct_key. Default %s." + % DEFAULT_KEY_TYPE + ), + ), + + parser.add_argument("--email", help="Email to be used for registration of an ACME account.") + + parser.add_argument( + "--is_new_acct", + action="store_true", + help="Register the key (from --acct_key) rather than assuming it's already registered.", ), ### certificate options @@ -59,13 +72,17 @@ def setup_parser(catalog): "--certificate_key", dest="cert_key_file", type=argparse.FileType("rb"), - help="Filepath to read to get certificate key. Default is to create one.", + help="File to load existing certificate key from. Default is to create key.", ) + parser.add_argument( "--cert_key_type", - choices=key_type_choices, - default="rsa2048", - help="Select type of certificate key to generate if not loading per --cert_key. Default rsa2048.", + choices=[kt for kt in key_type_choices if kt != "secp521r1"], + default=DEFAULT_KEY_TYPE, + help=( + "Type of cert key to generate if not loaded by --cert_key. Default %s." + % DEFAULT_KEY_TYPE + ), ), parser.add_argument( @@ -344,7 +361,7 @@ def main(): if args.acct_key_file: acct_key = AcmeKey.from_bytes(args.acct_key_file.read()) - is_new_acct = False + is_new_acct = args.is_new_acct else: acct_key = AcmeKey.create(args.acct_key_type) is_new_acct = True @@ -388,7 +405,7 @@ def main(): acct_key.to_file(account_key_file_path) logger.info("account key succesfully written to {0}.".format(account_key_file_path)) - certificate = acme_client.cert() + certificate = acme_client.get_certificate() # write out certificate and certificate key in out_dir directory with open(crt_file_path, "w") as certificate_file: diff --git a/sewer/client.py b/sewer/client.py index 8a6734fd..aed8791a 100644 --- a/sewer/client.py +++ b/sewer/client.py @@ -37,31 +37,29 @@ def __init__( ### do some type checking of some parameters - ### FIX ME ### spotty and not always complete; also, should raise TypeError, not ValueError + ### FIX ME ### spotty and not always complete; some should raise TypeError, not ValueError if not isinstance(domain_alt_names, (type(None), list)): raise ValueError( - """domain_alt_names should be of type:: None or list. You entered {0}""".format( - type(domain_alt_names) - ) + "domain_alt_names should be None or a list of strings, not %s" % domain_alt_names ) - elif not isinstance(contact_email, (type(None), str)): - raise ValueError( - """contact_email should be of type:: None or str. You entered {0}""".format( - type(contact_email) - ) - ) - elif LOG_LEVEL.upper() not in ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]: + + if not isinstance(contact_email, (type(None), str)): + raise ValueError("contact_email should be None or a string, not %s" % contact_email) + + if LOG_LEVEL.upper() not in ["DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"]: raise ValueError( - """LOG_LEVEL should be one of; 'DEBUG', 'INFO', 'WARNING', 'ERROR' or 'CRITICAL'. not {0}""".format( - LOG_LEVEL - ) + "LOG_LEVEL must be one of 'DEBUG', 'INFO', 'WARNING', 'ERROR' or 'CRITICAL'" ) - elif dns_class is not None and provider is not None: + + if dns_class is not None and provider is not None: raise ValueError( - "passed both the DEPRECATED 'dns_class' parameter as well as 'provider'." + "Client was passed both the DEPRECATED dns_class argument and provider." ) + if not isinstance(acct_key, AcmeKey) or not isinstance(cert_key, AcmeKey): + raise TypeError("Arguments acct_key and cert_key MUST be instances of crypto.AcmeKey.") + ### setup Client's global variables self.domain_name = domain_name @@ -212,7 +210,7 @@ def acme_register(self): with "not self.is_new_acct". But further work is needed - I don't think a 409 result is part of the protocol any more, for one thing. """ - self.logger.info("acme_register (newAccount)") + self.logger.info("acme_register%s" % " (is new account)" if self.is_new_acct else "") if not self.is_new_acct: payload = {"onlyReturnExisting": True} elif self.contact_email: @@ -510,12 +508,13 @@ def get_acme_header(self, url, needs_jwk=False): - "url" """ self.logger.debug("get_acme_header") - header = {"alg": "RS256", "nonce": self.get_nonce(), "url": url} + header = {"alg": self.acct_key.jws_alg, "nonce": self.get_nonce(), "url": url} if needs_jwk: header["jwk"] = self.acct_key.jwk() else: header["kid"] = self.acct_key.kid + return header def make_signed_acme_request(self, url, payload, needs_jwk=False): diff --git a/sewer/crypto.py b/sewer/crypto.py index 7016e48a..548d6eb3 100644 --- a/sewer/crypto.py +++ b/sewer/crypto.py @@ -1,7 +1,7 @@ from cryptography import x509 from cryptography.x509.oid import NameOID from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives.asymmetric import ec, padding, rsa +from cryptography.hazmat.primitives.asymmetric import ec, padding, rsa, utils from cryptography.hazmat.primitives.serialization import ( load_pem_private_key, Encoding, @@ -24,7 +24,7 @@ # AcmeKeyType = Union[RsaKeyType, EcKeyType] # and why does this [seem] to work? -AcmeKeyType = Union[openssl.rsa._RSAPrivateKey, openssl.ec._EllipticCurvePrivateKey] +PrivateKeyType = Union[openssl.rsa._RSAPrivateKey, openssl.ec._EllipticCurvePrivateKey] class AcmeKey: @@ -37,19 +37,19 @@ class AcmeKey: See new_ACME_key and load_ACME_key factory functions below. """ - pk: AcmeKeyType - - private_format: Any = None - jwk_const: Dict[str, str] - jwk_attr: Dict[str, str] - - def __init__(self, pk: AcmeKeyType) -> None: + def __init__(self, pk: PrivateKeyType) -> None: self.pk = pk + # subclass dependent, assigned there + self.private_format: Any + self.jwk_const: Dict[str, str] + self.jwk_attr: Dict[str, str] + self.jws_alg: str + # shared fields self.kid: Optional[str] = None self._jwk: Optional[Dict[str, str]] = None @staticmethod - def create(key_type: str) -> AcmeKeyType: + def create(key_type: str) -> "AcmeKey": """ Factory method to create a new key of key_type, returned as an AcmeKey. """ @@ -58,17 +58,10 @@ def create(key_type: str) -> AcmeKeyType: raise ValueError("AcmeKey.create: unrecognized key_type: %s" % key_type) Cls, arg = key_type_map[key_type] - # ? # I suppose this could be inverted to keep the key type specifics in Cls - - if Cls is AcmeRsaKey: - return Cls(rsa.generate_private_key(65537, arg, default_backend())) - if Cls is AcmeEcKey: - return Cls(ec.generate_private_key(arg, default_backend())) - - raise ValueError("AcmeKey.create: got bad class from type_map") + return Cls(Cls.create_pk(arg)) @staticmethod - def from_bytes(pem_data: bytes) -> AcmeKeyType: + def from_bytes(pem_data: bytes) -> "AcmeKey": """ load a key from the PEM-format bytes, return an AcmeKey @@ -85,7 +78,7 @@ def from_bytes(pem_data: bytes) -> AcmeKeyType: raise ValueError("AcmeKey.from_bytes: unrecognized key type: %s" % pk) @staticmethod - def from_file(filename: str) -> AcmeKeyType: + def from_file(filename: str) -> "AcmeKey": "convenience method to load a PEM-format key; returns the AcmeKey" with open(filename, "rb") as f: @@ -132,7 +125,7 @@ def jwk(self) -> Dict[str, str]: jwk = dict(self.jwk_const) # pylint: disable=E1101 for name, attr_name in self.jwk_attr.items(): # pylint: disable=E1101 val = getattr(pubnums, attr_name) - numbytes = (val.bit_length() + 7) // 8 + numbytes = getattr(self, "jwk_num_bytes", (val.bit_length() + 7) // 8) jwk[name] = safe_base64(val.to_bytes(numbytes, "big")) self._jwk = jwk return self._jwk @@ -145,44 +138,79 @@ def jwk(self) -> Dict[str, str]: class AcmeRsaKey(AcmeKey): + def __init__(self, pk: PrivateKeyType) -> None: + super().__init__(pk) + self.private_format = PrivateFormat.PKCS8 + self.jwk_const = {"kty": "RSA"} + self.jwk_attr = {"e": "e", "n": "n"} + self.jws_alg = "RS256" - # maybe TraditionalOpenSSL to come out the same as testdata keys? - private_format = PrivateFormat.PKCS8 - jwk_const = {"kty": "RSA"} - jwk_attr = {"e": "e", "n": "n"} + @staticmethod + def create_pk(key_size: int) -> PrivateKeyType: + return rsa.generate_private_key(65537, key_size, default_backend()) def sign_message(self, message: bytes) -> bytes: signature = self.pk.sign(message, padding.PKCS1v15(), hashes.SHA256()) return signature -class AcmeEcKey(AcmeKey): +class EcCurve: + "data class to hold EX info that's not readily inferred from the secp### tag" + + def __init__(self, curve: Any, alg: str, hash_type: Any, nbytes: int) -> None: + self.curve = curve + self.alg = alg + self.hash_type = hash_type + self.nbytes = nbytes - private_format = PrivateFormat.TraditionalOpenSSL - # jwk_const has to be set based on the key size - jwk_attr = {"x": "x", "y": "y"} - def __init__(self, pk: AcmeKeyType) -> None: - self.jwk_const = {"kty": "EC", "crv": "P-%s" % pk.curve.key_size} +known_curves: Dict[int, EcCurve] = { + 256: EcCurve(ec.SECP256R1, "ES256", hashes.SHA256, 32), + 384: EcCurve(ec.SECP384R1, "ES384", hashes.SHA384, 48), + # + # I thought LE accepted P-521 for account keys, but while chasing an intermitent bug + # 'detail': 'ECDSA curve P-521 not allowed' + # + # 521: EcCurve(ec.SECP521R1, "ES512", hashes.SHA512, 66), +} + + +class AcmeEcKey(AcmeKey): + def __init__(self, pk: PrivateKeyType) -> None: + key_size = pk.curve.key_size + info = known_curves[key_size] super().__init__(pk) + self.private_format = PrivateFormat.PKCS8 + self.jwk_const = {"kty": "EC", "crv": "P-%s" % key_size} + self.jwk_attr = {"x": "x", "y": "y"} + self.jws_alg = info.alg + + self.jkw_num_bytes = info.nbytes + + @staticmethod + def create_pk(key_size: int) -> PrivateKeyType: + info = known_curves[key_size] + return ec.generate_private_key(info.curve, default_backend()) def sign_message(self, message: bytes) -> bytes: - signature = self.pk.sign(message, ec.ECDSA(hashes.SHA256())) + info = known_curves[self.pk.curve.key_size] + # EC.sign returns ASN.1 encoded values for some inane reason + r, s = utils.decode_dss_signature(self.pk.sign(message, ec.ECDSA(info.hash_type()))) + signature = r.to_bytes(info.nbytes, "big") + s.to_bytes(info.nbytes, "big") + return signature -# Key Type Registry -# -# key_type_map - maps key type name to the AcmeKey subclass that handles it and -# some magic values that are arguments needed for creating a new key. +# key_type_map +# map known key type names to (concrete_class, key_size) -key_type_map: Dict[str, Tuple[Any, ...]] = { +key_type_map: Dict[str, Tuple[Any, int]] = { "rsa2048": (AcmeRsaKey, 2048), "rsa3072": (AcmeRsaKey, 3072), "rsa4096": (AcmeRsaKey, 4096), - "secp256r1": (AcmeEcKey, ec.SECP256R1), - "secp384r1": (AcmeEcKey, ec.SECP384R1), - "secp521r1": (AcmeEcKey, ec.SECP521R1), + "secp256r1": (AcmeEcKey, 256), + "secp384r1": (AcmeEcKey, 384), + "secp521r1": (AcmeEcKey, 521), } # extract just the names for option choice lists, etc. @@ -201,7 +229,7 @@ def __init__(self, *, cn: str, san: List[str], key: AcmeKey) -> None: csrb = x509.CertificateSigningRequestBuilder() csrb = csrb.subject_name(x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, cn)])) all_names = list(set([cn] + san)) - SAN: List = [x509.DNSName(name) for name in all_names] + SAN: List[x509.GeneralName] = [x509.DNSName(name) for name in all_names] csrb = csrb.add_extension(x509.SubjectAlternativeName(SAN), critical=False) self.csr = csrb.sign(key.pk, hashes.SHA256(), default_backend()) diff --git a/sewer/tests/test_Client.py b/sewer/tests/test_Client.py index cf160d0c..0fd4c9ec 100644 --- a/sewer/tests/test_Client.py +++ b/sewer/tests/test_Client.py @@ -248,9 +248,7 @@ def mock_instantiate_client(): with self.assertRaises(ValueError) as raised_exception: mock_instantiate_client() - self.assertIn( - "domain_alt_names should be of type:: None or list", str(raised_exception.exception) - ) + self.assertIn("None or a list of strings", str(raised_exception.exception)) class TestClientForSAN(TestClient): @@ -384,9 +382,7 @@ def mock_instantiate_client(): with self.assertRaises(ValueError) as raised_exception: mock_instantiate_client() - self.assertIn( - "domain_alt_names should be of type:: None or list", str(raised_exception.exception) - ) + self.assertIn("None or a list of strings", str(raised_exception.exception)) class TestClientUnits(TestCase): diff --git a/sewer/tests/test_crypto.py b/sewer/tests/test_crypto.py index 6f4a1c34..bf6b0b24 100644 --- a/sewer/tests/test_crypto.py +++ b/sewer/tests/test_crypto.py @@ -11,19 +11,32 @@ class Test_AcmeKey(unittest.TestCase): rsa_key_types = (("rsa2048", 2048), ("rsa3072", 3072), ("rsa4096", 4096)) - secp_key_types = (("secp256r1", 256), ("secp384r1", 384), ("secp521r1", 521)) + secp_key_types = (("secp256r1", 256), ("secp384r1", 384)) def fromfile_privbytes_frombytes_sign_key(self, key_type: KeyType) -> None: + """ + this has grown into a test for almost everything we do with keys + """ + type_name, key_size = key_type filename = "test/%s.pem" % type_name + # from_file loaded_key = AcmeKey.from_file(filename) self.assertTrue(loaded_key.pk.key_size == key_size) - # private_bytes + + # private_bytes ("assert" no exception) loaded_pb = loaded_key.private_bytes() + + # private bytes matches original file + with open(filename, "rb") as f: + file_bytes = f.read() + self.assertTrue(loaded_pb == file_bytes) + # from_bytes reloaded_key = AcmeKey.from_bytes(loaded_pb) self.assertTrue(loaded_key.pk.private_numbers() == reloaded_key.pk.private_numbers()) + # sign_message self.assertTrue(len(loaded_key.sign_message(b"Taketh uth to thine leaderth"))) From 31f1c4c6c4a23a080c3577c2c3ead72dd927c254 Mon Sep 17 00:00:00 2001 From: Martin Maney Date: Thu, 10 Sep 2020 13:14:01 -0500 Subject: [PATCH 3/3] Final small adjustments to match what LE staging doesn't accept. --- sewer/crypto.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/sewer/crypto.py b/sewer/crypto.py index 548d6eb3..ef667cf3 100644 --- a/sewer/crypto.py +++ b/sewer/crypto.py @@ -17,7 +17,7 @@ ### types for things defined here -### FIX ME ### what can we use for XxxKeyType? +### FIX ME ### what can we use for XxxKeyType? [[ not vital, just tightens up typing ]] # RsaKeyType = openssl.rsa._RSAPrivateKey # EcKeyType = openssl.ec._EllipticCurvePrivateKey @@ -108,6 +108,8 @@ def to_file(self, filename: str) -> None: with open(filename, "wb") as f: f.write(self.private_bytes()) + ### TODO ### store & load file format with kid, PK, timestamp registered, ? + def sign_message(self, message: bytes) -> bytes: raise NotImplementedError("subclass must implement sign_message") @@ -150,6 +152,11 @@ def create_pk(key_size: int) -> PrivateKeyType: return rsa.generate_private_key(65537, key_size, default_backend()) def sign_message(self, message: bytes) -> bytes: + """ + Yes, SHA256 is hardwired. As of Sep 2020, LE only accepts that hash + for RSA keys: "expected one of RS256, ES256, ES384 or ES512". + """ + signature = self.pk.sign(message, padding.PKCS1v15(), hashes.SHA256()) return signature @@ -210,7 +217,6 @@ def sign_message(self, message: bytes) -> bytes: "rsa4096": (AcmeRsaKey, 4096), "secp256r1": (AcmeEcKey, 256), "secp384r1": (AcmeEcKey, 384), - "secp521r1": (AcmeEcKey, 521), } # extract just the names for option choice lists, etc. @@ -224,6 +230,11 @@ class AcmeCsr: def __init__(self, *, cn: str, san: List[str], key: AcmeKey) -> None: """ temporary "just like Client.create_csr", more or less + + TODO: "must staple" extension; NOT elaborating subject name, since LE + suggests that even CN may be replaced by a meaningless number in some + vague future version of the server. I guess they're right that + browsers ignore the CN already (aside from displaying it if asked). """ csrb = x509.CertificateSigningRequestBuilder()