PKISharp / ACMESharpCore

An ACME v2 client library for .NET Standard (Let's Encrypt)
MIT License
325 stars 72 forks source link

Example code used by ACMECLI resets KeySize for RSA algorithm #31

Open pat-weisman opened 4 years ago

pat-weisman commented 4 years ago

First, thanks for this port to ACME v2! I ran into a minor issue when experimenting with the CLI reference implementation. If I choose to use RSA, rather than the default ES, algorithm, the JwsAlg is properly stored as RS256 (hashSize=256, keySize=2048). However, when the state information is restored, in ../src/Examples/ACMECLI/Program.cs:

                // ...
176:            if (LoadStateInto(ref accountKey, failThrow: false,
177:                    Constants.AcmeAccountKeyFile))
178:            {
179:                accountSigner = accountKey.GenerateTool();
                    // ...
186:            }

The current implementation of GenerateTool() resets tool.KeySize, rather than tool.HashSize when RS-style, vs ES-style keytypes are used. I made this change to correct this locally:

$ git diff ExamplesAccountKey.cs
diff --git a/src/examples/Examples.Common.PKI/ExamplesAccountKey.cs b/src/examples/Examples.Common.PKI/ExamplesAccountKey.cs
index 989bd3e..7b12c3e 100644
--- a/src/examples/Examples.Common.PKI/ExamplesAccountKey.cs
+++ b/src/examples/Examples.Common.PKI/ExamplesAccountKey.cs
@@ -24,7 +24,7 @@ namespace Examples.Common.PKI
             if (KeyType.StartsWith("RS"))
             {
                 var tool = new ACMESharp.Crypto.JOSE.Impl.RSJwsTool();
-                tool.KeySize = int.Parse(KeyType.Substring(2));
+                tool.HashSize = int.Parse(KeyType.Substring(2));
                 tool.Init();
                 tool.Import(KeyExport);
                 return tool;

I believe the above is the intended behavior. I'll submit a PR with this simple change for you to review and merge if it is acceptable. (See: https://github.com/PKISharp/ACMESharpCore/pull/32)