The account path used to store user credentials is calculated from the domain used to contact the relevant ACME CA server.
For instance, if the directory URL is https://my.domain.net/directory, then the account path will be $CONFIG_DIR/accounts/my.domain.net.
However, if non standard HTTP/HTTPS port need to be used, colons will be involved. For instance, https://my.domain.net:14000/directory will give $CONFIG_DIR/accounts/my.domain.net:14000.
Colons in paths are supported on POSIX systems, but not on Windows (it is reserved for the root drive letter).
This PR replaces colons by underscores for account paths on Windows, and leaves them untouched on Linux.
* Fix account path on Windows when colons are involved
* Protect colon in drive letter
* Refactor compat platform specific logic
When certbot is executing, several resources are opened. It is typically file handles and locks on them. Of course, theses resources need to be cleanup. It is done in Certbot by registering cleanup functions through atexit module, that ensures theses functions will be called when Certbot exit. This allow to not care about resource cleanup everywhere in the code, as it is processed globally.
The problem with atexit is it cleanup functions are called when the Python program exit. If the program is Certbot itself when used, this is Pytest in unit test execution. So during a unit test execution, cleanup is not called after a test and before its tearDown, but when Pytest exit, so way after tests and their respective tearDown.
But many tearDown implies to delete folders where this kind of resources are hold.
This is never a problem on Linux, thanks to its non-blocking file handling. It is usually not a problem on Windows, despite its blocking approach. But if the tearDown requires folder cleanup, exceptions are raised, and currently hidden as warnings. There is currently 504 exceptions of this type in Certbot core tests on Windows.
This PR starts to correct this situation. To do so, some of the functions cleanup normally called through atexit, are explicitly called as part of the tearDown process of relevant test classes, before directory removal is done. Theses situations come all from the certbot.tests.util.TempDirTestCase, so the code is in this specific tearDown process.
As a consequence, exceptions drop from 504 to 64.
Then there are still a significant part of them, that will be handled in later mitigation.
* Call atexit handlers before test tearDown to reduce errors on Windows
* Clear locks dict after global releasing execution
* Remove last tearDown errors.
* Clean out mock on open.
* Remove a test
* Reenable some tests
This PR updates Lexicon dependency to the latest version available, 3.0.6, for every lexicon-based DNS plugins. It updates also the provider construction to use the new ConfigResolverobject, and to remove the legacy configuration process.
In response to #5342.
Currently, certbot will execute the operations necessary to validate a challenge even if the challenge has already been validated before against the acme ca server. This can occur for instance if a certificate is asked and issue correctly, then deleted locally, then asked again.
It is a corner case, but it will lead to some heavy operations (like updating a DNS zone, or creating an HTTP server) that are not needed.
This PR corrects this behavior by not executing challenges already validated, and use them directly instead to issue the certificate.
Fixes#5342
* Avoid to execute a given challenge that have been already validated by acme ca server.
* Execute tls challenge on a separate dns name, to avoid reusing the existing valid http challenge.
* Align with master
* Improve log
* Simplify the implementation
* Update changelog
* Add a unit test to ensure that validated challenges are not rerun
* Extraction from #6541 to add flag to disable shuffle sleep on renew action
* Move the logic of random sleep to execute it only if there is effectively a certificate to renew.
* Add comments
* Correct lint
* Suspend lint rule
* Revert code cleaning
* Hide the flag
* Ignore lint
* Update cli.py
Not updating the guidelines in using.rst-- I don't want to encourage people to use this. now that Certbot preserves gid, the better way to set permissions is to change the group permisisons!
* Preserve other-read bit on private keys too
* fix integration test
* fix and rename permission routines in integration tests
* Remove duplicate route53 options
Duplicate route53 option (certbot-route53:auth) removed.
* Remove duplicate route53 options from help
Removed duplicate route53 options from help by adding a couple of if statements in cli.py.
* Fix formatting
* Use cleaner solution to remove duplicate route53 options from help
Used a cleaner method to remove the duplicate option from the help text
* Fix line too long
* Add regression test
Added a regression test and fixed a couple of minor issues.
* Fix trailing whitespace
* Add missing newline
* Put newline in correct place
* Rough draft of External Account Binding.
* Remove parameter --eab and namespace kid and hmac. Also add parameters to "register" subcommand.
* Refactor as much as possible of the EAB functionality into ExternalAccountBinding class.
* Remove debug line.
* Added external account binding to Directory.Meta.
* Rename to account_public_key, hmac_key and make some non-optional.
Rename command line argument to --eab-hmac-key.
* Error out when the server requires External Account Binding and the user
has not supplied kid and hmac key.
* Remove whitespace.
* Refactor a bit to make it possible to set the url argument.
* Move from_data method into client.
* Revert "Move from_data method into client."
This reverts commit 8963fae
* Refactored to use json field on Registration.
* Inherit from object according to Google Python Style Guide.
* Move to two separate ifs.
* Get tests to pass after External Account Binding additions.
* messages.py back to 100% test coverage with some EAB tests.
* .encode() this JSON key.
* Set eab parameter default values to None.
* * Remove unnecessary public key mock on most of the test.
* Restructure the directory mock to be able to mock both True and False for externalAccountRequired easily.
* Add EAB client tests.
* Move external_account_required check into BackwardsCompatibleClientV2 to be able to mock it.
* Update versions.
* Try 0.29.0.
* Revert "Try 0.29.0."
This reverts commit 5779509
* Try 0.29.0 again.
* Try this.
* Fix pylint failures.
* Add tests for external_account_required method.
* Test not needed, avoid:
************* Module acme.client_test
C: 1, 0: Too many lines in module (1258/1250) (too-many-lines)
* Move real external_account_required method into ClientV2 and pass through to it in BackwardsCompatibleClientV2.
* Handle missing meta key in server ACME directory.
* Add docstring for BackwardsCompatibleClientV2.external_account_required().
* Add tests for BackwardsCompatibleClientV2.external_account_required().
* Fix coverage for ACMEv1 code in BackwardsCompatibleClientV2.
* Disable pylint too-many-lines check for client_test.py.
* Fix versions.
* Remove whitespace that accidently snuck into an earlier commit.
* Remove these two stray whitespaces also.
* And the last couple of whitespaces.
* Add External Account Binding to changelog.
* Add dev0 suffix to setup.py.
Co-Authored-By: robaman <robert@kastel.se>
* Set to "-e acme[dev]" again.
Co-Authored-By: robaman <robert@kastel.se>
Fixes#1473.
writes privkey.pem to 0600 by default for new lineages
on renewals where a new privkey is generated, preserves group mode and gid
Things this PR does not do:
we talked about forcing 0600 on privkeys when a Certbot upgrade is detected. Instead, this PR only creates new lineages with the more restrictive permission to prevent renewal breakages.
this doesn't solve many of the problems mentioned in #1473 that are not directly related to the title issue!
* safe_open on archive keyfiles
* keep group from current lineage
* clean up integration test
* safe_open can follow symlinks
* fix tests on windows, maybe
* Address Brad's comments
* Revert changes to safe_open
* Test chown is called when saving new key
* Reorder chown operation
* Changelog and documentation
* Fix documentation style
* WIP on adding a random sleep for noninteractive renewal
* Update changelog
* Log the fact that we're randomly sleeping
* stdin may better define interactivity than stdout
* Try mocking time.sleep for all tests
* Move mocked sleep elsewhere
* mock the right object
* Somewhat ugly synthetic PTY trick
* Move set -u down below self-exec
* Revert "Move set -u down below self-exec"
This reverts commit 6bde65a738.
* Revert "Somewhat ugly synthetic PTY trick"
This reverts commit 89c704a4be.
* Log specific duration of random sleep
* Test coverage for random sleep() logic in main.py
Defaults path of Certbot are the following:
config: /etc/letsencrypt
workdir: /var/letsencrypt/lib
logs: /var/letsencrypt/log
On Windows, this translate into:
config: C:\etc\letsencrypt
workdir: C:\var\letsencrypt\lib
logs: C:\var\letsencrypt\log
As Windows does not follow the standard POSIX filesystem layout, theses paths do not have a lot of sense in this case.
This PR sets the following default paths when Certbot is run on Windows:
config: C:\Certbot
workdir: C:\Certbot\lib
logs: C:\Certbot\log
Better to decide the default paths for Certbot before users start to run it on Windows, to avoid future migration procedures.
Certbot relies heavily on bash scripts to deploy a development environment and to execute tests. This is fine for Linux systems, including Travis, but problematic for Windows machines.
This PR converts all theses scripts into Python, to make them platform independant.
As a consequence, tox-win.ini is not needed anymore, and tox can be run indifferently on Windows or on Linux using a common tox.ini. AppVeyor is updated accordingly to execute tests for acme, certbot and all dns plugins. Other tests are not executed as they are for Docker, unsupported Apache/Nginx/Postfix plugins (for now) or not relevant for Windows (explicit Linux distribution tests or pylint).
Another PR will be done on certbot website to update how a dev environment can be set up.
* Replace several shell scripts by python equivalent.
* Correction on tox coverage
* Extend usage of new python scripts
* Various corrections
* Replace venv construction bash scripts by python equivalents
* Update tox.ini
* Unicode lines to compare files
* Put modifications on letsencrypt-auto-source instead of generated scripts
* Add executable permissions for Linux.
* Merge tox win tests into main tox
* Skip lock_test on Windows
* Correct appveyor config
* Update appveyor.yml
* Explicit coverage py27 or py37
* Avoid to cover non supported certbot plugins on Windows
* Update tox.ini
* Remove specific warnings during CI
* No cover on a debug code for tests only.
* Update documentation and help script on venv/venv3.py
* Customize help message for Windows
* Quote correctly executable path with potential spaces in it.
* Copy pipstrap from upstream
On Linux, you can invoke os.rename(src, dst) even if dst already exists. In this case, destination file will be atomically replaced by the source file.
On Windows, this will lead to an OSError because changes are not atomic. This cause certbot renew to fail in particular, because the old certificate configuration needs to be replace by the new when a certificate is effectively renewed.
One could use the cross-platform function os.replace, but it is available only on Python >= 3.3.
This PR add a function in compat to handle correctly this case on Windows, and delegating everything else to os.rename.
* Cross platform compatible os.rename (we can use os.replace if its python 3)
* Use os.replace instead of custom non-atomic code.
* Avoid errors for lint and mypy. Add a test.
* flip challenge preference in Nginx
* Fix Nginx tests
* Flip challenge preference in Apache
* Flip challenge preference in standalone
* update changelog
* continue to run with tls-sni in integration tests for coverage
