diff options
author | bmw <bmw@users.noreply.github.com> | 2015-11-14 04:45:53 +0300 |
---|---|---|
committer | bmw <bmw@users.noreply.github.com> | 2015-11-14 04:45:53 +0300 |
commit | 151c674cba6cd1e88521dbb5141d0c25662757b1 (patch) | |
tree | 96bbf8da23ab2ea74ff9bc2c242e5d79fdafd386 | |
parent | 2aab8782ae395d5224cf9d5c294008ff493d23bf (diff) | |
parent | d8a32eeeb50056bd48bddc0bc717ae70e3ad1fef (diff) |
Merge pull request #1455 from letsencrypt/useragent
Add a User Agent string for client analytics
-rw-r--r-- | letsencrypt/cli.py | 44 | ||||
-rw-r--r-- | letsencrypt/client.py | 42 | ||||
-rw-r--r-- | letsencrypt/le_util.py | 40 | ||||
-rw-r--r-- | letsencrypt/tests/cli_test.py | 95 | ||||
-rw-r--r-- | letsencrypt/tests/client_test.py | 4 |
5 files changed, 174 insertions, 51 deletions
diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 30ac81092..54da4fdcc 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -17,7 +17,6 @@ import zope.component import zope.interface.exceptions import zope.interface.verify -from acme import client as acme_client from acme import jose import letsencrypt @@ -39,7 +38,6 @@ from letsencrypt.display import util as display_util from letsencrypt.display import ops as display_ops from letsencrypt.plugins import disco as plugins_disco - logger = logging.getLogger(__name__) @@ -304,7 +302,7 @@ def _report_new_cert(cert_path, fullchain_path): reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY) -def _auth_from_domains(le_client, config, domains, plugins): +def _auth_from_domains(le_client, config, domains): """Authenticate and enroll certificate.""" # Note: This can raise errors... caught above us though. lineage = _treat_as_renewal(config, domains) @@ -325,7 +323,7 @@ def _auth_from_domains(le_client, config, domains, plugins): # configuration values from this attempt? <- Absolutely (jdkasten) else: # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate(domains, plugins) + lineage = le_client.obtain_and_enroll_certificate(domains) if not lineage: raise errors.Error("Certificate could not be obtained") @@ -425,14 +423,23 @@ def choose_configurator_plugins(args, config, plugins, verb): authenticator = display_ops.pick_authenticator(config, req_auth, plugins) logger.debug("Selected authenticator %s and installer %s", authenticator, installer) + # Report on any failures if need_inst and not installer: diagnose_configurator_problem("installer", req_inst, plugins) if need_auth and not authenticator: diagnose_configurator_problem("authenticator", req_auth, plugins) + record_chosen_plugins(config, plugins, authenticator, installer) return installer, authenticator +def record_chosen_plugins(config, plugins, auth, inst): + "Update the config entries to reflect the plugins we actually selected." + cn = config.namespace + cn.authenticator = plugins.find_init(auth).name if auth else "none" + cn.installer = plugins.find_init(inst).name if inst else "none" + + # TODO: Make run as close to auth + install as possible # Possible difficulties: args.csr was hacked into auth def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals @@ -447,7 +454,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) - lineage = _auth_from_domains(le_client, config, domains, plugins) + lineage = _auth_from_domains(le_client, config, domains) le_client.deploy_certificate( domains, lineage.privkey, lineage.cert, @@ -461,7 +468,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo display_ops.success_renewal(domains) -def obtaincert(args, config, plugins): +def obtain_cert(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" if args.domains is not None and args.csr is not None: @@ -487,7 +494,7 @@ def obtaincert(args, config, plugins): _report_new_cert(cert_path, cert_fullchain) else: domains = _find_domains(args, installer) - _auth_from_domains(le_client, config, domains, plugins) + _auth_from_domains(le_client, config, domains) def install(args, config, plugins): @@ -512,18 +519,19 @@ def install(args, config, plugins): def revoke(args, config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" + # For user-agent construction + config.namespace.installer = config.namespace.authenticator = "none" if args.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", args.cert_path[0], args.key_path[0]) - acme = acme_client.Client( - config.server, key=jose.JWK.load(args.key_path[1])) + key = jose.JWK.load(args.key_path[1]) else: # revocation by account key logger.debug("Revoking %s using Account Key", args.cert_path[0]) acc, _ = _determine_account(args, config) - # pylint: disable=protected-access - acme = client._acme_from_config_key(config, acc.key) - acme.revoke(jose.ComparableX509(crypto_util.pyopenssl_load_certificate( - args.cert_path[1])[0])) + key = acc.key + acme = client.acme_from_config_key(config, key) + cert = crypto_util.pyopenssl_load_certificate(args.cert_path[1])[0] + acme.revoke(jose.ComparableX509(cert)) def rollback(args, config, plugins): @@ -625,7 +633,7 @@ class HelpfulArgumentParser(object): """ # Maps verbs/subcommands to the functions that implement them - VERBS = {"auth": obtaincert, "certonly": obtaincert, + VERBS = {"auth": obtain_cert, "certonly": obtain_cert, "config_changes": config_changes, "everything": run, "install": install, "plugins": plugins_cmd, "revoke": revoke, "rollback": rollback, "run": run} @@ -921,7 +929,13 @@ def _create_subparsers(helpful): helpful.add_group("revoke", description="Options for revocation of certs") helpful.add_group("rollback", description="Options for reverting config changes") helpful.add_group("plugins", description="Plugin options") - + helpful.add( + None, "--user-agent", default=None, + help="Set a custom user agent string for the client. User agent strings allow " + "the CA to collect high level statistics about success rates by OS and " + "plugin. If you wish to hide your server OS version from the Let's " + 'Encrypt server, set this to "".' + ) helpful.add("certonly", "--csr", type=read_file, help="Path to a Certificate Signing Request (CSR) in DER" diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 0f0ecdc6d..8e053e926 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -11,6 +11,8 @@ from acme import client as acme_client from acme import jose from acme import messages +import letsencrypt + from letsencrypt import account from letsencrypt import auth_handler from letsencrypt import configuration @@ -31,10 +33,30 @@ from letsencrypt.display import enhancements logger = logging.getLogger(__name__) -def _acme_from_config_key(config, key): +def acme_from_config_key(config, key): + "Wrangle ACME client construction" # TODO: Allow for other alg types besides RS256 - return acme_client.Client(directory=config.server, key=key, - verify_ssl=(not config.no_verify_ssl)) + net = acme_client.ClientNetwork(key, verify_ssl=(not config.no_verify_ssl), + user_agent=_determine_user_agent(config)) + return acme_client.Client(config.server, key=key, net=net) + + +def _determine_user_agent(config): + """ + Set a user_agent string in the config based on the choice of plugins. + (this wasn't knowable at construction time) + + :returns: the client's User-Agent string + :rtype: `str` + """ + + if config.user_agent is None: + ua = "LetsEncryptPythonClient/{0} ({1}) Authenticator/{2} Installer/{3}" + ua = ua.format(letsencrypt.__version__, " ".join(le_util.get_os_info()), + config.authenticator, config.installer) + else: + ua = config.user_agent + return ua def register(config, account_storage, tos_cb=None): @@ -86,7 +108,7 @@ def register(config, account_storage, tos_cb=None): public_exponent=65537, key_size=config.rsa_key_size, backend=default_backend()))) - acme = _acme_from_config_key(config, key) + acme = acme_from_config_key(config, key) # TODO: add phone? regr = acme.register(messages.NewRegistration.from_data(email=config.email)) @@ -100,6 +122,7 @@ def register(config, account_storage, tos_cb=None): acc = account.Account(regr, key) account.report_new_account(acc, config) account_storage.save(acc) + return acc, acme @@ -128,7 +151,7 @@ class Client(object): # Initialize ACME if account is provided if acme is None and self.account is not None: - acme = _acme_from_config_key(config, self.account.key) + acme = acme_from_config_key(config, self.account.key) self.acme = acme # TODO: Check if self.config.enroll_autorenew is None. If @@ -213,7 +236,7 @@ class Client(object): return self._obtain_certificate(domains, csr) + (key, csr) - def obtain_and_enroll_certificate(self, domains, plugins): + def obtain_and_enroll_certificate(self, domains): """Obtain and enroll certificate. Get a new certificate for the specified domains using the specified @@ -230,13 +253,6 @@ class Client(object): """ certr, chain, key, _ = self.obtain_certificate(domains) - # TODO: remove this dirty hack - self.config.namespace.authenticator = plugins.find_init( - self.dv_auth).name - if self.installer is not None: - self.config.namespace.installer = plugins.find_init( - self.installer).name - # XXX: We clearly need a more general and correct way of getting # options into the configobj for the RenewableCert instance. # This is a quick-and-dirty way to do it to allow integration diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 5626902ef..25260d755 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -3,6 +3,7 @@ import collections import errno import logging import os +import platform import re import subprocess import stat @@ -202,6 +203,45 @@ def safely_remove(path): raise +def get_os_info(): + """ + Get Operating System type/distribution and major version + + :returns: (os_name, os_version) + :rtype: `tuple` of `str` + """ + info = platform.system_alias( + platform.system(), + platform.release(), + platform.version() + ) + os_type, os_ver, _ = info + os_type = os_type.lower() + if os_type.startswith('linux'): + info = platform.linux_distribution() + # On arch, platform.linux_distribution() is reportedly ('','',''), + # so handle it defensively + if info[0]: + os_type = info[0] + if info[1]: + os_ver = info[1] + elif os_type.startswith('darwin'): + os_ver = subprocess.Popen( + ["sw_vers", "-productVersion"], + stdout=subprocess.PIPE + ).communicate()[0] + os_ver = os_ver.partition(".")[0] + elif os_type.startswith('freebsd'): + # eg "9.3-RC3-p1" + os_ver = os_ver.partition("-")[0] + os_ver = os_ver.partition(".")[0] + elif platform.win32_ver()[1]: + os_ver = platform.win32_ver()[1] + else: + # Cases known to fall here: Cygwin python + os_ver = '' + return os_type, os_ver + # Just make sure we don't get pwned... Make sure that it also doesn't # start with a period or have two consecutive periods <- this needs to # be done in addition to the regex diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 811d11da8..500ff074e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -10,9 +10,14 @@ import unittest import mock +from acme import jose + from letsencrypt import account +from letsencrypt import cli from letsencrypt import configuration +from letsencrypt import crypto_util from letsencrypt import errors +from letsencrypt import le_util from letsencrypt.plugins import disco @@ -20,7 +25,9 @@ from letsencrypt.tests import renewer_test from letsencrypt.tests import test_util +CERT = test_util.vector_path('cert.pem') CSR = test_util.vector_path('csr.der') +KEY = test_util.vector_path('rsa256_key.pem') class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods @@ -31,33 +38,36 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.config_dir = os.path.join(self.tmp_dir, 'config') self.work_dir = os.path.join(self.tmp_dir, 'work') self.logs_dir = os.path.join(self.tmp_dir, 'logs') + self.standard_args = ['--text', '--config-dir', self.config_dir, + '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, + '--agree-dev-preview'] def tearDown(self): shutil.rmtree(self.tmp_dir) def _call(self, args): - from letsencrypt import cli - args = ['--text', '--config-dir', self.config_dir, - '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, - '--agree-dev-preview'] + args + "Run the cli with output streams and actual client mocked out" + with mock.patch('letsencrypt.cli.client') as client: + ret, stdout, stderr = self._call_no_clientmock(args) + return ret, stdout, stderr, client + + def _call_no_clientmock(self, args): + "Run the client with output streams mocked out" + args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stdout') as stdout: with mock.patch('letsencrypt.cli.sys.stderr') as stderr: - with mock.patch('letsencrypt.cli.client') as client: - ret = cli.main(args) - return ret, stdout, stderr, client + ret = cli.main(args[:]) # NOTE: parser can alter its args! + return ret, stdout, stderr def _call_stdout(self, args): """ Variant of _call that preserves stdout so that it can be mocked by the caller. """ - from letsencrypt import cli - args = ['--text', '--config-dir', self.config_dir, - '--work-dir', self.work_dir, '--logs-dir', self.logs_dir, - '--agree-dev-preview'] + args + args = self.standard_args + args with mock.patch('letsencrypt.cli.sys.stderr') as stderr: with mock.patch('letsencrypt.cli.client') as client: - ret = cli.main(args) + ret = cli.main(args[:]) # NOTE: parser can alter its args! return ret, None, stderr, client def test_no_flags(self): @@ -113,9 +123,34 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue("--key-path" not in out) out = self._help_output(['-h']) - from letsencrypt import cli self.assertTrue(cli.usage_strings(plugins)[0] in out) + @mock.patch('letsencrypt.cli.client.acme_client.Client') + @mock.patch('letsencrypt.cli._determine_account') + @mock.patch('letsencrypt.cli.client.Client.obtain_and_enroll_certificate') + @mock.patch('letsencrypt.cli._auth_from_domains') + def test_user_agent(self, _afd, _obt, det, _client): + # Normally the client is totally mocked out, but here we need more + # arguments to automate it... + args = ["--standalone", "certonly", "-m", "none@none.com", + "-d", "example.com", '--agree-tos'] + self.standard_args + det.return_value = mock.MagicMock(), None + with mock.patch('letsencrypt.cli.client.acme_client.ClientNetwork') as acme_net: + self._call_no_clientmock(args) + os_ver = " ".join(le_util.get_os_info()) + ua = acme_net.call_args[1]["user_agent"] + self.assertTrue(os_ver in ua) + import platform + plat = platform.platform() + if "linux" in plat.lower(): + self.assertTrue(platform.linux_distribution()[0] in ua) + + with mock.patch('letsencrypt.cli.client.acme_client.ClientNetwork') as acme_net: + ua = "bandersnatch" + args += ["--user-agent", ua] + self._call_no_clientmock(args) + acme_net.assert_called_once_with(mock.ANY, verify_ssl=True, user_agent=ua) + def test_install_abspath(self): cert = 'cert' key = 'key' @@ -133,8 +168,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertEqual(args.chain_path, os.path.abspath(chain)) self.assertEqual(args.fullchain_path, os.path.abspath(fullchain)) + @mock.patch('letsencrypt.cli.record_chosen_plugins') @mock.patch('letsencrypt.cli.display_ops') - def test_installer_selection(self, mock_display_ops): + def test_installer_selection(self, mock_display_ops, _rec): self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert', '--key-path', 'key', '--chain-path', 'chain']) self.assertEqual(mock_display_ops.pick_installer.call_count, 1) @@ -273,7 +309,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods ['-d', '*.wildcard.tld']) def test_parse_domains(self): - from letsencrypt import cli plugins = disco.PluginsRegistry.find_all() short_args = ['-d', 'example.com'] @@ -353,7 +388,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods @mock.patch('letsencrypt.cli.display_ops.pick_installer') @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._init_le_client') - def test_certonly_csr(self, mock_init, mock_get_utility, + @mock.patch('letsencrypt.cli.record_chosen_plugins') + def test_certonly_csr(self, _rec, mock_init, mock_get_utility, mock_pick_installer, mock_notAfter): cert_path = '/etc/letsencrypt/live/blahcert.pem' date = '1970-01-01' @@ -378,11 +414,31 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue( date in mock_get_utility().add_message.call_args[0][0]) + @mock.patch('letsencrypt.cli.client.acme_client') + def test_revoke_with_key(self, mock_acme_client): + server = 'foo.bar' + self._call_no_clientmock(['--cert-path', CERT, '--key-path', KEY, + '--server', server, 'revoke']) + with open(KEY) as f: + mock_acme_client.Client.assert_called_once_with( + server, key=jose.JWK.load(f.read()), net=mock.ANY) + with open(CERT) as f: + cert = crypto_util.pyopenssl_load_certificate(f.read())[0] + mock_revoke = mock_acme_client.Client().revoke + mock_revoke.assert_called_once_with(jose.ComparableX509(cert)) + + @mock.patch('letsencrypt.cli._determine_account') + def test_revoke_without_key(self, mock_determine_account): + mock_determine_account.return_value = (mock.MagicMock(), None) + _, _, _, client = self._call(['--cert-path', CERT, 'revoke']) + with open(CERT) as f: + cert = crypto_util.pyopenssl_load_certificate(f.read())[0] + mock_revoke = client.acme_from_config_key().revoke + mock_revoke.assert_called_once_with(jose.ComparableX509(cert)) + @mock.patch('letsencrypt.cli.sys') def test_handle_exception(self, mock_sys): # pylint: disable=protected-access - from letsencrypt import cli - mock_open = mock.mock_open() with mock.patch('letsencrypt.cli.open', mock_open, create=True): exception = Exception('detail') @@ -415,7 +471,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods traceback.format_exception_only(KeyboardInterrupt, interrupt))) def test_read_file(self): - from letsencrypt import cli rel_test_path = os.path.relpath(os.path.join(self.tmp_dir, 'foo')) self.assertRaises( argparse.ArgumentTypeError, cli.read_file, rel_test_path) @@ -547,8 +602,6 @@ class MockedVerb(object): """ def __init__(self, verb_name): - from letsencrypt import cli - self.verb_dict = cli.HelpfulArgumentParser.VERBS self.verb_func = None self.verb_name = verb_name diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index f8da90f36..d396e25bc 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -70,8 +70,8 @@ class ClientTest(unittest.TestCase): dv_auth=None, installer=None) def test_init_acme_verify_ssl(self): - self.acme_client.assert_called_once_with( - directory=mock.ANY, key=mock.ANY, verify_ssl=True) + net = self.acme_client.call_args[1]["net"] + self.assertTrue(net.verify_ssl) def _mock_obtain_certificate(self): self.client.auth_handler = mock.MagicMock() |