ECDSA support.

[jfb]

What(What have you changed?)

add ECDSA support.

Why(Why did you change it?)

ECDSA better than RSA.

https://github.com/komuw/sewer/issues/202

[codecov[bot]]

Codecov Report

Merging #203 into master will decrease coverage by 0.00%. The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
- Coverage   88.28%   88.27%   -0.01%     
==========================================
  Files          19       19              
  Lines        1195     1203       +8     
==========================================
+ Hits         1055     1062       +7     
- Misses        140      141       +1     

Impacted Files	Coverage Δ
sewer/client.py	92.02% <88.88%> (-0.12%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4bcd909...00c95e4. Read the comment docs.

[jfb]

@mmaney

Default use RSA

import sewer.client
client = sewer.client.Client("example.com")

use ECDSA

import sewer.client
client = sewer.client.Client("example.com", curves="secp256r1")

[mmaney]

More to the point, the code in create_certificate has a conditional part that my eyes apparently skipped right over. Late night, perhaps not enough coffee this morning.

It needs test(s) for the new method, which should be easy to add. It will also need support in cli.py, which is probably where that little voice whispering that there's more to be done around loading and saving non-RSA keys is coming from. And if it had all that in place it would still wait until after 0.8.3 is out. :-/

And in that longer timeframe, there's a goal to get rid of the one or two uses of the... disparaged would be the word, I think, for OpenSsl.crypto, moving to the suggested cryptography library (as you've used in this). And along the way to that goal I expect all the crypto methods will wind up in a sewer.crypto library - this is one part of a general housecleaning and breakup of the global Client namespace that the code lives in now in favor of more cohesive classes. Maybe I can put the initial refactor into crypto.py near the top of the list post-release. Would you be interested in working on that, or are you just focused on the EC keys? (thinking about that, they can be used for the account keys on the LE side now, no?)

[mmaney]

So I've been looking at using ECDSA for the account keys, and that would add a lot of fussy changes. But I'm comfortable enough about what it will need to let this in... though of course there are a few changes. :-)

I think this revised code from cli at least hints at most of what I think is needed:

    parser.add_argument(
        "--certificate_type",
        choices = ["rsa2048", "rsa4096", "secp256r1", "secp384r1", "secp512r1"],
        help="Type of key to generate for the certificate."
    )

Same change of name in Client.init, and name-based switching logic between create_rsa and create_ec. account_type can be added later, and bits can be deprecated after a while. Oh, a bit of annoying logic to report on use of both bits and certificate_type. I have the feeling I'm forgetting something else you changed in the PR...

[mmaney]

@jfb It was sticky - I kept coming back to this bit of work. The crypto branch is intended to be merged into the trunk for the 0.8.4 release. Does this take care of your use cases satisfactorily? Aside from the testing and linting (and a mypy check that isn't incorporated into the test suite yet), I've generated a certificate with an EC key from staging.

[mmaney]

Oops... never tested with an EC account key (I knew there were other things that needed attention) and that's not working yet. :-(

[jfb]

@mmaney Thank you for all you have done for us!