@osirisinferi noticed [in chat](https://opensource.eff.org/eff-open-source/pl/sa85u4n71tywfpc15c1wu59wae) that "NEXT STEPS:" was ignoring `--quiet` and was being printed unconditionally.
I think it ended up being written this way in #8860 because I was trying not to avoid dumping ANSI escapes and newlines into the log file and confused myself in the process.
This change makes things a bit more explicit in separating presentation/message.
* fix 'NEXT STEPS' being printed to stdout during -q
* fix tests
Issuing a certificate with --quiet was crashing during the donation
atexit call because it was trying to use the /dev/null fd after the
displayer context manager had already closed it.
Due to macOS having some complications about Certbot from Homebrew being
in the PATH, the instructions we have in the Automated Renewal section
do not work for them. Instead, send those users to the instruction
generator.
While the previous approach of testing the functionality of snapctl
worked, the snapd developers told us they could not guarantee its
reliability.
---
As with #8955, I tested this on Debian 9, 10 and CentOS 7, 8, Stream.
I want this for #8949.
I think this is quite verbose, but purposefully so as an intervention to try prevent users from hitting this problem. It's more of a "How-To Guide" than a "Reference Guide" (in the lingo of https://documentation.divio.com).
* docs: add "Deleting Certificates" to user guide
* try a less convoluted explanation
about what the installer did in the first place
* add a warning early on: read the full thing
* erica's copy changes
* rewrite as a how-to guide
* rewrite self-signed step 2 for mental model++
* rewrite intro to "safely deleting certificates"
`distro.linux_distribution` was deprecated (https://github.com/python-distro/distro/pull/296) in the release of `distro` at the end of last week. The deprecation is causing the `nopin` nightly tests to fail.
This change migrates Certbot off that function.
As far as I can tell, the Arch Linux edge case described in the code comments no longer happens, but better to be safe than sorry I think.
* stop using deprecated distro.linux_distribution
* update comment
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* acme: deprecate ACMEv1 client classes
Adds pending deprecations to:
- acme.client.Client
- acme.client.BackwardsCompatibleClientV2
Adds a warning to Certbot when a v1 server is detected.
* move thsi change from 1.17 to 1.18
* revert some whitespace changes
While bumping pinned packages in #8928, we came across a new version of pylint (2.9.3). Upgrading to this version requires some changes to Certbot's code, which is what this change is about.
* pylint: upgrade pinned verson and fix new lints
* maxsplit should be 1, not -1, for rsplit
* docs: explain the situation with --manual renewal
* note that the non-hook command can't be cronned
* add xref to #renewing-certificates
* update manual description in the plugins table
* redirect manual users towards other plugins
* refer to authentication hook scripts in table
In the apache2 package on Debian-based distros, the default
000-default.conf virtual host does not include a ServerName.
Depending on the FQDN hostname of the machine and DNS setup, Apache
assigns a name to this unnamed vhost at runtime. As a result, the
Apache config end up with vhosts that have duplicative names.
Previously, Certbot did not identify that the nameless vhost could be
a match for the requested identifier, which would, depending on
configuration load order, cause the authenticator to fail.
This change causes Certbot to include all unnamed vhosts on top of
matched vhosts, during authentication. If no vhosts matched, the
existing behavior remains the same.
* apache: configure nameless vhosts during auth
* vhost is only unnamed if ServerName is not set
* also fix test to only match ServerName
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* cli: vary renewal advice for hookless manual certs
1. Don't print that the certificate will be automatically renewed,
because it won't be.
2. Add a "NEXT STEP" telling the user that they will need to manually
re-issue the certificate in order to renew it.
* kill superfluous comma
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* clarify wording of the next step
* fix the test
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* later printing of renewal and install retry advice
Move printing of advice for automated renewal, and retrying installation
in case of failure, towards the end of `run` and `certonly`.
Also adds some renewal advice for the --csr case (no autorenewal).
* update renewal advice for preconfigured-renewal
* rewrite in terms of "NEXT STEPS" for run/certonly
* fix lint
* re-add "Could not install certificate"
* update --csr renewal advice
* rewrite non-preconfigured-renewal renewal advice
1.10.0 was a bad release and this breaks our oldest Boulder tests.
I bumped the version to 1.10.0 in #8852 to get access to a new public display_util API, but that was the release with the broken deprecation of `--manual-public-ip-logging-ok`. So let's bump it to 1.10.1.
* revoke: try determine the server automatically
When revoking via --cert-name, use the server from the lineage (unless
overriden by the CLI).
* RenewableCert.storage might be None
* guard against an empty lineage server
* nginx: authenticate all matching vhosts for HTTP01
Previously, the nginx authenticator would set up the HTTP-01 challenge
response on a single HTTP vhost which matched the challenge domain.
The nginx authenticator will now set the challenge response on every
vhost which matches the challenge domain, including duplicates and HTTPS
vhosts.
This makes the authenticator usable behind a CDN where all origin
traffic is performed over HTTPS and also makes the authenticator work
more reliably against "invalid" nginx configurations, such as those
where there are duplicate vhosts.
* some typos
* dont authenticate the same vhost twice
One vhost may appear in both the HTTP and HTTPS vhost lists. Use a set()
to avoid trying to mod the same vhost twice.
* fix type annotations
* rewrite changelog entry
There is some code in [`_paths_parser`](https://github.com/certbot/certbot/blob/ae3ed200c09f2d1913ec69c33a2604ae1fe32eb5/certbot/certbot/_internal/cli/paths_parser.py#L17-L34) which has the effect of varying the value type of `config.cert_path` and `config.key_path` based on the CLI verb. When the verb is `revoke`, the type is a tuple `(path: str, contents: bytes)`, otherwise it is a single `str` representing the file path. (I wasn't able to find a written reason as to why it works this way).
This commit removes that special `revoke` case and ensures it is always a `str`.
Why change it now?
I am trying to write some changes and there's some code in `cert_manager` which only works if the verb is `revoke`, you hack `config.cert_path` to be a tuple beforehand, or you [(not actually in `master`) try sniff for the value type](https://github.com/certbot/certbot/blob/49911afaa62ade1fca4c4ec407f817720cb354e3/certbot/certbot/_internal/cert_manager.py#L224-L227). I have a bad feeling about such workarounds. I would prefer to just make these variables simpler to use, but I'm open to opinions.
In addition to the test suites, I've manually tested `revoke` (including by `--key-path`) and `install`. Are there other places I may have missed?
Unblocks #8636 and #8671.
* docs: rewrite "Revoking certificates"
- `--cert-name` is supported since a long time ago
- `--delete-after-revoke` is default
- Mention that non-default `--server` must be specified
- Document difference between acme key/cert key revocation methods
- Reshuffle text to keep more important things earlier
* minor edits
* remove revocation note
* remove "preauthorization" revocation method
* rewrite deletion note
* test: certbot-ci crash due to no p521 on boulder
The bugfix in #8598 added an integration test to request a certificate
for an EC P-521 key, which is unsupported when ACME_SERVER=boulder,
failing our nightly integration tests.
* add an integration test for all EC curves
* update_account: print correct message for -m ""
When -m "" was passed on the CLI, Certbot would print that it updated
the email to '' (an empty string) rather than printing that it removed
the contact details.
This commit also refactors the update_account tests to be a bit more
modern.
* use addCleanup instead of tearDown in tests
In 96a05d9, mypy testing was added to certbot-ci, but introduced an
undeclared dependency on acme.magic_typing, resulting in a crash when
run under the integration-external tox environment.
This change uses the typing module in certbot-ci in place of
acme.magic_typing. It is already provided via dev_constraints.
Although Certbot is a classic snap, it shouldn't load Python code from
the host system. This change prevents packages being loaded from the
"user site-packages directory" (PEP-370). i.e. Certbot will no longer
load DNS plugins installed via `pip install --user certbot-dns-*`.
* nginx: fix py2 unicode sandwich
The nginx parser would crash when saving configuraitons containing
Unicode, because py2's `str` type does not support Unicode.
This change fixes that crash by ensuring that a string type supporting
Unicode is used in both Python 2 and Python 3.
* nginx: add unicode to the integration test config
* update CHANGELOG
* tests: add certbot-dns-rfc2136 integration tests
* dont use 'with' form of socket.socket
fixes py2 crash
* address some feedback:
- conftest: make DNS server a global resource
- conftest: add dns_xdist parameter into node config
- conftest: add --dns-server=bind flag
- conftest: if configured, point the ACME server to the DNS server
- dnsserver: make it sort-of compatible with xdist (future-proofing)
- context: parameterize dns-rfc2136 credentials file (future proofing)
- context: reduce dns-rfc2136 propagation time to speed up tests
- tox: add a integration-dns-rfc2136 target
- rfc2136: add a test/zone for subdelegation
- rfc2136: skip tests if no DNS server is configured
* try add integration-dns-rfc2136 to CI
* mock recursive dns via RPZ
* update --dns-server args and tox.ini args
* address more feedback:
- dns_server: rename rfc2136 creds file to .tpl
- dns_server: dont vary dns server port, instead we will vary zone names (#8455)
- dns_server: log error if bind9 fails to stop cleanly
- dns_server: replace assert with raise
- context: remove redundant _worker_id
- context: remove redundant cleanup override
- context: fix seek/flush in credentials context manager
- context: rename skip_if_no_server -> ...bind_server
- context: add newline EOF
* conftest: document _setup_primary_node sideeffects
* ci: rfc2136-integration from standard->nightly
* fix _stop_bind (function was renamed to stop)
* ignore errors from shutil.rmtree during cleanup
* dns_server: check for crash while polling
* remove --dry-run from rfc2136 test
* cli: improve Obtaining/Renewing wording
* dont use logger, and use new phrasing
* .display_util.notify: dont wrap
As this function is supposed to be an analogue for print, we do not want
it to wrap by default.
* tests: fix leaking patch in eff_test.py
* tests: PrintTest->NotifyTest in .display.util
The function was renamed during #8432. This change renames the test as
well.
* IDisplay.notification: add `decorate` param.
The flag allows the caller to control whether the message will be
printed in a decorated way (wrapped by hlines) or in an undecorated
way (similar to print).
It is set to true by default, to reflect the existing behavior of the
function.
* IDisplay.notification: write message to debug log
In the same vein as IReporter, this ensures that all notifications which
are shown to the user also make an appearance in the debug log, which
will aid in troubleshooting.
* restore accidentally deleted newline in decoration
* add helper function for printing status messages
* register: use notify rather than logger
Undoes the change in #8393 in favor of the new helper
* comment .display and ._internal.log
Describing when it is suitable to use each
* add more comments to log.py
* make IDisplay.notification decorate arg private
* rename notify->print and move to .display.util
* rename .display.print back to .display.notify
because linters complain about print being a redefined builtin
The ErrorHandler context manager could produce very verbose CLI output
when handling long exception chains (PIP 3134 enhanced reporting).
Rather than logging every exception with its traceback to the CLI, this
commit changes ErrorHandler so that only the final exception in the
chain, without traceback, is logged to the CLI.
This is consistent with a previous change made in the global except
hook (#8000).
* nginx: reduced CLI logging when reloading nginx
Hides the output of `nginx -s reload` from the CLI, moving it to
debug-level logging.
Additionally, fixes an issue where Certbot did not properly capture the
output of the nginx reload and restart commands.
Fixes#8231
* remove leftover debugging
* reorder CHANGELOG
* don't use bare asserts
random25863.example.org appears in multiple port 80 virtualhosts in the
nginx testdata tarball and also is in the nginx-roundtrip-testdata.
Certbot doesn't handle these properly, which results in random test
failures.
This commit ensures that random25863.example.org only appears in a
single virtualhost and should ensure that the tests pass consistently.
This commit fixes an issue with the nginx parser where it would perform
case-sensitive matching against server_name.
This would cause the authenticator and installer to ignore existing
virtualhosts containing uppercase characters, resulting in duplicate
virtualhosts and broken configurations.
"Exact" and "wildcard" matching is now case-insensitive. Regex-based
matching will continue to respect the case mode of the pattern.
Fixes#6776.
In order to avoid potentially breaking changes in the snap CLI on the
host, this commit changes certbot.wrapper to use the snap REST API (via
curl and jq) to list connected Certbot plugins.
* snap: Fix "stack smashing" error in wrapper
certbot.wrapper had implicit dependencies on sed, awk and coreutils,
which were being accidentally provided through the host system. Because
certbot.wrapper modifies LD_LIBRARY_PATH, this was causing some systems
to load an incompatible combination of shared libraries, resulting sed
crashing.
This commit reduces the dependencies of this script to just gawk, and
explicitly stages it as part of the Certbot snap.
It additionally moves invocations of all host system programs to a
moment prior to the modification of LD_LIBRARY_PATH, and the invocation
of snapped programs to after the modification.
Fixes#8245
* snap: Don't modify LD_LIBRARY_PATH
* leftover tracing
* snap: revert curl/jq in wrapper, use gawk for now
* nginx: add --nginx-sleep-seconds
As described in #7422, reloading nginx is an asynchronous process and
Certbot does not know when it is complete. In an environment where this
reload takes a long time, the nginx plugin suffers from an issue where
it responds to and fails the ACME challenge before the nginx server is
ready to serve it.
Following the discussion in a previous PR #7740, this commit introduces
a new flag, --nginx-sleep-seconds, which may be used to increase the
duration that Certbot will wait for nginx to reload, from its previously
hard-coded value of 1s.
Fixes#7422
* update CHANGELOG
* nginx: update docstring for nginx_restart
* acme: add support for alternative cert. chains
* certbot: add --preferred-chain
* remove support for issuer SKI matching
* show --preferred-chain in "run" help
* warn if no chain matched and it's not a dry-run
* fix existing failing tests
* add unit, integration tests
* bump acme dependency to dev version
* simplify test to avoid py2.7 recursion bug
* add preferred_chain to STR_CONFIG_ITEMS
* reduce preferred_chain warning to info level
* acme: fix some docstrings in .messages
* certbot: fix docstring in crypto_util
* try to fix certbot-nginx acme dep problem
In #7771, the Apache configurator gained the ability to identify what
version of OpenSSL Apache's ssl_module is linked against. However, the
detection was only functional if the module was built as a DSO (which is
almost always the case).
This commit covers the case where the ssl_module is statically linked
within the Apache binary. It requires the user to specify the path to
the binary (with --apache-bin) and emits a warning if static linking is
detected but no path has been provided.
Fixes#7875 .
After [this comment](https://github.com/certbot/certbot/issues/7875#issuecomment-608145208) and evaluating the options, I opted to go with `stricttextualmsg`, as required by RFC 8555. Reasoning is that the ACME v1 code path (via OpenSSL) produces a `fullchain_pem` which satisfies `stricttextualmsg`, so we don't need to be more generous than that.
One downside of the `re` approach is that it doesn't seem capable of capturing repeating group matches. As a result, it matches each certificate individually, silently passing over any data in between the encapsulation boundaries, such as explanatory text, which is prohibited by RFC 8555.
It would be ideal to raise an error when encountering such a non-conformant chain, but we'd need to create a mini-parser to do it, I think.
* Fix fullchain parsing for CRLF chains.
fullchain parsing now works in two passes:
1. A first pass which is generous with what it accepts - basically
preeb(CERTIFICATE)+anything+posteb(CERTIFICATE). This determines
the boundaries for each certificate.
2. A second pass which normalizes (by parsing and re-encoding) each
certificate found in the first pass.
* typo in docstring
* remove redundant group in regex
* can't use assertRaisesRegex until py27 is gone
* acme: socket timeout for HTTP standalone servers
Adds a default 30 second timeout to the StreamRequestHandler for clients
connecting to standalone HTTP-01 servers. This should prevent most cases
of an idle client connection from preventing the standalone server from
shutting down.
Fixes#7386
* use idiomatic kwargs default value
* move HTTP01Server lower to fix mypy forward ref.
* fix test crash on macOS due to socket double-close
* maybe its not an OSError?
* disable coverage check on useless branch
* acme: re-populate uri in deactivate_authorization
* Use fresh authorizations in dry runs
--dry-run now deactivates 'valid' authorizations if it encounters them
when creating a new order.
Resolves#5116.
* remove unused code
* typo in local-oldest-requirements
* better error handling
* certbot-ci: AUTHREUSE to 100 + unskip dry-run test
* improve test coverage for error cases
* restore newline to local-oldest-requirements.txt
The value of --server will now be respected, except when it is the
default value, in which case it will be changed to the staging server,
preserving Certbot's existing behavior.
