Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement the --cert-name flag to select a lineage by its name. #3785

Merged
merged 26 commits into from Dec 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ce9305f
Rename and simplify main functions
ohemorange Nov 9, 2016
995eae8
pass certname to auth method
ohemorange Nov 9, 2016
c6853e1
Merge branch 'master' into certname-flag
ohemorange Nov 9, 2016
52206a9
find cert by certname flag
ohemorange Nov 11, 2016
c23d31c
Implement --cert-name command
ohemorange Nov 11, 2016
22642bc
more coverage
ohemorange Nov 12, 2016
5426721
Merge branch 'master' into certname-flag
ohemorange Nov 22, 2016
ebbeaf5
update based on change to master
ohemorange Nov 22, 2016
60bb060
add test coverage for new main methods (unlinted)
ohemorange Nov 29, 2016
e395f7a
don't ask to confirm new cert when we have domains and no existing ce…
ohemorange Nov 29, 2016
da6a477
Refactor and add --new-cert-name flag
ohemorange Nov 29, 2016
a63004a
Merge branch 'master' into certname-flag
ohemorange Nov 29, 2016
da05230
Merge branch 'master' into certname-flag
ohemorange Nov 30, 2016
709a8c6
add interactivity to rename verb
ohemorange Dec 1, 2016
fec660a
fix updated interactive rename command
ohemorange Dec 1, 2016
4374876
Update docs, cli flags, and error messages
ohemorange Dec 1, 2016
78d9cf8
fix no certname test
ohemorange Dec 1, 2016
4675186
use the handy dandy BaseCertManagerTest
ohemorange Dec 1, 2016
2c87cbb
xrange --> range for py3
ohemorange Dec 2, 2016
33c2a23
Merge branch 'master' into certname-flag
ohemorange Dec 6, 2016
e6e3261
Merge branch 'certname-flag' of https://github.com/certbot/certbot in…
ohemorange Dec 6, 2016
2bfddcb
Merge branch 'master' into certname-flag
ohemorange Dec 6, 2016
4562d78
allow noninteractive and more descriptive function names
ohemorange Dec 6, 2016
f56a11d
oneline some doctrings
ohemorange Dec 6, 2016
ab01927
unit test coverage
ohemorange Dec 7, 2016
41aecf5
Merge branch 'master' into certname-flag
ohemorange Dec 8, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 109 additions & 0 deletions certbot/cert_manager.py
@@ -1,14 +1,19 @@
"""Tools for managing certificates."""
import datetime
import logging
import os
import pytz
import traceback
import zope.component

from certbot import configuration
from certbot import errors
from certbot import interfaces
from certbot import renewal
from certbot import storage
from certbot import util

from certbot.display import util as display_util

logger = logging.getLogger(__name__)

Expand All @@ -30,6 +35,43 @@ def update_live_symlinks(config):
configuration.RenewerConfiguration(renewer_config),
update_symlinks=True)

def rename_lineage(config):
"""Rename the specified lineage to the new name.

:param config: Configuration.
:type config: :class:`certbot.interfaces.IConfig`

"""
disp = zope.component.getUtility(interfaces.IDisplay)
renewer_config = configuration.RenewerConfiguration(config)

certname = config.certname
if not certname:
filenames = renewal.renewal_conf_files(renewer_config)
choices = [storage.lineagename_for_filename(name) for name in filenames]
if not choices:
raise errors.Error("No existing certificates found.")
code, index = disp.menu("Which certificate would you like to rename?",
choices, ok_label="Select", flag="--cert-name")
if code != display_util.OK or not index in range(0, len(choices)):
raise errors.Error("User ended interaction.")
certname = choices[index]

new_certname = config.new_certname
if not new_certname:
code, new_certname = disp.input("Enter the new name for certificate {0}"
.format(certname), flag="--updated-cert-name")
if code != display_util.OK or not new_certname:
raise errors.Error("User ended interaction.")

