455 Commits
Author SHA1 Message Date
alexzorinandAdrien Ferrand 08d91b456b Use fresh authorizations in dry runs (#7442)
* 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
2019-11-04 21:23:25 +01:00
Brandon MooreandBrad Warren 1c05b9bd07 Dropped deprecated flags from commands (#7482)
This pull request addresses #7451 by removing the deprecated flags.

* Dropped deprecated flags from commands

* Updated changelog for dropped flags and deleted outdated tests

* removed init-script part of apache test
2019-11-04 09:50:57 -08:00
Joona HoikkalaandBrad Warren fb1aafb5d2 Use distro library for all OS version detection (#7467)
This pull request ensures that we use distro package in all the distribution version detection. It also replaces the custom systemd /etc/os-release parsing and adds a few version fingerprints to Apache override selection.

Fixes: #7405

* Revert "Try to use platform.linux_distribution() before distro equivalent (#7403)"

This reverts commit ca3077d034.

* Use distro for all os detection code

* Address review comments

* Add changelog entry

* Added tests

* Fix tests to return a consistent os name

* Do not crash on non-linux systems

* Minor fixes to distro compatibility checks

* Make the tests OS independent

* Update certbot/util.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Skip linux specific tests on other platforms

* Test fixes

* Better test state handling

* Lower the coverage target for Windows tests
2019-11-01 10:51:21 -07:00
Brad WarrenandGitHub 63d673a3e0 Remove references to TLS-SNI-01 outside of ACME (#7479)
This is a big part of #7214. It removes all references to TLS-SNI-01 outside of acme (and pytest.ini). Those changes will come in a subsequent PR. I thought this one was getting big enough.

* Remove references to TLS-SNI-01 in Apache plugin

* Remove references to TLS-SNI-01 from certbot-nginx

* Remove references to TLS-SNI from Certbot.

* Remove TLS-SNI reference from docs

* add certbot changelog

* Clarify test behavior
2019-10-31 10:17:29 -07:00
Brad WarrenandAdrien Ferrand 0f31d9b7ac Remove skip_unless cruft (#7410)
* Remove skip_unless cruft.

* remove unused import
2019-10-24 14:46:55 +02:00
alexzorinandAdrien Ferrand 118cb3c9b1 cli: allow --dry-run to be combined with --server (#7436)
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.
2019-10-10 00:09:25 +02:00
Adrien FerrandandBrad Warren ec3ec9068c Upgrade to pywin32>=225 and fix unit tests (#7429)
Fixes #7426
2019-10-08 16:17:08 -07:00
Jacob Hoffman-AndrewsandBrad Warren 3608abb01a Remove unnecessary account ID match check. (#7416)
* Remove unnecessary account ID match check.

Right now the Account object calculates an ID using md5. This is
unnecessary and causes problems on FIPS systems that forbid md5. It's
just as good to pick a random series of bytes for the ID, since the ID
gets read out of renewal/foo.conf.

However, if we switched the algorithm right now, we could wind up
breaking forward compatibility / downgradeability, since older versions
would run into this check.

Removing this check now lays the ground to change the ID-calculation
algorithm in the future.

Related to #1948 and
https://github.com/certbot/certbot/pull/1013#issuecomment-149983479.

* Remove test.

* Remove unused import.
2019-10-02 14:44:25 -07:00
Joona HoikkalaandBrad Warren ca3077d034 Try to use platform.linux_distribution() before distro equivalent (#7403)
Try to primarily fall back to using `platform.linux_distribution()` if `/etc/os-release` isn't available. Only use `distro.linux_distribution()` on Python >= 3.8.

* Try to use platform.linux_distribution() before distro equivalent

* Fix tests for py38

* Added changelog entry
2019-09-27 09:50:38 -07:00
ohemorangeandBrad Warren 18e6c6c2a8 Don't send OCSP requests for expired certificates (#7387)
Fixes #7152.

* don't check ocsp if cert is expired when getting cert information

* don't check ocsp if the cert is expired in ocsp_revoked

* update tests

* update changelog

* move pytz import to the top of the test file
2019-09-23 17:20:11 -07:00
Adrien FerrandandBrad Warren ab76834100 [Windows|Linux] Forbid os.stat and os.fstat (#7325)
Fixes #7212

This PR forbid os.stat and os.fstat, and fix or provide alternatives to avoid its usage in certbot outside of certbot.compat.filesystem.

* Reimplement private key mode propagation

* Remove other os.stat

* Remove last call of os.stat in certbot package

* Forbid stat and fstat

* Implement mode comparison checks

* Add unit tests

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Handle case where multiple ace concerns a given SID in has_min_permissions

* Add a new test scenario

* Add a simple test for has_same_ownership

* Fix name function

* Add a comment explaining an ACE structure

* Move a test in its dedicated class

* Improve a message error

* Calculate has_min_permission result using effective permission rights to be more generic.

* Change an exception message

* Add comments, avoid to skip a test.

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
2019-09-06 14:30:25 -07:00
ohemorangeandBrad Warren 0fe28a6459 Replace platform.linux_distribution with distro.linux_distribution (#7337)
Smallest possible fix for #7106 

* Replace platform.linux_dependencies with distro.linux_dependencies

* run build.py

* Add minimum version of 1.0.1

* Pin back requests package

* Update changelog
2019-08-27 18:31:35 -07:00
Adrien FerrandandBrad Warren 0324d1740e Ensure relpath is executed on paths in the same drive (#7335)
On Windows you can have several drives (`C:`, `D:`, ...), that is the roughly (really roughly) equivalent of mount points, since each drive is usually associated to a specific physical partition.

So you can have paths like `C:\one\path`, `D:\another\path`.

In parallel, `os.path.relpath(path, start='.')` calculates the relative path between the given `path` and a `start` path (current directory if not provided). In recent versions of Python, `os.path.relpath` will fail if `path` and `start` are not on the same drive, because a relative path between two paths like `C:\one\path`, `D:\another\path` is not possible.

In saw unit tests failing because of this in two locations. This occurs when the certbot codebase that is tested is on a given drive (like `D:`) while the default temporary directory used by `tempfile` is on another drive (most of the time located in `C:` drive).

This PR fixes that.
2019-08-23 12:53:30 -07:00
Adrien FerrandandGitHub 6882f006ac [Windows] Fix closing files descriptors during unit tests (#7326)
* Fix file descriptor cleanup during tests on Windows

* Fix lint

* Remove useless tearDown

* Clean pylint
2019-08-16 11:08:42 +02:00
Adrien FerrandandBrad Warren 56f609d4f5 Fix unit tests on Windows (#7270)
Fixes #6850

This PR makes the last corrections needed to run all unit tests on Windows:

add a function to check if a hook is executable in a cross-platform compatible way
handle correctly the PATH surgery for Windows during hook execution
handle correctly an account compatibility over both ACMEv1 and ACMEv2
remove (finally!) the @broken_on_windows decorator.

* Fix account_tests

* Fix hook executable test

* Remove the temporary decorator @broken_on_windows

* Fix util_test

* No broken unit test on Windows anymore

* More elegant mock

* Fix context manager

* Adapt coverage

* Corrections

* Adapt coverage

* Forbid os.access
2019-08-01 10:39:46 -07:00
e6bf3fe7f8 [Windows] Security model for files permissions - STEP 3f (#7233)
* Correct file permissions on TempHandler

* Forbid os.chown and os.geteuid, as theses functions can be harmful to the security model on Windows.

* Implement copy_ownership

* Apply copy_ownership

* Correct webroot tests (and activate another broken test !)

* Correct lint and mypy

* Ensure to apply mode in makedirs

* Apply strict permissions on directories created with tempfile.mkdtemp(), like on Unix.

* Ensure streamHandler has 0600 on Windows

* Reactivate a test on windows

* Pin oldest requirements to current internal libraries (acme and certbot)

* Add dynamically pywin32 in dependencies: always except for certbot-oldest to avoid to break the relevant tests.

* Administrative privileges are always required.

* Correct security implementation (not the logic yet)

* First correction. Allow to manipulate finely file permissions during their generation

* Align to master + fix lint + resolve correctly symbolic links

* Add a test for windows about default paths

* Strenghthen the detection of Linux/Windows to check the standard files layout.

* Fix lint and mypy

* Reflect non usage of cache discovery from dns google plugin to its tests, solving Windows tests on the way

* Apply suggestions from code review

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Add more details in a comment

* Retrigger build.

* Add documentation.

* Fix a test

* Correct RW clear down

* Update util.py

* Remove unused code

* Fix code style

* Adapt certbot coverage threshold on Linux due to Windows specific LOC addition.

* Various optimizations around file owner and file mode

* Fix last error

* Fix copy_ownership_and_apply_mode

* Fix lint

* Correct mypy

* Extract out first part from windows-file-permissions

* Ignore new_compat in coverage for now

* Create test package for compat

* Add unit tests for security module.

* Add pywin32

* Adapt linux coverages to the windows-specific LOCs added

* Clean imports

* Correct import

* Trigger CI

* Reactivate a test

* Create the certbot.compat package. Move logic in certbot.compat.misc

* Clean comment

* Add doc

* Fix lint

* Correct mypy

* 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

* Fix some mocks

* Update account_test.py

* Update os.py

* Update os.py

* Update local oldest requirements

* Implement the new linter_plugin

* Fix remaining linting errors

* Fix local oldest for nginx

* Remove custom check in favor of pylint plugin

* Remove check coding style

* Update linter_plugin.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Add several comments

* Update the setup.py

* Add documentation

* Update acme dependencies

* Update certbot/compat/os.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update certbot/compat/os.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update certbot/compat/os.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update linter_plugin.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update linter_plugin.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Corrections

* Handle os.path. Simplify checker.

* Add a comment to a reference implementation

* Update changelog

* Fix module registering

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update docs/contributing.rst

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update config and changelog

* Correction

* Correct os

* Fix merge

* Disable pylint checks

* Normalize imports

* Simplify security

* Corrections

* Reorganize module

* Clean code

* Clean code

* Remove coverage

* No cover

* Implement security.chmod

* Disable a test for now

* Disable hard error for now

* Add a first test. Remove unused import

* Recalibrate coverage

* Modifications for misc

* Correct function call

* Add some types

* Remove newline

* Use os_rename

* Implement security.open

* Revert to windows-files-permissions approach

* Fix lint

* Implement security.mkdir and security.makedirs

* Fix lint

* Clean lint

* Clean lint

* Revert "Clean lint"

This reverts commit 83bf81960a.

* Correct mock

* Conditionally add pywin32 on setuptools versions that support environment markers.

* Fix separator

* Fix separator

* Rename security into filesystem

* Change module security to filesystem

* Move rename into filesystem

* Rename security into filesystem

* Rename security into filesystem

* Rerun CI

* Fix import

* Fix pylint

* Implement copy_ownership_and_apply_mode

* Fix pylint

* Update certbot/compat/os.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Remove default values

* Rewrite a comment.

* Relaunch CI

* Pass as keyword arguments

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Make the private key permissions transfer platform specific

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Rename variable

* Fix comment0

* Add unit test for copy_ownership_and_apply_mode

* Adapt coverage

* Implement new methods.

* Remove the old method

* Reimplement make_or_verify_dir

* Finish migration

* Start to fix tests

* Fix ownership when creating a file with filesystem.open

* Fix security on TempHandler

* Fix validation path permissions

* Fix owner on mkdir

* Use a proper workdir for crypto tests

* Fix pylint

* Adapt coverage

* Update storage_test.py

* Update util_test.py

* Clean code

* Update certbot/compat/filesystem.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Add comment

* Update certbot/compat/filesystem.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Check permissions

* Change test mode

* Add unit test for filesystem.check_* functions

* Update filesystem_test.py

* Better logic for TempHandler

* Adapt coverage
2019-07-26 00:25:36 +02:00
Brad WarrenandGitHub 06a0dae67f Fix test_symlink_resolution on macOS. (#7263)
This fixes the test failures which can be seen at
https://travis-ci.com/certbot/certbot/builds/120123338.

The problem here is the path returned by tempfile.mkdtemp() contains a symlink.
For instance, one run of the function produced
'/var/folders/3b/zg8fdh5j71x92yyzc1tyllfw0000gp/T/tmp3k9ytfj1' which is a
symlink to
'/private/var/folders/3b/zg8fdh5j71x92yyzc1tyllfw0000gp/T/tmp3k9ytfj1'.

Removing this symlink before testing filesystem.realpath solves the problem.

You can see the macOS tests passing with this change at https://travis-ci.com/certbot/certbot/builds/120250667.
2019-07-23 11:01:29 -07:00
Adrien FerrandandBrad Warren 71ff47daad Implement a consistent realpath function in certbot.compat.filesystem (#7242)
Fixes #7115 

This PR creates a `realpath` method in `filesystem`, whose goal is to replace any call to `os.path.realpath` in Certbot. The reason is that `os.path.realpath` is broken on some versions of Python for Windows. See https://bugs.python.org/issue9949. The function created here works consistently across Linux and Windows.

As for the other forbidden functions in `os` module, our `certbot.compat.os` will raise an exception if its `path.realpath` function is invoked, and using the `os` module from Python is forbidden from the pylint check implemented in our CI.

Every call to `os.path.realpath` is corrected in `certbot` and `certbot-apache` modules.

* Forbid os.path.realpath

* Finish implementation

* Use filesystem.realpath

* Control symlink loops also for Linux

* Add a test for forbidden method

* Import a new object from os.path module

* Use same approach of wrapping than certbot.compat.os

* Correct errors

* Fix dependencies

* Make path module internal
2019-07-18 14:31:39 -07:00
Adrien FerrandandBrad Warren c4684f187a Add a test for the default directories on Windows (#7238)
There is a unit test to check that the default directories for Certbot are not diverging, in certbot.tests.cli_test:FlagDefaultTests:test_linux_directories.

But this test is not done on Windows.

This PR fixes that.
2019-07-11 17:49:52 -07:00
Adrien FerrandandBrad Warren 74292a10f5 [Windows] Security model for files permissions - STEP 3e (#7182)
This PR implements the filesystem.copy_ownership_and_apply_mode method from #6497.

This method is used in two places in Certbot, replacing os.chown, to copy the owner and group owner from a file to another one, and apply to the latter the given POSIX mode.

* Implement copy_ownership_and_apply_mode

* Update certbot/compat/os.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Remove default values

* Rewrite a comment.

* Relaunch CI

* Pass as keyword arguments

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Make the private key permissions transfer platform specific

* Update certbot/compat/filesystem.py

Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>

* Rename variable

* Fix comment0

* Add unit test for copy_ownership_and_apply_mode

* Adapt coverage

* Execute unconditionally chmod with copy_ownership_and_apply_mode. Improve doc.
2019-07-10 16:26:30 -07:00
Adrien Ferrandandohemorange 7d61e9ea56 [Windows] Security model for files permissions - STEP 3d (#6968)
* Implement security.mkdir and security.makedirs

* Fix lint

* Correct mock

* Rename security into filesystem

* Update apache and nginx plugins requirements

* Update certbot/plugins/webroot.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Reenable pylint here

* Move code

* Reimplement mkdir

* Control errors on eexist, remove superfluous chmod for makedirs

* Add proper skip for windows only tests

* Fix lint

* Fix mypy

* Clean code

* Adapt coverage threshold on Linux with addition of LOC specific to Windows

* Add forbiden functions to tests

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Simplify code

* Sync _get_current_user with part3c

* Use the simpliest implementation

* Remove exist_ok, simplify code.

* Simplify inline comment

* Update filesystem_test.py

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/plugins/webroot.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/plugins/webroot.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Add a test to check we set back os.mkdir correctly after filesystem.makedirs is called.

* Fix lint, adapt coverage
2019-07-03 16:20:43 -07:00
Brad Warrenandohemorange 20b595bc9e Simplify and deprecate viewing config changes (#7198)
* Remove apache and nginx from config_changes help

* Deprecate certbot_config changes.

* Document config_changes deprecation.

* Remove view_config_changes as IInstaller method.

* Remove view_config_changes from plugins.

* Add view_config_changes warnings.

* simplify test_config_changes_deprecation
2019-07-02 17:20:12 -07:00
Adrien Ferrandandohemorange 88876b9901 [Windows] Security model for files permissions - STEP 3c (#6967)
* Implement security.open

* Clean lint

* Rename security into filesystem

* Update certbot/compat/filesystem.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/util.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/lock.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/lock.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Simplify and make more clear comment on os.open.

* Secure implementation preventing race conditions

* Revert "Secure implementation preventing race conditions"

This reverts commit dbb8549219.

* Simplify the logic on Windows.

* Implement os.open to prevent race conditions

* Add unit tests

* Handle os.O_CREAT and os.O_EXCL directly from the Windows APIs

* Improve comments

* Use CREATE_ALWAYS

* Adapt coverage threshold to new Windows specific LOCs.

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/compat/os.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/compat/filesystem.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Add some comments

* Fix pylint

* Improve docstring

* Added test cases

* Improve docstring

* Update certbot/lock.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Update certbot/lock.py

Co-Authored-By: ohemorange <ebportnoy@gmail.com>

* Fix lint

* Adapt coverage

* Adapt coverage
2019-07-02 16:21:24 -07:00
Brad Warrenandohemorange 1c6210ee00 Fix certbot config_changes (#7197)
* Remove for_logging parameter.

* Remove broken/unused --num parameter.

* update changelog
2019-06-26 17:46:51 -07:00
Adrien FerrandandBrad Warren e9bcaaa576 [Windows] Security model for files permissions - STEP 3a (#6964)
This PR implements the filesystem.chmod method from #6497.

* Implement filesystem.chmod

* Conditionally add pywin32 on setuptools versions that support environment markers.

* Update apache plugin requirements

* Use a try/except import approach similar to lock

* Add comments about well-known SIDs

* Add main command

* Call filesystem.chmod in tests, remove one test

* Add test for os module

* Update environment marker

* Ensure we are not building wheels using an old version of setuptools

* Added a link to list of NTFS rights

* Simplify sid comparison

* Enable coverage

* Sometimes, double-quote is the solution

* Add entrypoint

* Add unit tests to filesystem

* Resolve recursively the link, add doc

* Move imports to the top of the file

* Remove string conversion of the ACL, fix setup

* Ensure admins have all permissions

* Simplify dacl comparison

* Conditionally raise for windows temporary workaround

* Add a test to check filesystem.chown is protected against symlink loops
2019-06-20 10:52:43 -07:00
Adrien FerrandandBrad Warren d75908c645 [Windows] Security model for files permissions - STEP 3b (#6965)
* Modifications for misc

* Add some types

* Use os_rename

* Move rename into filesystem

* Use our os package

* Rename filesystem.rename to filesystem.replace

* Disable globally function redefined lint in os module
2019-06-11 17:08:48 -07:00
Adrien FerrandandJoona Hoikkala 889aeb31df Validate OCSP responses in case an explicit responder is designated (#7054)
* Validate OCSP response for responders that are not the certificate's issuer.

* Improve OCSP tests using a issuer/responder pair for OCSP responses

* Clean code

* Update ocsp_test.py

* Add various comments

* Add several cases of ocsp responder. More factories for the resilience tests.

* Update ocsp_test.py
2019-06-03 22:55:26 +03:00
Felix LechnerandBrad Warren 641aba68b1 Ignore editor backups when running hooks. (#7109)
* Ignore editor backups when running hooks.

When processing hooks, certbot also runs editor backups even though
such files are outdated, clearly warranted correction and may quite
possibly be defective.

That behavior could lead to unexpected breakage, and perhaps even pose
security risks---for example, if a previous script was careless with
file permissions. As an aggravating factor, the backup runs after the
corrected version and could unintentionally override a fix the user
thought was properly implemented.

This commit causes editor backup files ending in tilde (~) to be
excluded when running hooks.

Additional information can be found here:

https://github.com/certbot/certbot/issues/7107
https://community.letsencrypt.org/t/editor-backup-files-executed-as-renewal-hooks/94750

* Add unit test for hook scripts with filenames ending in tilde.

* Provide changelog entry for not running hook scripts ending in tilde.

* Add Felix Lechner to the list of contributors.
2019-05-30 15:02:15 -07:00
Adrien FerrandandBrad Warren 7d35f95293 Avoid to delete both webroot_map and webroot_path (#7095)
* Always restore webroot_path in renewal config.

* Add unit tests to ensure correct behavior

* Add changelog

* Add certbot as modified package
2019-05-28 15:16:12 -07:00
Brad WarrenandGitHub d1330efe41 Print warning when certbot-auto has insecure permissions. (#6995)
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.
2019-04-30 10:45:03 -07:00
ohemorangeandBrad Warren c99079fb0a Warn install users that future versions of certbot will automatically redirect (#6976)
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
2019-04-26 12:43:09 -07:00
Adrien FerrandandBrad Warren d5de24d9fc [Windows] Security model for files permissions - STEP 2 (#6895)
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
2019-04-12 13:32:51 -07:00
Adrien Ferrand 04152c21b5 Update to Pylint 1.9.4 and corrections 2019-04-09 09:22:19 +02:00
Adrien Ferrand 4515a52d3f Merge branch 'master' into pylint
# Conflicts:
#	acme/acme/client.py
#	acme/acme/crypto_util.py
#	acme/acme/standalone.py
#	certbot-apache/certbot_apache/configurator.py
#	certbot-apache/certbot_apache/parser.py
#	certbot-apache/certbot_apache/tests/tls_sni_01_test.py
#	certbot-apache/certbot_apache/tests/util.py
#	certbot-apache/certbot_apache/tls_sni_01.py
#	certbot-nginx/certbot_nginx/configurator.py
#	certbot-nginx/certbot_nginx/parser.py
#	certbot-nginx/certbot_nginx/tests/util.py
#	certbot/account.py
#	certbot/cert_manager.py
#	certbot/cli.py
#	certbot/configuration.py
#	certbot/main.py
#	certbot/ocsp.py
#	certbot/plugins/dns_common_lexicon.py
#	certbot/plugins/standalone.py
#	certbot/plugins/util.py
#	certbot/plugins/webroot.py
#	certbot/tests/auth_handler_test.py
#	certbot/tests/cert_manager_test.py
#	certbot/tests/display/util_test.py
#	certbot/tests/main_test.py
#	certbot/tests/util.py
#	certbot/util.py
#	tox.ini
2019-04-02 22:32:01 +02:00
Adrien FerrandandBrad Warren 232e0ea50f Rely on universal newline mode on python 3 for windows (#6866) 2019-04-01 09:50:08 -07:00
Adrien FerrandandBrad Warren ea568d4dc2 [Windows] Fix ErrorHandler tests, by disabling signal error handling (#6868)
This PR is a part of the effort to remove the last broken unit tests in certbot codebase for Windows, as described in #6850.

It solves the problems associated to ErrorHandler in Windows (enlighted by tests errors) by ... wipping out the problem: no signal is handled by ErrorHandler on Windows. See the relevant inline comment in certbot.error_handler for explanation and sources.
2019-03-28 16:50:42 -07:00
Adrien FerrandandBrad Warren 6ce6c67932 [Windows] Security model for files permissions - STEP 1 (#6893)
This PR is the first part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)

This step 1 refactor existing certbot.compat module into certbot.compat.misc, without any logic changed. Package certbot.compat will host the new modules that constitute the security model for Windows.

* Create the certbot.compat package. Move logic in certbot.compat.misc

* Add doc

* Fix lint

* Correct mypy

* Update client.py
2019-03-28 15:51:48 -07:00
Adrien FerrandandBrad Warren 821bec6997 Remove tls-sni related flags in cli. Add a deprecation warning instead. (#6853)
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
2019-03-26 17:46:32 -07:00
Adrien FerrandandBrad Warren 537bffbc23 [Windows] Fix some unit tests (#6865)
This PR is a part of the effort to remove the last broken unit tests in certbot codebase for Windows, as described in #6850.

This PR fixes various unit tests on Windows, whose resolution was only to modify some logic in the tests, or minor changes in certbot codecase impacting Windows only (like handling correctly paths with DOS-style).

* Correct several tests

* Skip test definitively

* Test to be reactivated with #6497

* Mock log system to avoid errors due to multiple calls to main in main_test

* Simplify mock

* Update cli_test.py

* One test to be repaired when windows file permissions PR is merged
2019-03-25 12:56:28 -07:00
Adrien FerrandandBrad Warren 3ed3787bd8 Implement Retry-After, and refactor authorization polling (#6766)
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
2019-03-01 17:03:33 -08:00
Brad WarrenandAdrien Ferrand c7f8f15e9b Fix spurious test_relevant_values* failures (#6799)
* Mock set_by_cli in _test_relevant_values_common.

* Empty commit

* Revert "Mock set_by_cli in _test_relevant_values_common."

This reverts commit 9dfec8dfa9.

* mock less

* Use plugin_common code instead of reimplementing.

* Revert 2nd implementation.

* Simplify certbot.storage.relevant_values() tests.

In addition to cleaning up the code a bit, it also removes the problems we've
seen in these tests with the global state used in cli.py.
2019-03-02 01:04:26 +01:00
Adrien FerrandandGitHub 9c405a3cd1 Fix cryptography OCSP support (#6751)
* 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
2019-02-28 00:16:52 +01:00
Brad WarrenandJoona Hoikkala 31b4b8e57c Log the execution of manual hooks (#6788)
* Move logging to execute and fix tests.

* update changelog
2019-02-22 16:42:01 +02:00
Adrien FerrandandBrad Warren b10ceb7d90 Fix test sdists with atexit handlers (#6769)
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
2019-02-21 16:55:08 -08:00
Adrien FerrandandBrad Warren 209a0c4d2c [Windows] Refactor lock_and_call using queues (#6778)
* Refactor lock_and_call using queues

* Update util.py

* Replace queue by event

* Add comments

* Update certbot/tests/util.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Update certbot/tests/util.py

Co-Authored-By: adferrand <adferrand@users.noreply.github.com>

* Add control on timeout
2019-02-19 15:15:06 -08:00
Adrien FerrandandBrad Warren 0489ca5888 [Windows] Fixes lock_and_call test method (#6772)
The method `lock_and_call`, in `certbot.tests.util` is designed to acquire a lock on a foreign process, then execute a callable in the current process. This is done to closely reproduce the lock mechanism involved between two certbot instances that are running in parallel.

This method uses the `multiprocessing` module. But its implementation in `lock_and_call` is broken for Windows: the two processes fail to communicate, leading to a deadlock.

In fact, `multiprocessing` module is using the fork mechanism on Linux, and the spawn mechanism on Windows, leading to behavior inconsistencies between the two platforms.

As this method is for tests, and not for production code, I did not try to make two implementations "by the book", one suitable for Windows, the other for Linux, like for the `certbot.lock` module.

Instead, I use a `subprocess` approach with a trigger file allowing to coordinate the current process and the subprocess. With this, `lock_and_call` is running from the same code both on Linux and Windows.

Relevant tests in the `certbot.tests.lock_test` test module are now enabled for Windows.

* Implement new lock_and_call method

* Reactivate tests for Windows
2019-02-15 18:51:22 -08:00
Adrien FerrandandBrad Warren e40d929e80 [Windows|Unix] New platform independent locking mechanism - the revenge (#6663)
First PR about this issue, #6440, involved to much refactoring to ensure a correct behavior on Linux and installer plugins.

This PR proposes a new implementation of a lock mechanism for Linux and Windows, without implying a refactoring. It takes strictly the existing behavior for Linux, and add the appropriate logic for Windows. The `lock` module formalizes two independant mechanism dedicated to each platform, to improve maintainability.

Tests related to locking are re-activated for Windows, or definitively skipped because of irrelevancy for this platform. 6 more tests are enabled overall.

* Reimplement lock file on basic level

* Remove unused code

* Re-activate some tests

* Update doc

* Reactivate tests relevant to locks in Windows. Correct a test that was not testing what is supposed to test.

* Clean compat.

* Move close sooner in Windows lock implementation

* Add strong mypy types

* Use os.name

* Refactor lock mechanism logic

* Enable more tests

* Update lock.py

* Update lock_test.py
2019-02-14 16:55:27 -08:00
Brad WarrenandAdrien Ferrand 432e18d943 Revert "Call atexit handlers before test tearDown to remove errors on Windows (#6667)" (#6752)
This reverts commit ca25d1b66a.
2019-02-07 21:40:45 +01:00
Brad WarrenandGitHub ab79d1d44a Revert "Use built-in support for OCSP in cryptography >= 2.5 (#6603) (#6747)
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
2019-02-06 16:36:32 -08:00
Adrien FerrandandBrad Warren 2ddaf3db04 Use built-in support for OCSP in cryptography >= 2.5 (#6603)
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
2019-02-05 10:45:15 -08:00