16 Commits
Author SHA1 Message Date
Adrien FerrandandBrad Warren 9e5bca4bbf Lint certbot code on Python 3, and update Pylint to the latest version (#7551)
Part of #7550

This PR makes appropriate corrections to run pylint on Python 3.

Why not keeping the dependencies unchanged and just run pylint on Python 3?
Because the old version of pylint breaks horribly on Python 3 because of unsupported version of astroid.

Why updating pylint + astroid to the latest version ?
Because this version only fixes some internal errors occuring during the lint of Certbot code, and is also ready to run gracefully on Python 3.8.

Why upgrading mypy ?
Because the old version does not support the new version of astroid required to run pylint correctly.

Why not upgrading mypy to its latest version ?
Because this latest version includes a new typshed version, that adds a lot of new type definitions, and brings dozens of new errors on the Certbot codebase. I would like to fix that in a future PR.

That said so, the work has been to find the correct set of new dependency versions, then configure pylint for sane configuration errors in our situation, disable irrelevant lintings errors, then fixing (or ignoring for good reason) the remaining mypy errors.

I also made PyLint and MyPy checks run correctly on Windows.

* Start configuration

* Reconfigure travis

* Suspend a check specific to python 3. Start fixing code.

* Repair call_args

* Fix return + elif lints

* Reconfigure development to run mainly on python3

* Remove incompatible Python 3.4 jobs

* Suspend pylint in some assertions

* Remove pylint in dev

* Take first mypy that supports typed-ast>=1.4.0 to limit the migration path

* Various return + else lint errors

* Find a set of deps that is working with current mypy version

* Update local oldest requirements

* Remove all current pylint errors

* Rebuild letsencrypt-auto

* Update mypy to fix pylint with new astroid version, and fix mypy issues

* Explain type: ignore

* Reconfigure tox, fix none path

* Simplify pinning

* Remove useless directive

* Remove debugging code

* Remove continue

* Update requirements

* Disable unsubscriptable-object check

* Disable one check, enabling two more

* Plug certbot dev version for oldest requirements

* Remove useless disable directives

* Remove useless no-member disable

* Remove no-else-* checks. Use elif in symetric branches.

* Add back assertion

* Add new line

* Remove unused pylint disable

* Remove other pylint disable
2019-12-10 14:12:50 -08:00
Adrien Ferrandandohemorange e048da1e38 Reorganize imports (#7616)
* Isort execution

* Fix pylint, adapt coverage

* New isort

* Fix magic_typing lint

* Second round

* Fix pylint

* Third round. Store isort configuration

* Fix latest mistakes

* Other fixes

* Add newline

* Fix lint errors
2019-12-09 15:50:20 -05:00
ohemorangeandBrad Warren 96e02d614b Make uncomplicated modules private (#7528)
* Create _internal package for Certbot's non-public modules

* Move account.py to _internal

* Move auth_handler.py to _internal

* Move cert_manager.py to _internal

* Move client.py to _internal

* Move error_handler.py to _internal

* Move lock.py to _internal

* Move main.py to _internal

* Move notify.py to _internal

* Move ocsp.py to _internal

* Move renewal.py to _internal

* Move reporter.py to _internal

* Move storage.py to _internal

* Move updater.py to _internal

* update apache and nginx oldest requirements

* Keep the lock file as certbot.lock

* nginx oldest tests still need to rely on newer certbot

* python doesn't have good dependency resolution, so specify the transitive dependency

* update required minimum versions in nginx setup.py
2019-11-08 16:19:21 -08:00
Adrien FerrandandBrad Warren ec3ec9068c Upgrade to pywin32>=225 and fix unit tests (#7429)
Fixes #7426
2019-10-08 16:17:08 -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
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 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
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
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 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