lineage = lineage_for_certname(config, certname)
if not lineage:
raise errors.ConfigurationError("No existing certificate with name "
"{0} found.".format(certname))
storage.rename_renewal_config(certname, new_certname, renewer_config)
disp.notification("Successfully renamed {0} to {1}."
.format(certname, new_certname), pause=False)

def _report_lines(msgs):
"""Format a results report for a category of single-line renewal outcomes"""
return " " + "\n ".join(str(msg) for msg in msgs)
Expand Down Expand Up @@ -106,3 +148,70 @@ def certificates(config):

# Describe all the certs
_describe_certs(parsed_certs, parse_failures)

def _search_lineages(config, func, initial_rv):
"""Iterate func over unbroken lineages, allowing custom return conditions.

Allows flexible customization of return values, including multiple
return values and complex checks.
"""
cli_config = configuration.RenewerConfiguration(config)
configs_dir = cli_config.renewal_configs_dir
# Verify the directory is there
util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())

rv = initial_rv
for renewal_file in renewal.renewal_conf_files(cli_config):
try:
candidate_lineage = storage.RenewableCert(renewal_file, cli_config)
except (errors.CertStorageError, IOError):
logger.debug("Renewal conf file %s is broken. Skipping.", renewal_file)
logger.debug("Traceback was:\n%s", traceback.format_exc())
continue
rv = func(candidate_lineage, rv)
return rv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally wondering about the case where there's more than one matching lineage and whether we'd simply get func() of the last one, but now I see that since the intermediate rv is an argument to func, someone writing a func could also choose to have it accumulate values as it goes, like in a list, if they want information about multiple lineages. So, nice, flexible design there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flexibility is my goal! I'll put this in the docstring to make it clear.


def lineage_for_certname(config, certname):
"""Find a lineage object with name certname."""
def update_cert_for_name_match(candidate_lineage, rv):
"""Return cert if it has name certname, else return rv
"""
matching_lineage_name_cert = rv
if candidate_lineage.lineagename == certname:
matching_lineage_name_cert = candidate_lineage
return matching_lineage_name_cert
return _search_lineages(config, update_cert_for_name_match, None)

def domains_for_certname(config, certname):
"""Find the domains in the cert with name certname."""
def update_domains_for_name_match(candidate_lineage, rv):
"""Return domains if certname matches, else return rv
"""
matching_domains = rv
if candidate_lineage.lineagename == certname:
matching_domains = candidate_lineage.names()
return matching_domains
return _search_lineages(config, update_domains_for_name_match, None)

def find_duplicative_certs(config, domains):
"""Find existing certs that duplicate the request."""
def update_certs_for_domain_matches(candidate_lineage, rv):
"""Return cert as identical_names_cert if it matches,
or subset_names_cert if it matches as subset
"""
# TODO: Handle these differently depending on whether they are
# expired or still valid?
identical_names_cert, subset_names_cert = rv
candidate_names = set(candidate_lineage.names())
if candidate_names == set(domains):
identical_names_cert = candidate_lineage
elif candidate_names.issubset(set(domains)):
# This logic finds and returns the largest subset-names cert
# in the case where there are several available.
if subset_names_cert is None:
subset_names_cert = candidate_lineage
elif len(candidate_names) > len(subset_names_cert.names()):
subset_names_cert = candidate_lineage
return (identical_names_cert, subset_names_cert)

return _search_lineages(config, update_certs_for_domain_matches, (None, None))
23 changes: 21 additions & 2 deletions certbot/cli.py
Expand Up @@ -68,6 +68,7 @@
rollback Rollback server configuration changes made during install
config_changes Show changes made to server config during installation
update_symlinks Update cert symlinks based on renewal config file
rename Update a certificate's name
plugins Display information about installed plugins
certificates Display information about certs configured with Certbot

