Cryptographic refactoring; adds ECDSA keys; removes pyopenssl as dependency (#212)

* Crypto refactoring, with the addition of ECDSA keys, including cli options.
Incompatible changes in Client.__init__ interface and some internals!

* Crypto fixes, clarifications, lots of documentation updates, and no, LE doesn't P-521 :-(

* Final small adjustments to match what LE staging doesn't accept.

Author: mmaney

.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)

Makefile

@@ -60,10 +60,27 @@ 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
 	@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 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 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

README.md

@@ -1,5 +1,22 @@
 ## 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
+  + 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)
 [![codecov](https://codecov.io/gh/komuW/sewer/branch/master/graph/badge.svg)](https://codecov.io/gh/komuW/sewer)
@@ -8,7 +25,7 @@
 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).
 

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.

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:

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!

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,15 +11,15 @@ 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
 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

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.