Fixes#7031
I use the same approach than in `CreateVenv()` and `CompareVersions()`: a new bash function `CheckPathPermissions()` is declared an execute a python script passed to the interpreter through stdin.
This allows:
* to not require the temp_dir that holds a temporary script to be executed
* to reduce at the bare minimum the change to make on the order of bash command to execute (including when the temp_dir is created)
* Fix check permissions logic in certbot-auto by making a temp dir useless
* Update CHANGELOG.md
This PR attempts to better inform people about the problem identified at https://community.letsencrypt.org/t/certbot-auto-deployment-best-practices/91979/.
I was hesitant to add the flag --no-permissions-check, however, if there's some obscure distro out there (or custom user setup) that has a strange users and groups, I didn't want us to either:
Have to put out a bug fix release
Refuse to fix the problem and let them deal with warnings on every run
* add check_permissions.py
* Update letsencrypt-auto.template.
* build letsencrypt-auto
* Add test_permissions_warnings to auto_test
* Allow uid/gid < 1000.
* Add --no-permissions-check to Certbot.
* Add --no-permissions-check to certbot-auto.
* Add test farm test that letsencrypt-auto is quiet.
As a bonus, this new test will catch problems like the one that the caused
0.33.1 point release.
* Update CHANGELOG about permissions check.
* Update permissions comment.
* Fix symlink handling.
* Use a better default in auto_test.py.
First step of #6960.
* Warn install users that future versions of certbot will automatically redirect
* Only warn when the user declines or auto-declines redirect
* Unit tests
* Update changelog
* certbot-dns-linode : Added support for linode version 4 tokens
* certbot-dns-linode : Added credentials ini option to override automatic api version detection
* certbot-dns-linode : Added clearer messages and documentation based on review
* certbot-dns-linode : Added check for empty 'linode_version' config instead of missing
* certbot-dns-linode : Fix rebase on master
* certbot-dns-linode : Updated local-oldest-requirements.txt
* Updated CHANGELOG to indicate Linode v4 API key support
Fixes#6974.
This PR removes the fallback that consists in retrying to send the keyAuthorization field during a challenge request in case of malformed request.
* Remove keyAuthorization fallback dump in challenges response
* Correct import
* Add changelog entry
This PR is the second part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)
This PR creates the module certbot.compat.os, that delegates everything to os, and that will be the safeguard against problematic methods of the standard module. On top of that, a quality check wrapper is called in the lint tox environment. This wrapper calls pylint and ensures that standard os module is no used directly in the certbot codebase.
Finally local oldest requirements are updated to ensure that tests will take the new logic when running.
* Add executable permissions
* Add the delegate certbot.compat.os module, add check coding style to enforce usage of certbot.compat.os instead of standard os
* Load certbot.compat.os instead of os
* Move existing compat test
* Update local oldest requirements
* Import sys
* Update account_test.py
* Update os.py
* Update os.py
* Update local oldest requirements
* Implement the new linter_plugin
* Fix local oldest for nginx
* Remove check coding style
* Update linter_plugin.py
* Add several comments
* Update the setup.py
* Add documentation
* Update acme dependencies
* Update certbot/compat/os.py
* Update docs/contributing.rst
* Update linter_plugin.py
* Handle os.path. Simplify checker.
* Add a comment to a reference implementation
* Update changelog
* Fix module registering
* Update docs/contributing.rst
* Update config and changelog
This PR adds a step to Apache plugin config_test when run on Fedora. Because Fedora now creates self signed certificate and related key material upon first startup of httpd. This was causing issues for users who run certbot-auto or install certbot (and mod_ssl) and run Certbot directly after.
Fixes: #6828
* Try to restart httpd on Fedora if config check fails
* Update CHANGELOG.md
The changelog should still say <version> - master because it will be fixed up automatically by the release script at https://github.com/certbot/certbot/blob/master/tools/_release.sh#L69.
* Protect certbot-auto against non numerical version release in some RPM distributions (#6913)
Fixes#6912
Bash evaluate all condition in a predicate statement, eg. `"$SOMEVAR" = "test" -a "$ANOTHERVAR" = "test2"`, even if it is not necessary, for instance if the first condition is false in the example here.
As a consequence, on non-Fedora distributions, an evaluation of the distribution version could be done on non numeric value, eg. `"6.7" -eq "29"`, making certbot-auto failing in this case.
This PR fixes that, by evaluating the version on RPM distributions only if we are on Fedora. Otherwise, version will be "0".
(cherry picked from commit c2d9ea1f61)
* Update changelog about #6912 fix. (#6914)
(cherry picked from commit 30eafba997)
* cleanup changelog
In CentOS 6 default httpd configuration, the `LoadModule ssl_module ...` is handled in `conf.d/ssl.conf`. As the `VirtualHost` configuration files in `conf.d/` are loaded in alphabetical order, this means that all files that have `<IfModule mod_ssl.c>` and are loaded before `ssl.conf` are effectively ignored. This PR moves the `LoadModule ssl_module` to the main `httpd.conf` while leaving a conditional `LoadModule` directive in `ssl.conf`.
Features
- Reads the module configuration from `ssl.conf` in case some modifications to paths have been made by the user.
- Falls back to default paths if the directive doesn't exist.
- Moves the `LoadModule` directive in `ssl.conf` inside `<IfModule !mod_ssl.c>` to avoid printing warning messages of duplicate module loads.
- Adds `LoadModule ssl_module` inside of `<IfModule !mod_ssl.c>` to the top of the main `httpd.conf`.
- Ensures that these modifications are not made multiple times.
Fixes: #6606
* Fix CentOS6 installer issue
* Changelog entry
* Address review comments
* Do not enable mod_ssl if multiple different values were found
* Add test comment
* Address rest of the review comments
* Address review comments
* Better ifmodule argument checking
* Test fixes
* Make linter happy
* Raise an exception when differing LoadModule ssl_module statements are found
* If IfModule !mod_ssl.c with LoadModule ssl_module already exists in Augeas path, do not create new LoadModule directive
* Do not use deprecated assertion functions
* Address review comments
* Kick tests
* Revert "Kick tests"
This reverts commit 967bb574c2.
* Address review comments
* Add pydoc return value to create_ifmod
This PR is a part of the tls-sni-01 removal plan described in #6849.
As `acme` is a library, we need to put some efforts to make a decent deprecation path before totally removing tls-sni in it. While initialization of `acme.challenges.TLSSNI01` was already creating deprecation warning, not all cases were covered.
For instance, and innocent call like this ...
```python
if not isinstance(challenge, acme.challenges.TLSSNI01):
print('I am not using this TLS-SNI deprecated stuff, what could possibly go wrong?')
```
... would break if we suddenly remove all objects related to this challenge.
So, I use the _Deprecator Warning Machine, Let's Pacify this Technical Debt_ (Guido ®), to make `acme.challenges` and `acme.standalone` patch themselves, and display a deprecation warning on stderr for any access to the tls-sni challenge objects.
No dev should be able to avoid the deprecation warning. I set the deprecation warning in the idea to remove the code on `0.34.0`, but the exact deprecation window is open to discussion of course.
* Modules challenges and standalone patch themselves to generated deprecation warning when tls-sni related objects are accessed.
* Correct unit tests
* Correct lint
* Update challenges_test.py
* Correct lint
* Fix an error during tests
* Update coverage
* Use multiprocessing for coverage
* Add coverage
* Update test_util.py
* Factor the logic about global deprecation warning when accessing TLS-SNI-01 attributes
* Fix coverage
* Add comment for cryptography example.
* Use warnings.
* Add a changelog
* Fix deprecation during tests
* Reload
* Update acme/acme/__init__.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update CHANGELOG.md
* Pick a random free port.
This PR is a part of the tls-sni-01 removal plan described in #6849.
This PR removes --tls-sni-01-port, --tls-sni-01-address and tls-sni-01/tls-sni options from --preferred-challenges. They are replace by deprecation warning, indicating that these options will be removed soon.
This deprecation, instead of complete removal, is done to avoid certbot instances to hard fail if some automated scripts still use these flags for some users.
Once this PR lands, we can remove completely theses flags in one or two release.
* Remove tls-sni related flags in cli. Add a deprecation warning instead.
* Adapt tests to cli and renewal towards tls-sni flags deprecation
* Add https_port option. Make tls_sni_01_port show a deprecation warning, but silently modify https_port if set
* Migrate last items
* Fix lint
* Update certbot/cli.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Ensure to remove all occurences of tls-sni-01
* Remove unused parameter
* Revert modifications on cli-help.txt
* Use logger.warning instead of sys.stderr
* Update the logger warning message
* Remove standalone_supported_challenges option.
* Fix order of preferred-challenges
* Remove supported_challenges property
* Fix some tests
* Fix lint
* Fix tests
* Add a changelog
* Clean code, fix test
* Update CI
* Reload
* No hard date for tls-sni removal
* Remove useless cast to list
* Update certbot/tests/renewal_test.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Add entry to the changelog
* Add entry to the changelog
Fixes#6698
Fedora maintainers engaged a deprecation path for Python 2.x with Fedora 29. As a first step, python2-virtualenv does not install the virtualenv binary anymore, in favor of python3-virtualenv, and so the installation of Python 3 virtual environments by default.
However, certbot-auto installs python2-virtualenv for all recent RPM distributions, and relies of the execution of virtualenv, and this is failing the process.
Since the plan in the future is to remove Python 2.x from Fedora, this PR follows this logic to fix certbot-auto: started to Fedora 29, certbot-auto will install and execute certbot on Python 3. This implies to detect that we are on Fedora 29+, install python3-virtualenv that will install also Python 3 dependencies and virtualenv binary, then instruct the process to use Python 3. This is in fact similar to EOL distributions shipping with Python 2.6, and for which Python 3.4 from EPEL is installed and used.
Older versions of Fedora continue to use Python 2.x, and their process is untouched. Four scenarios are covered here:
fresh Fedora 28: old process is used, nothing changes
fresh Fedora 29: new process is used, Python 3 is installed, certbot runs on it
update Fedora 29 from 28, already installed certbot-auto without rebootstrapping required: existing venv continue to be used, certbot runs on it
update Fedora 29 from 28, already installed certbot-auto with rebootstrapping required: new process is used, installing python3-virtualenv, python3-devel and python3-rpm-macros, Python 3 is installed, certbot runs on it
* Add a step to handle python3 on fedora29
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update rpm_python3.sh
* Rebuild certbot-auto
* Empty commit to relaunch CI pipeline
* Add changelog
* Update CHANGELOG.md
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update CHANGELOG.md
Fixes#5789
This PR is about allowing Certbot to respect the Retry-After HTTP header that an ACME CA server can return to a client POSTing to a challenge, to instruct him and retry the request later.
However, this feature was not easily implementable in the current code of certbot.auth_handler, because the code became really hard to read. In fact, @bmw was thinking that the code was really deceiving, and a lot of supposed functionalities declared in the comments were in fact not implemented or not functional.
So I took the time to understand what was going on, and effectively, most of the code is in fact not usable or not used. Then I did a refactoring against the bare ACME spec about what to do to prepare challenges, instruct the ACME CA server to perform them, then polling regularly the authorization resources until they are decided (valid or invalid).
And of course this implementation takes care of Retry-After ^^
I added a lot of comments in the new implementation, to explain what is going on for a future developer. The workflow I used is relying on the relationships between authorizations and challenges states as described in section 7.1.6 of the ACME spec draft: https://datatracker.ietf.org/doc/draft-ietf-acme-acme/
* Clean auth_handler a bit, and implement retry-after.
* Remove a debug logger
* Correct tests
* Fix mypy and lint. Setup max retries and default retry after accordingly.
* Ease a comparison in tests
* Update documentation
* Add tests
* Adapt windows coverage threshold to the global LOC reduction
* Update certbot/auth_handler.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Corrections under review
* Correction under review
* Update certbot/auth_handler.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Corrections under review
* Update auth_handler_test.py
* Reimplementing user readable report for failed authorizations
* Fixes two tests
* Fix another test + lint + mypy
* Update auth_handler.py
* Update auth_handler_test.py
* Fix tests
* Update certbot/auth_handler.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Raise directly the exception on polling timout
* Improve interface documentation
* Move the wait on top of the loop, to be used initially or after a new loop iteration. Do not wait for negative values.
* Always display the report about failed authorizations.
* Clarify an exception.
* Return, instead of break
* Use setdefault
* Remove useless assertion
* Adapt tests
* Improve a test about retry after value.
* Update certbot/auth_handler.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Add a complete test on best_effort
* Add entry to the changelog
* Gather all failed authzrs to be reported in one unique report in case of best_effort
* Build complete warn/report/raise process about failed authzrs
* Reenabling OCSP cryptography support
* Refactor the validation logic of OCSP response to match the OpenSSL one
* Prepare runtime for OCSP response test
* Move unrelated test to another relevant place
* Reimplement OCSP status checks in integration tests
* Clean script
* Protect OCSP check against connection errors
* Update tests/certbot-boulder-integration.sh
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Cleaning
* Add a specific script for letsencrypt-auto install+help
* Remove inconsistent assertion
* Add executable permissions
* Remove unused variable
* Move testdata
* Corrected cleanup code
* Empty commit
Fixes#6755.
POSTing the `keyAuthorization` in a JWS token when answering an ACME challenge, has been deprecated for some time now. Indeed, this is superfluous as the request is already authentified by the JWS signature.
Boulder still accepts to see this field in the JWS token, and ignore it. Pebble in non strict mode also. But Pebble in strict mode refuses the request, to prepare complete removal of this field in ACME v2.
Certbot still sends the `keyAuthorization` field. This PR removes it, and makes Certbot compliant with current ACME v2 protocol, and so Pebble in strict mode.
See also [letsencrypt/pebble#192](https://github.com/letsencrypt/pebble/issues/192) for implementation details server side.
* New implementation, with a fallback.
* Add deprecation on changelog
* Update acme/acme/client.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Fix an instance parameter
* Update changelog, extend coverage
* Update comment
* Add unit tests on keyAuthorization dump
* Update acme/acme/client.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Restrict the magic of setting a variable in immutable object in one place. Make a soon to be removed method private.
So merging the study from @bmw and me, here is what happened.
Each invocation of `certbot.logger.post_arg_parse_setup` create a file handler on `letsencrypt.log`. This function also set an atexit handler invoking `logger.shutdown()`, that have the effect to close all logger file handler not already closed at this point. This method is supposed to be called when a python process is close to exit, because it makes all logger unable to write new logs on any handler.
Before #6667 and this PR, for tests, the atexit handle would be triggered only at the end of the pytest process. It means that each test that launches `certbot.logger.post_arg_parse_setup` add a new file handler. These tests were typically connecting the file handler on a `letsencrypt.log` located in a temporary directory, and this directory and content was wipped out at each test tearDown. As a consequence, the file handles, not cleared from the logger, were accumulating in the logger, with all of them connected to a deleted file log, except the last one that was just created by the current test. Considering the number of tests concerned, there were ~300 file handler at the end of pytest execution.
One can see that, on prior #6667, by calling `print(logger.getLogger().handlers` on the `tearDown` of these tests, and see the array growing at each test execution.
Even if this represent a memory leak, this situation was not really a problem on Linux: because a file can be deleted before it is closed, it was only meaning that a given invocation of `logger.debug` for instance, during the tests, was written in 300 log files. The overhead is negligeable. On Windows however, the file handlers were failing because you cannot delete a file before it is closed.
It was one of the reason for #6667, that added a call to `logging.shutdown()` at each test tearDown, with the consequence to close all file handlers. At this point, Linux is not happy anymore. Any call to `logger.warn` will generate an error for each closed file handler. As a file handler is added for each test, the number of errors grows on each test, following an arithmetical suite divergence.
On `test_sdists.py`, that is using the bare setuptools test suite without output capturing, we can see the damages. The total output takes 216000 lines, and 23000 errors are generated. A decent machine can support this load, but a not a small AWS instance, that is crashing during the execution. Even with pytest, the captured output and the memory leak become so large that segfaults are generated.
On the current PR, the problem is solved, by resetting the file handlers array on the logging system on each test tearDown. So each fileHandler is properly closed, and removed from the stack. They do not participate anymore in the logging system, and can be garbage collected. Then we stay on always one file handler opened at any time, and tests can succeed on AWS instances.
For the record, here is all the places where the logging system is called and fail if there is still file handlers closed but not cleaned (extracted from the original huge output before correction):
```
Logged from file account.py, line 116
Logged from file account.py, line 178
Logged from file client.py, line 166
Logged from file client.py, line 295
Logged from file client.py, line 415
Logged from file client.py, line 422
Logged from file client.py, line 480
Logged from file client.py, line 503
Logged from file client.py, line 540
Logged from file client.py, line 601
Logged from file client.py, line 622
Logged from file client.py, line 750
Logged from file cli.py, line 220
Logged from file cli.py, line 226
Logged from file crypto_util.py, line 101
Logged from file crypto_util.py, line 127
Logged from file crypto_util.py, line 147
Logged from file crypto_util.py, line 261
Logged from file crypto_util.py, line 283
Logged from file crypto_util.py, line 307
Logged from file crypto_util.py, line 336
Logged from file disco.py, line 116
Logged from file disco.py, line 124
Logged from file disco.py, line 134
Logged from file disco.py, line 138
Logged from file disco.py, line 141
Logged from file dns_common_lexicon.py, line 45
Logged from file dns_common_lexicon.py, line 61
Logged from file dns_common_lexicon.py, line 67
Logged from file dns_common.py, line 316
Logged from file dns_common.py, line 64
Logged from file eff.py, line 60
Logged from file eff.py, line 73
Logged from file error_handler.py, line 105
Logged from file error_handler.py, line 110
Logged from file error_handler.py, line 87
Logged from file hooks.py, line 248
Logged from file main.py, line 1071
Logged from file main.py, line 1075
Logged from file main.py, line 1189
Logged from file ops.py, line 122
Logged from file ops.py, line 325
Logged from file ops.py, line 338
Logged from file reporter.py, line 55
Logged from file selection.py, line 110
Logged from file selection.py, line 118
Logged from file selection.py, line 123
Logged from file selection.py, line 176
Logged from file selection.py, line 231
Logged from file selection.py, line 310
Logged from file selection.py, line 66
Logged from file standalone.py, line 101
Logged from file standalone.py, line 88
Logged from file standalone.py, line 97
Logged from file standalone.py, line 98
Logged from file storage.py, line 52
Logged from file storage.py, line 59
Logged from file storage.py, line 75
Logged from file util.py, line 56
Logged from file webroot.py, line 165
Logged from file webroot.py, line 186
Logged from file webroot.py, line 187
Logged from file webroot.py, line 204
Logged from file webroot.py, line 223
Logged from file webroot.py, line 234
Logged from file webroot.py, line 235
Logged from file webroot.py, line 237
Logged from file webroot.py, line 91
```
* Reapply #6667
* Make setuptools delegates tests execution to pytest, like in acme module.
* Clean handlers at each tearDown to avoid memory leaks.
* Update changelog
Apache plugin will now use command line default values from `ApacheConfingurator.OS_DEFAULTS` instead of respective distribution override when `CERTBOT_DOCS=1` environment variable is present.
Fixes: #6234
* Apache: respect CERTBOT_DOCS environment variable
* Move the tests to apache plugin
I think this is causing failures in some of our tests so this PR reverts the change until we can fix the problem.
The 2nd commit is to keep the change using more idiomatic wording in the changelog for another change that got included in this PR.
* Revert "Use built-in support for OCSP in cryptography >= 2.5 (#6603)"
This reverts commit 2ddaf3db04.
* keep changelog correction
Attempts to configure all of the following VirtualHosts for answering the HTTP challenge:
* VirtualHosts that have the requested domain name in either `ServerName` or `ServerAlias` directive.
* VirtualHosts that have a wildcard name that would match the requested domain name.
This also applies to HTTPS VirtualHosts, making Apache plugin able to handle cases where HTTP redirection takes place in reverse proxy or similar, before reaching the Apache HTTPD.
Even though also HTTPS VirtualHosts are selected, Apache plugin tries to ensure that at least one of the selected VirtualHosts listens to HTTP-01 port (configured with `--http-01-port` CLI option). So in a case where only HTTPS VirtualHosts exist, but user wants to configure those, `--http-01-port` parameter needs to be set for the port configured to the HTTPS VirtualHost(s).
Fixes: #6730
* Select all matching VirtualHosts for HTTP-01 challenges instead of just one
* Finalize PR and add tests
* Changelog entry
In response to #6594. [Fixes #6594.]
To execute OCSP requests, certbot relies currently on a openssl binary execution. If openssl is not present in the PATH, the OCSP check will be silently ignored. Since version 2.4, cryptography has support for OCSP requests, without the need to have openssl binary available locally.
This PR takes advantage of it, and will use the built-in support of OCSP in cryptography for versions >= 2.4. Otherwise, fallback is done do a direct call to openssl binary, allowing oldest requirements to still work with legacy cryptography versions.
Update: requirement is now cryptography >= 2.5, to avoid to rely on a private method from cryptography.
* Implement logic using cryptography
* Working OSCP using pure cryptography
* Fix openssl usage in unit tests
* Reduce verbosity
* Add tests
* Improve naive skipIf
* Test resiliency
* Update ocsp.py
* Validate OCSP response. Unify OCSP URL get
* Improve resiliency checks, correct lint/mypy
* Improve hash selection
* Fix warnings when calling openssl bin
* Load OCSP tests assets as vectors.
* Update ocsp.py
* Protect against invalid ocsp response.
* Add checks to OCSP response
* Add more control on ocsp response
* Be lenient about assertion that next_update must be in the future, similarly to openssl.
* Construct a more advanced OCSP response mock to trigger more logic in ocsp module.
* Add test
* Refactor signature process to use crypto_util
* Fallback for cryptography 2.4
* Avoid a collision with a meteor.
* Correct method signature documentation
* Relax OCSP update interval
* Trigger built-in ocsp logic from cryptography with 2.5+
* Update pinned version of cryptography
* Update certbot/ocsp.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Update ocsp.py
* Update ocsp_test.py
* Update CHANGELOG.md
* Update CHANGELOG.md
The existing `acme.TLSALPN01` challenge class did not have
a `response_cls`, meaning it was not possible to use a tls-alpn-01
challenge with `client.answer_challenge`.
To support the above a simple `TLSALPN01Response` class is added that
doesn't provide the ability to solve a tls-alpn-01 challenge end to end
(e.g. generating and serving the correct response certificate) but that
does allow the challenge to be initiated. This is sufficient for users
that have set up the challenge response independent of the `acme`
module code.
Resolves#6676
