Fixes#7713.
As discussed in #7713, providing a Powershell script as hook for Certbot is not working currently. This is because hooks are run in a `cmd` environment, that recognizes only `.bat` files as valid scripts that can be run from their bare name on command line.
On the other hand, the Powershell both `.bat` and `.ps1` scripts as valid scripts.
This PR makes hooks command be executed by Powershell, instead of `cmd` as `Popen` does by default when `shell=true` is used. It also modifies the tests to handle this new environment, in particular in term of encoding (UTF-16-LE is the default one in Powershell).
* Run hooks in powershell on Windows
* Fix hook test
* Fallback to unittest.mock
* In fact, shell_cmd as a list of str could not work. Declare only str as acceptable input for shell_cmd.
* Added changelog
Fixes#7268
I removed the reference to automatically selecting which ACME protocol we use, since at some point we'll want to rip out the non-spec-compliant ACMEv1 code.
Python 2 is going to get harder and harder to install locally so I don't think we should assume/require devs to have it installed.
This PR builds on #7905 so our developer guide only has people use Python 3.
Part of #7886.
This PR conditionally installs `mock` in `certbot-dns-*/setup.py` based on setuptools version and python version, when possible. It then updates the tests to use `unittest.mock` when `mock` isn't available.
* Do not require mock in Python 3 in certbot-dns modules
* update changelog
* error when trying to build wheels with old setuptools
* add type: ignores
This PR is exactly the same as #7895, but know we know a little bit more about what was going on with `mypy`.
Part of #7886.
This PR conditionally installs mock in `certbot/setup.py` based on setuptools version and python version, when possible. It then updates `certbot` tests to use `unittest.mock` when `mock` isn't available.
* Conditionally install mock in certbot
* use unittest.mock when third-party mock isn't available in certbot
* Add type:ignores because of https://github.com/python/mypy/issues/1153
* error out on newer python and older setuptools
* error when trying to build wheels with old setuptools
* Revert "Do not require mock in Python 3 in certbot module (#7895)"
This reverts commit 77871ba71c.
* Revert "Do not require mock in Python 3 in acme module (#7894)"
This reverts commit cd0acf5dcc.
Part of #7886.
This PR conditionally installs mock in `certbot/setup.py` based on setuptools version and python version, when possible. It then updates `certbot` tests to use `unittest.mock` when `mock` isn't available.
* Conditionally install mock in certbot
* use unittest.mock when third-party mock isn't available in certbot
* Add type:ignores because of https://github.com/python/mypy/issues/1153
* error when trying to build wheels with old setuptools
* Fix dangerous default argument
* Remove unused imports
* Remove unnecessary comprehension
* Use literal syntax to create data structure
* Use literal syntax instead of function calls to create data structure
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
Translate a proxy specified by an environment variable ("http_proxy"
or "HTTP_PROXY") into options recognized by "openssl ocsp". Support
is limited to HTTP proxies which don't require authentication.
Fixes#6150
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
Fixes#7594.
Removes the code asking interactively if the user would like to add a redirect.
* Remove interactive redirect ask
* display.enhancements is no longer used, so remove it.
* update changelog
* remove references to removed display.enhancements
* add redirect_default flag to enhance_config to conditionally set default for redirect value
* Update default in help text.
Fixes#7350.
This PR changes the parsed modules from a `set` to a `dict`, with the filepath argument as the value. Accordingly, after calling `enable_mod` to enable `ssl_module`, modules now need to be re-parsed, so call `reset_modules`.
* Add mechanism for selecting apache config file, based on work done in #7191.
* Check OpenSSL version
* Remove os imports
* debian override still needs os
* Reformat remaining apache tests with modules dict syntax
* Clean up more apache tests
* Switch from property to method for openssl and add tests for coverage.
* Sometimes the dict location will be None in which case we should in fact return None
* warn thoroughly and consistently in openssl_version function
* update tests for new warnings
* read file as bytes, and factor out the open for testing
* normalize ssl_module_location path to account for being relative to server root
* Use byte literals in a python 2 and 3 compatible way
* string does need to be a literal
* patch builtins open
* add debug, remove space
* Add test to check if OpenSSL detection is working on different systems
* fix relative test location for cwd
* put </IfModule> on its own line in test case
* Revert test file to status in master.
* Call augeas load before reparsing modules to pick up the changes
* fix grep, tail, and mod_ssl location on centos
* strip the trailing whitespace from fedora
* just use LooseVersion in test
* call apache2ctl on debian systems
* Use sudo for apache2ctl command
* add check to make sure we're getting a version
* Add boolean so we don't warn on debian/ubuntu before trying to enable mod_ssl
* Reduce warnings while testing by setting mock _openssl_version.
* Make sure we're not throwing away any unwritten changes to the config
* test last warning case for coverage
* text changes for clarity
This PR builds on #7657 and cleans up additional unnecessary pylint comments and some stray comments referring to pylint: disable comments that have been deleted that I didn't notice in my review of that PR.
* Remove stray pylint link.
* Cleanup more pylint comments
* Cleanup magic_typing imports
* Remove unneeded pylint: enable comments
This PR is an alternative to #7125.
Instead of disabling the strict mode on Pebble, this PR fixes the JWS payloads regarding RFC 8555 to be compliant, and allow certbot to work with Pebble v2.1.0+.
* Fix acme compliance to RFC 8555.
* Working mixin
* Activate back pebble strict mode
* Use mixin for type
* Update dependencies
* Fix also in fields_to_partial_json
* Update pebble
* Add changelog
This PR is the first part of work described in #6724.
It reintroduces the tls-alpn-01 challenge in `acme` module, that was introduced by #5894 and reverted by #6100. The reason it was removed in the past is because some tests showed that with `1.0.2` branch of OpenSSL, the self-signed certificate containing the authorization key is sent to the requester even if the ALPN protocol `acme-tls/1` was not declared as supported by the requester during the TLS handshake.
However recent discussions lead to the conclusion that this behavior was not a security issue, because first it is coherent with the behavior with servers that do not support ALPN at all, and second it cannot make a tls-alpn-01 challenge be validated in this kind of corner case.
On top of the original modifications given by #5894, I merged the code to be up-to-date with our `master`, and fixed tests to match recent evolution about not displaying the `keyAuthorization` in the deserialized JSON form of an ACME challenge.
I also move the logic to verify if ALPN is available on the current system, and so that the tls-alpn-01 challenge can be used, to a dedicated static function `is_available` in `acme.challenge.TLSALPN01`. This function is used in the related tests to skip them, and will be used in the future from Certbot plugins to trigger or not the logic related to tls-alpn-01, depending on the OpenSSL version available to Python.
* Reimplement TLS-ALPN-01 challenge and standalone TLS-ALPN server from #5894.
* Setup a class method to check if tls-alpn-01 is supported.
* Add potential missing parameter in validation for tls-alpn
* Improve comments
* Make a class private
* Handle old versions of openssl that do not terminate the handshake when they should do.
* Add changelog
* Explicitly close the TLS connection by the book.
* Remove unused exception
* Fix lint
Fixes#7835
I had to mock out `get_serial_from_cert` to keep a test from failing, because `cert_path` was mocked itself in `test_report_human_readable`.
Also, I kept the same style for the serial number as the recent Let's Encrypt e-mail: lowercase hexadecimal without a `0x` prefix and without colons every 2 chars. Shouldn't be a problem to change the format if required.
Fixes#5484
This PRs makes Certbot expose two new environment variables in the auth and cleanup hooks of the `manual` plugin:
* `CERTBOT_REMAINING_CHALLENGES` contains the number of challenges that remain after the current one (so it equals to 0 when the script is called for the last challenge)
* `CERTBOT_ALL_DOMAINS` contains a comma-separated list of all domains concerned by a challenge for the current certificate
With these variables, an hook script can know when it is run for the last time, and then trigger appropriate finalizers for all challenges that have been executed. This will be particularly useful for certificates with a lot of domains validated with DNS-01 challenges: instead of waiting on each hook execution to check that the relevant DNS TXT entry has been inserted, these waits can be avoided thanks to the latest hook verifying all domains in one run.
* Inject environment variables in manual scripts about remaining challenges
* Adapt tests
* Less variables and less lines
* Update manual.py
* Update manual_test.py
* Add documentation
* Add changelog
Certificates are public information by design: they are provided by
web servers without any prior authentication required. In a public
key cryptographic system, only the private key is secret information.
The private key file is already created as accessible only to the root
user with mode 0600, and these file permissions are set before any key
content is written to the file. There is no window within which an
attacker with access to the containing directory would be able to read
the private key content.
Older versions of Certbot (prior to 0.29.0) would create private key
files with mode 0644 and rely solely on the containing directory
permissions to restrict access. We therefore cannot (yet) set the
relevant default directory permissions to 0755, since it is possible
that a user could install Certbot, obtain a certificate, then
downgrade to a pre-0.29.0 version of Certbot, then obtain another
certificate. This chain of events would leave the second
certificate's private key file exposed.
As a compromise solution, document the fact that it is safe for the
common case of non-downgrading users to change the permissions of
/etc/letsencrypt/{live,archive} to 0755, and explain how to use chgrp
and chmod to make the private key file readable by a non-root service
user.
This provides guidance on the simplest way to solve the common problem
of making keys and certificates usable by services that run without
root privileges, with no requirement to create a custom (and hence
error-prone) executable hook.
Remove the existing custom executable hook example, so that the
documentation contains only the simplest and safest way to solve this
very common problem.
Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
After getting a +1 from everyone on the team, this PR removes the use of `codecov` from the Certbot repo because we keep having problems with it.
Two noteworthy things about this PR are:
1. I left the text at https://github.com/certbot/certbot/blob/4ea98d830bcc3d1b980a4055243c6a6a25d8dc54/.azure-pipelines/INSTALL.md#add-a-secret-variable-to-a-pipeline-like-codecov_token because I think it's useful to document how to set up a secret variable in general.
2. I'm not sure what the text "Option -e makes sure we fail fast and don't submit to codecov." in `tox.cover.py` refers to but it seems incorrect since `-e` isn't accepted or used by the script so I just deleted the line.
As part of this, I said I'd open an issue to track setting up coveralls (which seems to be the only real alternative to codecov) which is at https://github.com/certbot/certbot/issues/7810.
With my change, failure output looks something like:
```
$ tox -e py27-cover
...
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------------
certbot/certbot/__init__.py 1 0 100%
certbot/certbot/_internal/__init__.py 0 0 100%
certbot/certbot/_internal/account.py 191 4 98% 62-63, 206, 337
...
certbot/tests/storage_test.py 530 0 100%
certbot/tests/util_test.py 374 29 92% 211-213, 480-484, 489-499, 504-511, 545-547, 552-554
------------------------------------------------------------------------------------------
TOTAL 14451 647 96%
Command '['/path/to/certbot/dir/.tox/py27-cover/bin/python', '-m', 'coverage', 'report', '--fail-under', '100', '--include', 'certbot/*', '--show-missing']' returned non-zero exit status 2
Test coverage on certbot did not meet threshold of 100%.
ERROR: InvocationError for command /Users/bmw/Development/certbot/certbot/.tox/py27-cover/bin/python tox.cover.py (exited with code 1)
_________________________________________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________________________________________
ERROR: py27-cover: commands failed
```
I printed the exception just so we're not throwing away information.
I think it's also possible we fail for a reason other than the threshold not meeting the percentage, but I've personally never seen this, `coverage report` output is not being captured so hopefully that would inform devs if something else is going on, and saying something like "Test coverage probably did not..." seems like overkill to me personally.
* remove codecov
* remove unused variable group
* remove codecov.yml
* Improve tox.cover.py failure output.
Related to https://github.com/certbot/certbot/pull/7482, this removes some references to deprecated options in Certbot.
The only references I didn't remove were:
* In `certbot/tests/testdata/sample-renewal*` which contains a lot of old values and I think there's even some value in keeping them so we know if we make a change that suddenly causes old renewal configuration files to error.
* In the Apache and Nginx plugins and I created https://github.com/certbot/certbot/issues/7508 to resolve that issue.
If you run `mypy --platform darwin certbot/certbot/util.py` you'll get:
```
certbot/certbot/util.py:303: error: Name 'distro' is not defined
certbot/certbot/util.py:319: error: Name 'distro' is not defined
certbot/certbot/util.py:369: error: Name 'distro' is not defined
```
This is because mypy's logic for handling platform specific code is pretty simple and can't figure out what we're doing with `_USE_DISTRO` here. See https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks for more info.
Setting `_USE_DISTRO` to the result of `sys.platform.startswith('linux')` solves the problem without changing the overall behavior of our code here though.
This fixes part of https://github.com/certbot/certbot/issues/7803, but there's more work to be done on Windows.
