From b0becec26ed41435708f1393533fceccd2276f77 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 03:38:10 +0100 Subject: [PATCH 1/7] support --keysize N cmdline param to give RSA key size also: improve tests for usual key sizes --- letsencrypt/client/client.py | 9 ++++++--- letsencrypt/client/crypto_util.py | 2 +- letsencrypt/client/tests/crypto_util_test.py | 2 ++ letsencrypt/scripts/main.py | 5 ++++- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index dd4e23c6e..ecbcf6fc4 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -330,7 +330,7 @@ def validate_key_csr(privkey, csr=None): "The key and CSR do not match") -def init_key(): +def init_key(key_size): """Initializes privkey. Inits key and CSR using provided files or generating new files @@ -339,7 +339,10 @@ def init_key(): the namedtuple to easily work with the protocol. """ - key_pem = crypto_util.make_key(CONFIG.RSA_KEY_SIZE) + if key_size < CONFIG.RSA_KEY_SIZE: + logging.warning("Generating keys smaller than %d bits is NOT recommended!", CONFIG.RSA_KEY_SIZE) + + key_pem = crypto_util.make_key(key_size) # Save file le_util.make_or_verify_dir(CONFIG.KEY_DIR, 0o700) @@ -348,7 +351,7 @@ def init_key(): key_f.write(key_pem) key_f.close() - logging.info("Generating key: %s", key_filename) + logging.info("Generating key (%d bits): %s", key_size, key_filename) return Client.Key(key_filename, key_pem) diff --git a/letsencrypt/client/crypto_util.py b/letsencrypt/client/crypto_util.py index c11719343..627e51cb6 100644 --- a/letsencrypt/client/crypto_util.py +++ b/letsencrypt/client/crypto_util.py @@ -145,7 +145,7 @@ def csr_matches_pubkey(csr, privkey): # based on M2Crypto unit test written by Toby Allsopp -def make_key(bits=CONFIG.RSA_KEY_SIZE): +def make_key(bits): """Generate PEM encoded RSA key. :param int bits: Number of bits, at least 1024. diff --git a/letsencrypt/client/tests/crypto_util_test.py b/letsencrypt/client/tests/crypto_util_test.py index e80988d83..3e943e898 100644 --- a/letsencrypt/client/tests/crypto_util_test.py +++ b/letsencrypt/client/tests/crypto_util_test.py @@ -98,6 +98,8 @@ class MakeKeyTest(unittest.TestCase): def test_it(self): from letsencrypt.client.crypto_util import make_key M2Crypto.RSA.load_key_string(make_key(1024)) + M2Crypto.RSA.load_key_string(make_key(2048)) + M2Crypto.RSA.load_key_string(make_key(4096)) class ValidPrivkeyTest(unittest.TestCase): diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index ff3c3c792..5fc1a9bcb 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -37,6 +37,9 @@ def main(): parser.add_argument("-b", "--rollback", dest="rollback", type=int, default=0, metavar="N", help="Revert configuration N number of checkpoints.") + parser.add_argument("-B", "--keysize", dest="key_size", type=int, + default=CONFIG.RSA_KEY_SIZE, metavar="N", + help="RSA key shall be sized N bits.") parser.add_argument("-k", "--revoke", dest="revoke", action="store_true", help="Revoke a certificate.") parser.add_argument("-v", "--view-config-changes", @@ -100,7 +103,7 @@ def main(): # Prepare for init of Client if args.privkey is None: - privkey = client.init_key() + privkey = client.init_key(args.key_size) else: privkey = client.Client.Key(args.privkey[0], args.privkey[1]) From 8070b78e3d22993872bbe3766cb0f073ebea1254 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 04:57:47 +0100 Subject: [PATCH 2/7] talk to servers on non-standard https port (like -s acme.example.org:8443) note: I removed Network.server attribute, it was not used (only .server_url is used). I also removed the sanity-check for the ACME server (so one can use server:port syntax, which was considered invalid). I don't think this sanity-check is needed, because in the end, we can't fully check if it is correct anyway (you can always give valid, but non-working server names or ports). Also, many people will use the default ACME server, which is builtin and correct. --- letsencrypt/client/client.py | 1 - letsencrypt/client/network.py | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index dd4e23c6e..0100f4f4a 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -59,7 +59,6 @@ class Client(object): :type dv_auth: :class:`letsencrypt.client.interfaces.IAuthenticator` """ - sanity_check_names([server]) self.network = network.Network(server) self.authkey = authkey diff --git a/letsencrypt/client/network.py b/letsencrypt/client/network.py index 135349c41..82c98c363 100644 --- a/letsencrypt/client/network.py +++ b/letsencrypt/client/network.py @@ -17,13 +17,12 @@ logging.getLogger("requests").setLevel(logging.WARNING) class Network(object): """Class for communicating with ACME servers. - :ivar str server: Certificate authority server + :ivar str server: Certificate authority server (server[:port]) :ivar str server_url: Full URL of the CSR server """ def __init__(self, server): - self.server = server - self.server_url = "https://%s/acme/" % self.server + self.server_url = "https://%s/acme/" % server def send(self, msg): """Send ACME message to server. From 93b247c4838f02c94596aceea16267ca7936bce7 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 13:56:57 +0100 Subject: [PATCH 3/7] misc. improvements to ACME servername:port code changed ACME_SERVER to include the port. this is so the commandline options help will also display the port and also to make it clear at the constant definition that giving a port is supported there. improved docstrings --- letsencrypt/client/CONFIG.py | 4 ++-- letsencrypt/client/network.py | 8 ++++++-- letsencrypt/scripts/main.py | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/letsencrypt/client/CONFIG.py b/letsencrypt/client/CONFIG.py index 7d0b581fb..e65277221 100644 --- a/letsencrypt/client/CONFIG.py +++ b/letsencrypt/client/CONFIG.py @@ -2,8 +2,8 @@ import os.path -ACME_SERVER = "letsencrypt-demo.org" -"""CA hostname. +ACME_SERVER = "letsencrypt-demo.org:443" +"""CA hostname (and optionally :port). If you create your own server... change this line diff --git a/letsencrypt/client/network.py b/letsencrypt/client/network.py index 82c98c363..2ec93136d 100644 --- a/letsencrypt/client/network.py +++ b/letsencrypt/client/network.py @@ -17,11 +17,15 @@ logging.getLogger("requests").setLevel(logging.WARNING) class Network(object): """Class for communicating with ACME servers. - :ivar str server: Certificate authority server (server[:port]) - :ivar str server_url: Full URL of the CSR server + :ivar str server_url: Full URL of the ACME service """ def __init__(self, server): + """Initialize Network instance. + + :param str server: ACME (CA) server[:port] + + """ self.server_url = "https://%s/acme/" % server def send(self, msg): diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index ff3c3c792..c308b55da 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -30,7 +30,7 @@ def main(): parser.add_argument("-d", "--domains", dest="domains", metavar="DOMAIN", nargs="+") parser.add_argument("-s", "--server", dest="server", - help="The ACME CA server address.") + help="The ACME CA server [%s]." % CONFIG.ACME_SERVER) parser.add_argument("-p", "--privkey", dest="privkey", type=read_file, help="Path to the private key file for certificate " "generation.") From e58c344649625908a08885c0dfb567e25cce315d Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 14:17:14 +0100 Subject: [PATCH 4/7] handle invalid key sizes in a helpful way --- letsencrypt/client/client.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index ecbcf6fc4..dd484cc31 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -339,10 +339,12 @@ def init_key(key_size): the namedtuple to easily work with the protocol. """ - if key_size < CONFIG.RSA_KEY_SIZE: - logging.warning("Generating keys smaller than %d bits is NOT recommended!", CONFIG.RSA_KEY_SIZE) - - key_pem = crypto_util.make_key(key_size) + try: + key_pem = crypto_util.make_key(key_size) + except ValueError as err: + logging.fatal(str(err)) + logging.info("Note: The default RSA key size is %d bits." % CONFIG.RSA_KEY_SIZE) + sys.exit(1) # Save file le_util.make_or_verify_dir(CONFIG.KEY_DIR, 0o700) From aadf6da7bf16ea0f71da3370dabd66e230449ee0 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 14:21:59 +0100 Subject: [PATCH 5/7] show default RSA_KEY_SIZE in cmdline help --- letsencrypt/scripts/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index 5fc1a9bcb..1d7acda97 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -39,7 +39,7 @@ def main(): help="Revert configuration N number of checkpoints.") parser.add_argument("-B", "--keysize", dest="key_size", type=int, default=CONFIG.RSA_KEY_SIZE, metavar="N", - help="RSA key shall be sized N bits.") + help="RSA key shall be sized N bits. [%d]" % CONFIG.RSA_KEY_SIZE) parser.add_argument("-k", "--revoke", dest="revoke", action="store_true", help="Revoke a certificate.") parser.add_argument("-v", "--view-config-changes", From b72897677eb7e8d8a9cd11bed405b5720bf19403 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 23:20:18 +0100 Subject: [PATCH 6/7] use default value for -s argument --- letsencrypt/scripts/main.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/letsencrypt/scripts/main.py b/letsencrypt/scripts/main.py index c308b55da..9a83b6fcb 100755 --- a/letsencrypt/scripts/main.py +++ b/letsencrypt/scripts/main.py @@ -30,7 +30,8 @@ def main(): parser.add_argument("-d", "--domains", dest="domains", metavar="DOMAIN", nargs="+") parser.add_argument("-s", "--server", dest="server", - help="The ACME CA server [%s]." % CONFIG.ACME_SERVER) + default=CONFIG.ACME_SERVER, + help="The ACME CA server. [%(default)s]") parser.add_argument("-p", "--privkey", dest="privkey", type=read_file, help="Path to the private key file for certificate " "generation.") @@ -72,10 +73,9 @@ def main(): zope.component.provideUtility(displayer) installer = determine_installer() - server = CONFIG.ACME_SERVER if args.server is None else args.server if args.revoke: - revoc = revoker.Revoker(server, installer) + revoc = revoker.Revoker(args.server, installer) revoc.list_certs_keys() sys.exit() @@ -104,7 +104,7 @@ def main(): else: privkey = client.Client.Key(args.privkey[0], args.privkey[1]) - acme = client.Client(server, privkey, auth, installer) + acme = client.Client(args.server, privkey, auth, installer) # Validate the key and csr client.validate_key_csr(privkey) From 47e11a5824701593ab6c5df1dbdb1a61cd36fb49 Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Fri, 23 Jan 2015 23:39:02 +0100 Subject: [PATCH 7/7] use lazy logging --- letsencrypt/client/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/client/client.py b/letsencrypt/client/client.py index dd484cc31..763178d19 100644 --- a/letsencrypt/client/client.py +++ b/letsencrypt/client/client.py @@ -343,7 +343,7 @@ def init_key(key_size): key_pem = crypto_util.make_key(key_size) except ValueError as err: logging.fatal(str(err)) - logging.info("Note: The default RSA key size is %d bits." % CONFIG.RSA_KEY_SIZE) + logging.info("Note: The default RSA key size is %d bits.", CONFIG.RSA_KEY_SIZE) sys.exit(1) # Save file