* find_comments implementation and AugeasCommentNode creation
* Use dummy value for ancestor
* Add NotImplementedError when calling find_comments with exact parameter
* Remove parameter 'exact' from find_comments interface
* Fix comment
This PR fixes a regression in #7337 (0.38.0) that certbot cannot run with Apache on RHEL 6.
In RHEL 6, `distro.linux_distribution()` returns `RedHatEnterpriseServer`.
In RHEL 6:
```py
>>> import distro
>>> distro.linux_distribution()
(u'RedHatEnterpriseServer', u'6.10', u'Santiago')
>>> import platform
>>> platform.linux_distribution()
('Red Hat Enterprise Linux Server', '6.10', 'Santiago')
```
In RHEL 7:
```py
>>> import distro
>>> distro.linux_distribution()
('Red Hat Enterprise Linux Server', '7.6', 'Maipo')
>>> import platform
>>> platform.linux_distribution()
('Red Hat Enterprise Linux Server', '7.6', 'Maipo')
```
* fix to run with Apache on RHEL 6
* fix docs
* DualParserNode, DualCommentNode and DualDirectiveNode implementations
* Add DualBlockNode
* Address review comments
* Address review comments
* Call the right assertion after name change
* Simplify isPass
* Add explanation to _create_matching_list pydoc
* Break when match was found
Add metadata keyword argument to the ParserNode interface, allowing the initialization of the object from contents of the metadata - if the implementation allows it. As an example, Augeas implementation needs nothing more than the Augeas DOM path of a configuration directive to be able to populate the ParserNode instance with all data relevant to the DirectiveNode.
The checks also allow skipping the otherwise required keyword arguments if metadata is provided.
* Allow creating ParserNode instances using information from metadata dictionary
* Update certbot-apache/certbot_apache/interfaces.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update certbot-apache/certbot_apache/interfaces.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Address review comments
* Fix filepath comment
* Update certbot-apache/certbot_apache/interfaces.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
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>
This PR contains the changes requested in initial pre-review comments of #7308
Move properties to class pydocs in interfaces.py
Prefer class ABC register() functionality instead of class inheritance for interface classes
Add apache implementation specific functions to interfaces
* Move class argument definitions to class pydoc
* Add apache specific functionality to the interface
* Bring inheritance back
* Define initialization for different ParserNode classes
* Add parsernode utils to check keyword arguments and document the defaults in pydoc
* Fix pydocs and make BlockNode a child of DirectiveNode
* Refine docs, and remove unused __init__ from BlockNode
* Split parsernode util tests to their own respective file
* Skip cover for dummy calls to super
* Add types to method documentation
* Add documentation for children
This PR adds OVERRIDE_CLASS in certbot-apache/entrypoint.py for Scientific Linux. Fixes#7248.
* add OVERRIDE_CLASS for Scientific Linux os name
* add entry for Scientific Linux using "scientific" as key
* Update changelog
See https://community.letsencrypt.org/t/ssl-error-after-cert-renew/99430.
The first commit of this PR is a simple, clean revert of #7191. Subsequent commits add back pieces of that PR we want to keep.
I also reverted #7299 which landed in a separate PR, but needs to be reverted to keep including the TLS config files in the certbot-apache package when it is built.
I tested this on Ubuntu 18.04 by installing a cert to Apache using Certbot master and then running certbot renew with this branch. I watched the Apache plugin update the configuration file to remove SSLSessionTickets off.
* Revert "Disable TLS session tickets for Apache 2.4.11+ (#7191)"
This reverts commit 9174c631d9.
* Keep hashes with TLS session tickets disabled.
* dont delete changelog entries
* add changelog entry
* Revert "Clean the useless entries in MANIFEST.in (#7299)"
This reverts commit f4d17d9a6b.
* Implement the logic
* Update tests
* Fix lint and changelog
* Update configurator.py
* Move the TLS configs in a dedicated folder. Fix the formalism of their naming and location.
* Improve existing test to check all TLS config have their hash registered in Certbot
* Corrections after review
* Improve a test
* Remove commented useless lines in TLS configs
* Add a nice warning. Because I am nice.
* Fix lint
* Add a test
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
This PR is a part of the actions necessary to make Certbot-CI work on Windows, in order to execute the integration tests on this platform.
Following #7156, this PR changes how the integration tests are setup against Pebble to not need Docker anymore.
As a reminder, one can check #7156 and letsencrypt/pebble#240 to see the rationale about why using Docker is a problem to run the integration tests on Windows.
Basically, this PR executes directly Pebble using its executable, since it is build using Go, and Go produces self-contained executable that can run without any installation on Linux and on Windows. During the integration tests setup, Certbot-CI will get the Pebble (and Challtestsrv) executables for the defined target version on the GitHub releases. The binaries are persisted on the filesystem, so it is not needed to download them again on the second integration tests execution. Nonetheless, we are talking about 20MB of executables.
Since the setup needs to hold a state, I also took this occasion to refactor the acme_server, in order to use on object oriented approach and improve the readability/maintainability.
Once this PR and #7156 are merged, Docker will not be needed anymore for the main integration tests usecase, that is to use Pebble.
* Complete process
* Fix nginx cert path
* Check conditionnally docker
* Update gitignore, fix apacheconftest
* Full object
* Carriage return
* Move to official v2.1.0 of pebble
* Fix name
* Update acme_server.py
* Relaunch CI
* Update certbot-ci/certbot_integration_tests/utils/acme_server.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update certbot-ci/certbot_integration_tests/utils/acme_server.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update docstring
* Update documentation
* Configure a stdout to ACMEServer
* Map all process through defined stdout
* Remove unused variable
* Handle using signals
* Use failsafe entering context
* Remove failsafe rmtree, that is not needed anymore
This pull request moves the functionality within `AugeasConfigurator` that previously existed as a parent class of `ApacheConfigurator` to `ApacheConfigurator` and `ApacheParser` accordingly.
Most of the methods were moved as-is, and one (`recovery_routine()`) was completely removed. Few of the methods had to be split between the configurator and parser, good example of this is `save()`.
The Augeas object now lives completely within the `ApacheParser`.
* Remove augeasconfigurator
* Fix references
* Adjust tests accordingly
* Simplify test
* Address review comments
* Address review comments
* Move test_recovery_routine_reload
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
Since #7073 for Certbot and letsencrypt/boulder@3918714 for Boulder have landed, the bash scripts that remained after certbot-ci are not useful anymore outside of Certbot.
Only remaining place is the apacheconftest-with-pebble tox target, which leverages pebble-fetch.py script to expose a running ACME server to the apache-conf-test script.
This PR refactor apacheconftest-with-pebble to use certbot-ci instead. Finally, this PR remove the remaining integration tests bash scripts, that are _common.sh, boulder-fetch.py and pebble-fetch.py.
* Disconnect common and boulder-fetch
* Prepare reconnection of apacheconftest to new pebble deployment logic
* Finish the configuration for apacheconftest
* Add executable flag to python script
* Fix shebang
* Delete pebble-fetch.sh
This PR is the second part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)
This PR creates the module certbot.compat.os, that delegates everything to os, and that will be the safeguard against problematic methods of the standard module. On top of that, a quality check wrapper is called in the lint tox environment. This wrapper calls pylint and ensures that standard os module is no used directly in the certbot codebase.
Finally local oldest requirements are updated to ensure that tests will take the new logic when running.
* Add executable permissions
* Add the delegate certbot.compat.os module, add check coding style to enforce usage of certbot.compat.os instead of standard os
* Load certbot.compat.os instead of os
* Move existing compat test
* Update local oldest requirements
* Import sys
* Update account_test.py
* Update os.py
* Update os.py
* Update local oldest requirements
* Implement the new linter_plugin
* Fix local oldest for nginx
* Remove check coding style
* Update linter_plugin.py
* Add several comments
* Update the setup.py
* Add documentation
* Update acme dependencies
* Update certbot/compat/os.py
* Update docs/contributing.rst
* Update linter_plugin.py
* Handle os.path. Simplify checker.
* Add a comment to a reference implementation
* Update changelog
* Fix module registering
* Update docs/contributing.rst
* Update config and changelog
This PR adds a step to Apache plugin config_test when run on Fedora. Because Fedora now creates self signed certificate and related key material upon first startup of httpd. This was causing issues for users who run certbot-auto or install certbot (and mod_ssl) and run Certbot directly after.
Fixes: #6828
* Try to restart httpd on Fedora if config check fails
* Update CHANGELOG.md
In CentOS 6 default httpd configuration, the `LoadModule ssl_module ...` is handled in `conf.d/ssl.conf`. As the `VirtualHost` configuration files in `conf.d/` are loaded in alphabetical order, this means that all files that have `<IfModule mod_ssl.c>` and are loaded before `ssl.conf` are effectively ignored. This PR moves the `LoadModule ssl_module` to the main `httpd.conf` while leaving a conditional `LoadModule` directive in `ssl.conf`.
Features
- Reads the module configuration from `ssl.conf` in case some modifications to paths have been made by the user.
- Falls back to default paths if the directive doesn't exist.
- Moves the `LoadModule` directive in `ssl.conf` inside `<IfModule !mod_ssl.c>` to avoid printing warning messages of duplicate module loads.
- Adds `LoadModule ssl_module` inside of `<IfModule !mod_ssl.c>` to the top of the main `httpd.conf`.
- Ensures that these modifications are not made multiple times.
Fixes: #6606
* Fix CentOS6 installer issue
* Changelog entry
* Address review comments
* Do not enable mod_ssl if multiple different values were found
* Add test comment
* Address rest of the review comments
* Address review comments
* Better ifmodule argument checking
* Test fixes
* Make linter happy
* Raise an exception when differing LoadModule ssl_module statements are found
* If IfModule !mod_ssl.c with LoadModule ssl_module already exists in Augeas path, do not create new LoadModule directive
* Do not use deprecated assertion functions
* Address review comments
* Kick tests
* Revert "Kick tests"
This reverts commit 967bb574c2.
* Address review comments
* Add pydoc return value to create_ifmod
Add a new test to make sure that we are covering all the branches of get_virtual_hosts() regardless of the order that Augeas returns the found VirtualHost paths.
Fixes: #6813
* Add a test to ensure test coverage regardless of the order of returned vhosts
* Use deepcopy instead, and increase coverage requirement back to 100%
To fix one of the two uncovered lines in certbot-apache, given in #6880. Instead of adding a test to just increase the coverage, this fixes the uncovered line by removing the unused code.
* Add a dedicated configuration to define what is the HTTPS port for this certbot instance.
* Remove tls-sni in apache plugin
* Update constants.py
* Update interfaces.py
* Remove option
* Simplify a test
Apache plugin will now use command line default values from `ApacheConfingurator.OS_DEFAULTS` instead of respective distribution override when `CERTBOT_DOCS=1` environment variable is present.
Fixes: #6234
* Apache: respect CERTBOT_DOCS environment variable
* Move the tests to apache plugin
Attempts to configure all of the following VirtualHosts for answering the HTTP challenge:
* VirtualHosts that have the requested domain name in either `ServerName` or `ServerAlias` directive.
* VirtualHosts that have a wildcard name that would match the requested domain name.
This also applies to HTTPS VirtualHosts, making Apache plugin able to handle cases where HTTP redirection takes place in reverse proxy or similar, before reaching the Apache HTTPD.
Even though also HTTPS VirtualHosts are selected, Apache plugin tries to ensure that at least one of the selected VirtualHosts listens to HTTP-01 port (configured with `--http-01-port` CLI option). So in a case where only HTTPS VirtualHosts exist, but user wants to configure those, `--http-01-port` parameter needs to be set for the port configured to the HTTPS VirtualHost(s).
Fixes: #6730
* Select all matching VirtualHosts for HTTP-01 challenges instead of just one
* Finalize PR and add tests
* Changelog entry
This will immediately address the breakage reported in #6682 and tracked at #6685. Pip 19.0.0 and 19.0.1 don't allow commas in filenames, so don't use commas in filenames in certbot-apache test code.
I've confirmed that this fixes the issue on a machine that fails with the version of certbot-auto currently in master: recent version of virtualenv, python 2.7.
Steps to test:
push master to test box
run tools/venv.py
activate venv
pip --version: 19.0.1
pip install ./certbot-apache/: fails
push branch code to test box
confirm pip --version still 19.0.1
pip install ./certbot-apache/: success
* Rename old,default.conf to old-and-default.conf
* Update changelog
* sites-enabled should contain a symlink to sites-available
Fixes#6585.
I wrote up three suggestions for fixing this at https://github.com/certbot/certbot/issues/6585#issuecomment-448054502. I took the middle approach of requiring the user to provide an ACME server to use. I like this better than the other approaches which were:
> Resolve#5938 instead of this issue.
There is value in these tests as is over the compatibility tests in that they don't use Docker and run on different OSes.
> Spin up a local Python server to return the directory object.
Trying to set up a dummy ACME server seemed hacky and error prone.
Other notes about this PR are:
* I put the Pebble setup in `tox.ini` rather than `.travis.yml` as this seems much cleaner and more natural.
* I created a new `tox` environment called `apacheconftest-with-pebble` that reuses the code from `testenv:apacheconftest` so `apacheconftest` can continue to be used with servers other than Pebble like is done in our test farm tests.
* I chose the environment variable `SERVER` for consistency with our integration tests. I chose to not give this environment variable a default but to fail fast when it is not set.
* I ran test farm tests on this PR and they passed.
* 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
