22 Commits
Author SHA1 Message Date
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
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 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 FerrandandBrad Warren 85a859d63f Make Certbot runnable on Windows (#6296)
* Add and use a compatibility layer to allow certbot to be run on windows.

* Fix path comparison

* Corrections on compat and util for tests

* Less intrusive way to parse prefix in webroot plugin working for both linux and windows.

* Disable pylint import-error for some optional imports in compat.py

* Ensure path is normalized before prefixes are generated in webroot plugin

* Same prefixes in linux and windows, in fact root path is not needed in webroot plugin

* Check that user has administrative rights before continuing on windows (necessary for symlink creation)

* More straightforward way to test administrative rights on windows

* Try to resolve import error in travis ci

* OK. We go for full introspection to trick the ci.

* Move the administrative rights control to the certbot entrypoint

* Add comment for a really non trivial code.

* Allow some commands to be run on a shell without admin rights

* Avoid races conditions on windows for lock files

* Add sphinx doc to the compat functions.

* Remove irrelevant Windows error in the lock mechanism.

* Some corrections on compat
2018-09-08 07:34:27 -07:00
Dmitry FigolandBrad Warren 36dfd06503 Prepare certbot module for mypy check untyped defs (#6005)
* Prepare certbot module for mypy check untyped defs

* Fix #5952

* Bump mypy to version 0.600 and fix associated bugs

* Fix pylint bugs after introducing mypy

* Implement Brad's suggestions

* Reenabling pylint and adding nginx mypy back
2018-05-18 06:28:17 -07:00
sydneyliandBrad Warren d5efefd979 Re-land proper webroot directory cleanup (#5577)
* fix(webroot): clean up directories properly

* fix(webroot): undo umask in finally

* Fix for MacOS
2018-02-15 15:55:08 -08:00
sydneyliandBrad Warren 99aec1394d Revert "Proper webroot directory cleanup (#5453)" (#5574)
This reverts commit ad0a99a1f5.
2018-02-14 12:09:17 -08:00
sydneyliandBrad Warren ad0a99a1f5 Proper webroot directory cleanup (#5453)
* fix(webroot): clean up directories properly

* fix(webroot): undo umask in finally
2018-02-13 10:50:04 -08:00
Joona HoikkalaandBrad Warren 99f00d21c4 Skip menu in webroot plugin when there's nothing to choose from (#5183)
* Skip menu in webroot, when there's nothing to choose from

* Added testcase
2017-10-13 12:25:33 -07:00
Zach ShepherdandBrad Warren f51d345d5b Low-impact cleanup of IDisplay (#4818)
Remove unused help-related display code. When NcursesDisplay was
removed[1], help was deprecated. This change removes the remaining
bits and pieces of code.

Remove unused escape-related display code. When NcursesDisplay was
removed[1], escape was deprecated. This change removes the remaining
bits and pieces of code.

Remove uses of unused menu parameters.

Remove unused default_status/default_state argument from checklist.
(This seems safe because not only is it unused, the parameter has
different names in the interface and implementation)

1 - d54cb76432

Resolves #4795.
2017-06-15 17:14:38 -07:00
Zach ShepherdandBrad Warren b41472afce Enhance display.util to support input validation (#4372)
* display: support validation of user input

To avoid each caller of `display.input` and `display.directory_select`
needing to implement validation logic, this allows for a validator to be
supplied as a part of the call.

Following the existing pattern from `webroot`, this validator is expected
to throw a `Error` when it encounters invalid input. The user then
receives a `notification` is re-prompted.

Testing Done:
 * tox -e py27
 * tox -e lint

* plugins: update webroot to use display's validation functionality

This change updates the webroot plugin to use the now-built-in validation
functionality in display, reducing duplicated code.

Testing Done:
 * tox -e py27
 * tox -e lint

* display: move validation logic to ops

To avoid adding complexity to `IDisplay` methods, move validation logic
to helper methods in `display.ops`.

Testing Done:
 * tox -e py27
 * tox -e lint
2017-04-24 17:36:00 -07:00
Brad WarrenandGitHub 9c9004aff1 Stop IDisplay AssertionErrors (#4010)
Fixes #3996.

I'm pretty confident this PR solves the problem. I've audited all calls to IDisplay methods and the assertions done in certbot.display.util are now done in all our unit tests.

With that said, it wouldn't hurt to have someone else double check I didn't miss anything. The easiest way to do this is to grep for IDisplay in our code and ensure all calls to IDisplay methods are valid. This means every method call other than notification (because a notification call is always OK) either provides a value for default or force_interactive. This is defined in interfaces.py.

I've also been considering removing the assertion that's been causing us trouble here from our release. The only argument I have for not doing so is it may hinder 3rd party plugin development. When they use IDisplay, they have the same problem as we do with prompting users without a TTY. Not keeping this assertion in makes it more likely they won't notice the issue and Certbot will crash on an unsuspecting user.

With that said, none of our known 3rd party plugins use IDisplay at all.

* Provide force_interactive in _get_certname

* Use force_interactive when asking for webroot

* Factor IDisplay assertion into it's own function

* Add util.patch_get_utility()

* Allow custom path to patch_get_utiity

* Change GetEmailTest to use patch_get_utility

* Use new_callable to create new objects

* Modify tests to use patch_get_utility

* Improve FreezableMock documentation

* Add user facing error to TTY magic

* Comment out assert_valid_call

* Add test_input_assertion_fail2()
2017-01-10 16:25:33 -08:00
Brad WarrenandPeter Eckersley ae379568b1 Mitigate problems for people who run without -n (#3916)
* CLI flag for forcing interactivity

* add --force-interactive

* Add force_interactive error checking and tests

* Add force_interactive parameter to FileDisplay

* add _can_interact

* Add _return_default

* Add **unused_kwargs to NoninteractiveDisplay

* improve _return_default assertion

* Change IDisplay calls and write tests

* Document force_interactive in interfaces.py

* Don't force_interactive with a new prompt

* Warn when skipping an interaction for the first time

* add specific logger.debug message
2016-12-19 12:45:40 -08:00
Peter EckersleyandGitHub 107851ee9b Document defaults (#3863)
* Begin fixing incorrect defaults

* Fix more defaults

* Make more defaults correct

* Update cli-help.txt

(To show what this PR does)

* Lint

* Extend argparse rather than vendoring it

* lint

* Move sample User Agent generation into the same module as UA generation

* Revert cli-help.txt to previous release version

* Slightly more consistent linebreaks
2016-12-13 17:32:46 -08:00
Erica PortnoyandBrad Warren d54cb76432 Remove the curses dialog, thereby deprecating the --help and --dialog command line options (#3665)
* Remove the curses dialog, thereby deprecating the --help and --dialog command line options

* Deprecate --dialog and suppress --text
2016-10-21 15:45:57 -07:00
Yen Chi Hsuan fe76d558ed Enable unit tests of certbot core on Python 3 2016-08-06 21:09:43 +08:00
Brad Warren 57e6d1995b log louder 2016-05-24 16:32:03 -07:00
Brad Warren 420e64f039 Simplify webroot chown and rm errors 2016-05-24 13:43:45 -07:00
Brad Warren 8c9c6bd07b Merge branch 'master' into enoent 2016-05-11 08:54:16 -07:00
Brad Warren 3961b70deb s/letsencrypt/certbot 2016-04-13 16:03:59 -07:00