Expand Down Expand Up @@ -326,7 +327,7 @@ def __init__(self, args, plugins, detect_defaults=False):
"register": main.register, "renew": main.renew,
"revoke": main.revoke, "rollback": main.rollback,
"everything": main.run, "update_symlinks": main.update_symlinks,
"certificates": main.certificates}
"certificates": main.certificates, "rename": main.rename}

# List of topics for which additional help can be provided
HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS)
Expand Down Expand Up @@ -686,6 +687,19 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
help="Domain names to apply. For multiple domains you can use "
"multiple -d flags or enter a comma separated list of domains "
"as a parameter.")
helpful.add(
[None, "run", "certonly"],
"--cert-name", dest="certname",
metavar="CERTNAME", default=None,
help="Certificate name to apply. Only one certificate name can be used "
"per Certbot run. To see certificate names, run 'certbot certificates'."
"If there is no existing certificate with this name and "
"domains are requested, create a new certificate with this name.")
helpful.add(
"rename",
"--updated-cert-name", dest="new_certname",
metavar="NEW_CERTNAME", default=None,
help="New name for the certificate. Must be a valid filename.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it valid to use only --new-cert-name with a new lineage, instead of --cert-name? (Are they synonyms when there is no existing lineage with the specified name?) I can definitely imagine some users mentally parsing this as (new-cert)-name instead of new-(cert-name) and so trying to use it in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh good point. Maybe --updated-cert-name is better? Or just --new-certname and then also call it --certname in other places? I don't want to make this flag for anything other than setting a new cert name.

helpful.add(
[None, "testing", "renew", "certonly"],
"--dry-run", action="store_true", dest="dry_run",
Expand Down Expand Up @@ -738,6 +752,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis
"regardless of whether it is near expiry. (Often "
"--keep-until-expiring is more appropriate). Also implies "
"--expand.")
helpful.add(
"automation", "--renew-with-new-domains",
action="store_true", dest="renew_with_new_domains", help="If a "
"certificate already exists for the requested certificate name "
"but does not match the requested domains, renew it now, "
"regardless of whether it is near expiry.")
helpful.add(
["automation", "renew", "certonly"],
"--allow-subset-of-names", action="store_true",
Expand Down Expand Up @@ -1015,7 +1035,6 @@ def __call__(self, parser, namespace, domain, option_string=None):
"""Just wrap add_domains in argparseese."""
add_domains(namespace, domain)


def add_domains(args_or_config, domains):
"""Registers new domains to be used during the current client run.

Expand Down
8 changes: 5 additions & 3 deletions certbot/client.py
Expand Up @@ -263,7 +263,7 @@ def obtain_certificate(self, domains):
return (self.obtain_certificate_from_csr(domains, csr, authzr=authzr)
+ (key, csr))

def obtain_and_enroll_certificate(self, domains):
def obtain_and_enroll_certificate(self, domains, certname):
"""Obtain and enroll certificate.

Get a new certificate for the specified domains using the specified
Expand All @@ -272,6 +272,7 @@ def obtain_and_enroll_certificate(self, domains):

:param list domains: Domains to request.
:param plugins: A PluginsFactory object.
:param str certname: Name of new cert

:returns: A new :class:`certbot.storage.RenewableCert` instance
referred to the enrolled cert lineage, False if the cert could not
Expand All @@ -286,13 +287,14 @@ def obtain_and_enroll_certificate(self, domains):
"Non-standard path(s), might not work with crontab installed "
"by your operating system package manager")

new_name = certname if certname else domains[0]
if self.config.dry_run:
logger.debug("Dry run: Skipping creating new lineage for %s",
domains[0])
new_name)
return None
else:
return storage.RenewableCert.new_lineage(
domains[0], OpenSSL.crypto.dump_certificate(
new_name, OpenSSL.crypto.dump_certificate(
OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped),
key.pem, crypto_util.dump_pyopenssl_chain(chain),
configuration.RenewerConfiguration(self.config.namespace))
Expand Down