this PR finally removes all uses of self-signed certificates from
certbot-nginx
i plan to create one last PR related to this deprecating
`acme.crypto_util.make_self_signed_cert` and moving the function to
certbot-compatibility-test which is the only place it's currently used
i think we could also do additional refactoring in certbot-nginx by
moving the _make_server_ssl call out of choose_or_make_vhost and make
deploy_cert responsible for calling it if the returned vhosts aren't
ssl. in this case, we could then skip updating cert and key directives a
second time as this is duplicate work if we just made the server ssl
i considered doing this, but it's a bigger refactor, breaks more tests,
and i'm not sure it really buys us much so i skipped it. i could take
this on or create an issue for it if you think it's important for us to
do for some reason tho ohemorange
this is another tiny piece of my nginx refactoring. with
https://github.com/certbot/certbot/pull/10455, this function is now
never called outside of tests with `create_if_no_match=False` so this PR
removes the unnecessary parameter
luckily, tests still Just Work™ with this change
Fixes https://github.com/certbot/certbot/issues/6180.
New output:
```
--deploy-hook DEPLOY_HOOK
Command to be run in a shell once for each successfully issued certificate, including on subsequent renewals. Unless --disable-hook-validation is
used, the command’s first word must be the absolute pathname of an executable or one found via the PATH environment variable. For this command, the
shell variable $RENEWED_LINEAGE will point to the config live subdirectory (for example, "/etc/letsencrypt/live/example.com") containing the new
certificates and keys; the shell variable $RENEWED_DOMAINS will contain a space-delimited list of renewed certificate domains (for example,
"example.com www.example.com") (default: None)
```
Pre and post hooks are still only shown in `renew` and `reconfigure`
help, though perhaps there is less confusion over those so it's not
necessary.
this is the first part of the nginx refactoring work i wanted to do.
ohemorange, if this conflicts with your work on updating our nginx ssl
config, please feel free to either ignore this for now or point me to
your branch after merging this and i can fix up any merge conflicts
myself like i previously offered
the main thing this PR does is create a new choose_or_make_vhosts
function with the current choose_vhosts behavior and makes choose_vhosts
only return existing ssl vhosts which i think is the behavior we want
when setting up HSTS or OCSP stapling. [this is what we do in
apache](867b499f9b/certbot-apache/src/certbot_apache/_internal/configurator.py (L1795-L1829)),
enabling HSTS or OCSP stapling on an HTTP vhost seems wrong/dangerous,
and since we don't have cert and key information in these [enhance
calls](867b499f9b/certbot/src/certbot/interfaces.py (L255)),
any SSL vhost we create will be left with snakeoil certs which also
seems very wrong
of course, this simple change to certbot-nginx's prod code required many
changes to its tests. the config file for headers.com was introduced
[here](https://github.com/certbot/certbot/pull/6068) specifically for
testing HSTS so i added the SSL configuration it needs to make work with
the choose_vhost changes. that then broke the IP tests for headers.com
that were added in https://github.com/certbot/certbot/pull/10145/ so i
created a new no-listens.com vhost for testing that
if this is merged, my plan in the next PR or two is to:
1. since choose_or_make_vhosts is now always called with
create_if_no_match=True in prod code, i plan to remove that variable,
make that the default behavior of the function, and fix up tests
2. then, since choose_or_make_vhosts is only called in deploy_cert, i
plan to pass the cert and key to it so it can be used in
_make_server_ssl instead of the dummy certs currently being used there
i could do more of this in this PR if you want ohemorange, but i figured
it rarely hurts to break things up especially when the code is kind of
tricky like it is (to me) here
this is the quickest somewhat sane fix for the current CI problem that i
could think of
the domain being used in the self-signed cert really seems irrelevant.
on my main test VPS, socket.gethostname returns "brad-certbot-dev". on
my laptop, it's "MacBookPro". i manually tested this branch a bit on my
VPS and nginx seems content
using a simple static string like this seems unlikely to break anything
to me and i think helps clearly identify where the self-signed cert is
coming from if ever causes a problem for anyone in the future
fixes#10404
unfortunately, exactly what `python setup.py clean` did doesn't seem
well documented so i dug into the code with a debugger. executing the
`clean` subcommand is done by [this
code](9cc2f5c05c/setuptools/_distutils/command/clean.py (L54-L77))
where the relevant build variables are set by the `build` subcommand
[here](9cc2f5c05c/setuptools/_distutils/command/build.py (L52))
and
[here](9cc2f5c05c/setuptools/_distutils/command/build.py (L112)).
it turns out us running `python setup.py clean` was already redundant
with `rm -rf build` on the next line
i built two releases, one on the latest commit in this PR and another on
44f1dd677b
before the switch to `python -m build`. a simple diff of the resulting
tarballs and wheels fails, presumably because of metadata differences,
but after untaring or unzipping the files, the contents are identical
for all of our built packages
Several of these have been fixed, so let's update the requirement if
necessary and remove the warning catching.
`python-dateutil 2.9.0` was released Feb 29, 2024, so it's not widely
packaged in non-EOL major distros yet.
`pytest-cov 4.1.0` was released May 24, 2023.
Our pinned versions were already higher than these requirements.
Alternatively, we could just remove the warnings and not update the
minimum requirement, but I think it's nicer to note it in requirements
for anyone running our tests, like packagers.
We already require `poetry-plugin-export>=1.9.0`. `1.7.0` updated its
`requests-toolbelt` requirement to `>=1.0.0`, which is greater than the
minimum version needed to remove the warning.
It's a [drop-in
replacement](https://docs.astral.sh/uv/pip/compatibility/) that speeds
things up. I don't see any reason why not.
`--use-pep517` is [set by default](
https://docs.astral.sh/uv/pip/compatibility/#pep-517-build-isolation),
so we don't need it.
`--disable-pip-version-check` also does nothing on uv.
`uv` [uses
separate](https://docs.astral.sh/uv/pip/compatibility/#build-constraints)
`UV_BUILD_CONSTRAINT` and `UV_CONSTRAINT`. I just added it to both to do
the simplest thing here. We could split them.
We probably don't actually need to pipstrap pip anymore, I could take
that out.
What's happening with `parsedatetime` and `python-digitalocean` is that
they were always secretly wrong. Since `pip` compiles bytecode by
default, it was suppressing the errors. If you add the
`--compile-bytecode` flag to `uv`, it passes, but I don't think we
should do that. You can see the failure happen on main by passing
`--no-compile` to the pip args and running `certbot -r -e oldest`.
Now what I don't understand is that some places seem to say the `'\/'`
error from `parsedatetime` only started in python 3.12, whereas others
see it on earlier python. Perhaps pytest is vendorizing python or
something. Not too worried about that, needed to get updated anyway, and
it's an accurate oldest version based on our oldest OSes.
`python-digitalocean` is techincally newer than debian 11, but we've
made that decision before so it seems fine to me.
This uses the new `storage.atomic_rewrite` function to write only the
ARI-related value to the config file, leaving other values the same.
Updates `storage.atomic_rewrite` to handle the case where the
`"renewalparams"` section might be empty (which occurs in the new
unittests), and adds some comments.
Note: `atomic_rewrite` is mocked out as a no-op in
`test_resilient_ari_directory_fetches` and `test_renew_before_expiry`
because those tests have a simplistic config object on their
mock_renewable_cert that doesn't include a filename. If we try to write
the config in those tests we'd get an error (trying to write to `None`).
Since those tests aren't intended to test the "store and obey
Retry-After" behavior, I figured it's reasonable to skip it in the name
of testing; but of course, open to idea about the best way to navigate
this.
Part of #10355
Part of https://github.com/certbot/certbot/issues/10403
We were never actually updating the versions in certbot-ci and letstest.
Not that it really matters, but let's do that there as well.
Final part of https://github.com/certbot/certbot/issues/10403
I tested running `tools/snap/generate_dnsplugins_snapcraft.sh
certbot-dns-dnsimple` and it put the correct description in to the
`snapcraft.yaml` file.
Part of https://github.com/certbot/certbot/issues/10403.
As far as I can tell, "stick it in setup.py" is the official way of
handling complex dependencies. But since the version is static, we have
a little more choice here than we had with `certbot/pyproject.toml`.
We could put the version in the respective `pyproject.toml`s and read it
directly from the toml file with something like
[this](https://stackoverflow.com/a/78082561). Or otherwise load and
parse that file. The benefit of doing it that way is that all
non-certbot versions would be canonically in the `pyproject.toml`, and
also if we wanted we could use that same toml parsing to change the
version at release time instead of `sed`. I actually suspect `acme`,
`certbot-ci`, and `certbot-compatibility-test` will be the only ones
where we can completely delete `setup.py`, as the others all have
lockstep dependencies. (side note - we just never update `certbot-ci`
version. it's still set at `0.32.0.dev0`. there's no way this matters
but just noting.) I chose to do it this way instead because it seems
cleaner since we have to keep `setup.py` around anyway, but I don't have
a strong preference.
Based on what I've read, there's not actually a clean way to grab and
insert the version number within the toml file. This is due to [design
decisions](https://github.com/toml-lang/toml/issues/77) by the toml
authors. The clean `all` extras specification that we used in
`certbot/pyproject.toml` [seems to be an
outlier](https://github.com/pypa/setuptools/discussions/3627#discussioncomment-6476654)
because it's pip handling the self-reference, not toml.
This was causing oldest tests to fail on my mac, which has an open file
limit of 256. Locks were being released at exit, but there were more
than 256 tests being run at once. Holding onto the file descriptor for
temporary files was making us keep the files open.
I also removed unnecessary setUps and tearDowns in subclasses so that
this could be fixed in only one spot.
If you wanted to do any testing locally, I was throwing this in places:
```
import errno, os, resource
open_file_handles = []
for fd in range(resource.getrlimit(resource.RLIMIT_NOFILE)[0]):
try: os.fstat(fd)
except OSError as e:
if e.errno == errno.EBADF: continue
open_file_handles.append(fd)
print(f'location description: {len(open_file_handles)}')
```
Alternative to https://github.com/certbot/certbot/pull/10408/ and
https://github.com/certbot/certbot/pull/10415/ that fixes production
code for account meta and puts autouse fixtures in certbot and acme
tests. Overrides all `time.sleep` calls while we're at it.
Fixes the production code where it's simple/clean, and fixes the tests
for HTTPServer-based code because we just don't have that many mac users
using standalone.
The responsibility for atomic updates to a config file was previously
spread across different functions. This moves the atomic update to the
lowest level of the call graph.
Also, factor out the code that creates a ConfigObj based on various
inputs (archive dir, cert locations, and renewal params). This allows
cleanly reusing it across the "update" and "create new" paths.
Now the "create new config" code path doesn't have to do any renaming,
and the "update config" code path can assume the input file exists (and
error if it doesn't).
This will make it easier and cleaner to reuse the config-writing code in
#10377.
Part of #10355
this is the final part of
https://github.com/certbot/certbot/issues/10195. this fixes
https://github.com/certbot/certbot/issues/10195
the changes in the first commit were done automatically with the
command:
```
ruff check --fix --extend-select UP006 --unsafe-fixes
```
the second commit configures ruff to check for this to avoid regressions
thanks for bearing with me thru these somewhat large automatically
generated PRs ohemorange 🙏
this is another part of https://github.com/certbot/certbot/issues/10195
these changes were done automatically with the command:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot-apache
```
This PR is modeled on https://github.com/certbot/certbot/pull/10373/,
and is part of https://github.com/certbot/certbot/issues/10183.
relevant requirements:
use_tls13 >= 1.13.0
session_tix_off implemented: nginx >= 1.5.9 and openssl_version >=
1.0.2l
session tix off by default: >= 1.23.2
oldest non-deprecated major distros nginx versions:
debian 11 1.18.0
epel 8 1.23.1
ubuntu 22.04 1.18.0
Therefore, we can stop testing for use tls 13 and session tix off
allowed, with the same caveat as [in this
comment](https://github.com/certbot/certbot/pull/10373#issuecomment-3134101604).
While we could add a new split for configs that don't require session
tickets off to be set explicitly since it's the default, I don't think
it's worth doing now. I added a note in the comments about this.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
this is another part of https://github.com/certbot/certbot/issues/10195
the first commit was done automatically with the command:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot/src/certbot/_internal
```
this was unfortunately insufficient as it left a line in webroot.py with
over 100 characters. i fixed this manually in my second commit
this is another part of https://github.com/certbot/certbot/issues/10195
these changes were done automatically with the commands:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot
git checkout certbot/src/certbot/_internal
```
fixing up the files under certbot/src/certbot/_internal will be done in
another PR
This sets up a `pyproject.toml` file for certbot, initially generated
[using](https://hatch.pypa.io/latest/intro/#existing-project) `hatch new
--init` and modifying from there. Since we dynamically require acme of a
matching version, I kept that around in `setup.py` to do the simplest
thing in this PR.
Other possible (future) implementations:
- setuptools has a beta implementation to read from a
`requirements.txt`. we could generate one of those.
- we could just hardcode it and update at release time. I like not
having to keep the version up to date in various places but maybe it's
actually fine
- something something integration with poetry pinning?
I think the syntax for setting version dynamically in `pyproject.toml`
is much nicer than what we do in `setup.py`. It's a little silly to do
it there after we've bothered to calculate it, but I put it there in the
hopes of being able to remove it from `setup.py` someday/somehow.
It would be nice to access the version dynamically set in
`pyproject.toml` in `setup.py`, but I do not think it is likely to be
possible.
Here are some useful links regarding this migrations:
- [How to modernize a `setup.py` based
project?](https://packaging.python.org/en/latest/guides/modernize-setup-py-project/)
- [Writing your
`pyproject.toml`](https://packaging.python.org/en/latest/guides/writing-pyproject-toml/)
- [Configuring setuptools using `pyproject.toml` files](
https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html)
- [`pyproject.toml`
specification](https://packaging.python.org/en/latest/specifications/pyproject-toml/)
- [Platform compatibility
tags](https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/)
this is in response to
https://github.com/certbot/certbot/pull/10399#issuecomment-3166305086
this PR does two things:
1. it clarifies what is meant by "build dependencies" in DESIGN.md
2. fixes our workaround for
https://github.com/python-poetry/poetry/issues/4103 which broke when we
moved most of our code under `src` directories. i kept the previous `rm
-rf ${REPO_ROOT}/*/*.egg-info` line around for `letstest` and to
hopefully add some robustness for us if we ever move our code around
again
Alternative implementation for #7908.
In this PR:
- set up ruff in CI (add to `tox.ini`, mark dep in `certbot/setup.py`)
- add a `ruff.toml` that ignores particularly annoying errors. I think
line length isn't actually necessary to set with this workflow since
we're not checking it but putting it there for future usage.
- either fix or ignore the rest of the errors that come with the default
linting configuration. fixed errors are mostly unused variables. ignored
are usually where we're doing weird import things for a specific reason.
Part of #10355.
This allows combining the NamespaceConfig object with a cache of ACME
clients, so we don't have to make the whole large NamespaceConfig object
available all the way down the renewal call stack.
The new AriClientPool is responsible for instantiating ACME clients for
the purpose of ARI fetching. It provides only a simple user agent,
listing the Certbot version. The only CLI flag it observes is
--no-verify-ssl.
i wrote this in response to erica's thread at
https://opensource.eff.org/eff-open-source/pl/xtuemgdti78xfx1hn9jwbrfrjy
this hopefully helps some, but i think our logic here is unfortunately
fairly complicated which is reflected in the code comments. feel free to
suggest alternate wording or even just close this if you think our
comments currently in main are good enough
this fixes https://github.com/certbot/certbot/issues/10176 and fixes
https://github.com/certbot/certbot/issues/10257. it is based on
https://github.com/certbot/certbot/pull/10017 and ohemorange said it's
fine for me to cherry-pick their commit here
this change accomplishes a few things:
* because PYTHONPATH is no longer set to
`"$SNAP/lib/python3.12/site-packages:${PYTHONPATH}"` which evaluates to
`"$SNAP/lib/python3.12/site-packages:"` if PYTHONPATH wasn't previously
set, Certbot no longer searches for Python modules in the current
working directory which was causing #10176. i was able to reproduce this
problem with our currently released snap and verify that this change
fixes that problem
* since we no longer set PYTHONPATH at all, it won't be set in user
hooks which was causing https://github.com/certbot/certbot/issues/10257
* as an added bonus, scripts that start with `#!/usr/bin/env
/snap/certbot/current/bin/python3` as suggested
[here](https://github.com/certbot/certbot/issues/10257#issuecomment-2809421320)
are still able to find and import certbot and its dependencies so those
scripts should continue to work. i verified this is the case with manual
testing
finally, i created two news fragments here based on the text at
https://towncrier.readthedocs.io/en/stable/tutorial.html#creating-news-fragments
which says
> You can associate multiple issue numbers with a news fragment by
giving them the same contents.
when run on this PR `towncrier --draft` outputs:
```
Loading template...
Finding news fragments...
Rendering news fragments...
Draft only -- nothing has been written.
What is seen below is what would be written.
## 4.2.0.dev0 - 2025-07-31
### Changed
- Catches and ignores errors during the directory fetch for ARI checking so
that these errors do not hinder the actual certificate issuance.
([#10342](https://github.com/certbot/certbot/issues/10342))
- Removed the dependency on `pytz`.
([#10350](https://github.com/certbot/certbot/issues/10350))
### Fixed
- The Certbot snap no longer sets the environment variable PYTHONPATH stopping
it from picking up Python files in the current directory and polluting the
environment for Certbot hooks written in Python.
([#10176](https://github.com/certbot/certbot/issues/10176),
[#10257](https://github.com/certbot/certbot/issues/10257))
- Previously, we claimed to set FAILED_DOMAINS and RENEWED_DOMAINS env
variables for use by post-hooks when certificate renewals fail, but we were
not actually setting them. Now, we are.
([#10259](https://github.com/certbot/certbot/issues/10259))
- Added `--eab-hmac-alg` parameter to support custom HMAC algorithm for
External Account Binding.
([#10281](https://github.com/certbot/certbot/issues/10281))
- Certbot now always uses the server value from the renewal configuration file
for ARI checks instead of the server value from the current invocation of
Certbot. This helps prevent ARI requests from going to the wrong server if
the user changes CAs.
([#10339](https://github.com/certbot/certbot/issues/10339))
```
---------
Co-authored-by: Erica Portnoy <erica@eff.org>
blast from the past! resurrects
https://github.com/certbot/certbot/pull/9803 with all of @bmw's changes.
i figured instead of force-pushing a basically brand new branch and
obliterating the old review, i'd just start from a clean slate
fixes#8272
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@eff.org>
fixes https://github.com/certbot/certbot/issues/10336 and fixes
https://github.com/certbot/certbot/issues/10357 using the plan at
https://github.com/certbot/certbot/issues/10336#issuecomment-3109192677
while this PR makes the renewal_time function slightly less nice, i
think us catching and handling the exceptions in certbot makes the most
sense so we can do exactly what we want around terminal and file logging
with this change, a output from a failed `sudo certbot renew` run looks
like
```
$ sudo certbot renew
Saving debug log to /var/log/letsencrypt/letsencrypt.log
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Processing /etc/letsencrypt/renewal/example.org.conf
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
An error occurred requesting ACME Renewal Information (ARI). If this problem persists and you think it's a bug in Certbot, please open an issue at https://github.com/certbot/certbot/issues/new/choose.
Certificate not yet due for renewal
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
The following certificates are not due for renewal yet:
/etc/letsencrypt/live/example.org/fullchain.pem expires on 2025-10-23 (skipped)
No renewals were attempted.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
```
`sudo certbot renew -q` produces no output and the relevant messages in
the log file look like:
```
2025-07-30 19:51:13,267:WARNING:certbot._internal.renewal:An error occurred requesting ACME Renewal Information (ARI). If this problem persists and you think it's a bug in Certbot, please open an issue at https://github.com/certbot/certbot/issues/new/choose.
2025-07-30 19:51:13,267:DEBUG:certbot._internal.renewal:Error while requesting ARI was:
Traceback (most recent call last):
File "/home/brad/certbot/acme/src/acme/client.py", line 366, in renewal_time
raise ValueError('im some error')
ValueError: im some error
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/brad/certbot/certbot/src/certbot/_internal/renewal.py", line 351, in _ari_renewal_time
return acme.renewal_time(cert_pem)[0]
File "/home/brad/certbot/acme/src/acme/client.py", line 370, in renewal_time
raise errors.ARIError(error_msg, now + default_retry_after) from e
acme.errors.ARIError: ('failed to fetch renewal_info URL https://acme-staging-v02.api.letsencrypt.org/acme/renewal-info/oXQaBm1Qt4YtSizBfrSNiElszRY.LMjTHFS4HPbSRMOzLrA9OZId', datetime.datetime(2025, 7, 31, 1, 51, 13, 267088))
```
something weird happened to the changelog in
https://github.com/certbot/certbot/pull/10319. a 4.2.0 entry was added
below the entry for `5.0.0 - main` despite 4.2.0 not having been
released. since it's sounding like we're expecting our next release to
be 4.2.0 and not 5.0, i merged these two changelog entries into one for
4.2.0
i also modified our setup.py files to use 4.2.0.dev0 instead of
5.0.0.dev0 altho this isn't strictly necessary because our release
script will automatically set all version numbers to whatever version we
give it on the command line before building the release
Part of #10183
> Option 4. Stop updating old files with security improvements. If
people want to be on old software they can but then they're not getting
the nice new things. We can either warn or not warn if we see people
using them, either on certbot install (what, who's installing new
certbot on these machines), new cert, cert renewal, or certbot update.
The second two would require code changes, I'm pretty sure. I don't
think we should warn too often because that's how we get people to
silence all output. This is a little weird because we don't usually keep
around deprecated things. We could also warn loudly and see if people
complain. Or do some sort of brownout.
This PR warns every time certbot is run. We could make it run less often
(only when a new config file is installed, probably), but that's a more
extensive code change, and honestly I think it's probably fine? But I
can change it.
Fixes https://github.com/certbot/certbot/issues/10342
When doing ARI checks in acme.renewal_time, we catch RequestException
and return a default value. That's so an unavailable ARI server doesn't
cause issues.
Before we get to acme.renewal_time, we have to create an ACME client,
and in the process fetch a directory. We should make the directory fetch
similarly resilient.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
my desire to do this came from the discussion at
https://github.com/certbot/certbot/pull/10358#discussion_r2198273164
the code i'm deleting here came from
https://github.com/certbot/certbot/pull/4733
i think doing string parsing on the exception like this is convoluted
and overkill. i also agree with erica from the linked thread above that
we shouldn't be raising a ValueError here, especially when the docstring
for this function says `:raises requests.exceptions.RequestException: in
case of any problems` and doesn't mention ValueError
i prefer we do the simple thing and just delete the code. in the my
opinion unlikely event we decide polishing this important, i think we
can reconsider more complex approaches here
When making an ARI request, use the server listed in the cert's renewal
conf rather than the one passed in certbot's config.
Fixes#10339
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
https://github.com/certbot/certbot/pull/10297 modified
`letsencrypt-auto-source/letsencrypt-auto`. It should never be modified
except during a release, and we have a test to make sure of it. Old
scripts pull directly from that file on github, so let's put that back
asap.
Another token of gratitude for a super useful tool and service.
More about codespell: https://github.com/codespell-project/codespell .
I personally introduced it to dozens if not hundreds of projects already
and so far only positive feedback.
CI workflow has 'permissions' set only to 'read' so also should be safe.
---------
Signed-off-by: Yaroslav O. Halchenko <debian@onerussian.com>
Fixes https://github.com/certbot/certbot/issues/10259
This PR moves post-hook execution from `main.renew` to
`renewal.handle_renewal_request` so that failed and renewed domains
actually get passed into post-hook execution as promised, even when
failures happened.
I suspect the original PR was being overly cautious by putting the whole
thing into a try/finally so that post-hooks definitely happen, but
`handle_renewal_request` is already full of exception catching. I
understand the worry about executing a pre-hook and then failing to
execute its matching post-hook, but the code really is already
structured to make sure that that won't happen. And then when we added
`FAILED_DOMAINS` and `RENEWED_DOMAINS`, we both kept that
overly-cautious hooks execution location, but also kept the error so we
have a summary at the end...which meant that if failures happened, the
env vars were never set.
If we really want to keep the `hooks.run_saved_post_hooks` call on the
outside of everything in main, we can, but then we will have to do one
of the following:
- pass in the output lists to be filled out during execution. not my
favorite pattern
- throw the output lists in the error object or make a wrapper error,
not sure, haven't looked at `errors.py` too closely
- stop raising that final error where we report failures at the very
bottom. it's a little outdated maybe but I do like it and I think people
are used to it
- raise that error in main, returning the number of parse and renewal
failures. this is my favorite of the options, but I still like it less
than what I've implemented here.
Here's the integration/regression test failing on main:
https://dev.azure.com/certbot/certbot/_build/results?buildId=9237&view=logs&j=fca58cec-e7ce-563a-f36f-5c233894d750
You can see here that that branch just has the integration test without
the fix (and removing other tests for efficiency):
https://github.com/certbot/certbot/compare/main...test-fail-env-on-main
It's the default, but just to be clear, this should definitely have two
reviewers.
this was the wrong/misleading comment i remember erica mentioning in our
discussions yesterday. the problem here is modern versions of certbot
also always save the server url. see
31599bad83/certbot/src/certbot/_internal/storage.py (L288-L291)
i personally don't think this requires two reviews and if whoever gets
to this first agrees, i think you should feel free to merge this
I added the exact same service hook we use for nightly failures for
release failures.
<img width="1347" alt="Screenshot 2025-06-11 at 10 32 18 AM"
src="https://github.com/user-attachments/assets/b4728d0b-212b-4ecb-84c6-0ed62715f0ff"
/>
Service hooks can be viewed here:
https://dev.azure.com/certbot/certbot/_settings/serviceHooks
Now there's no reason to keep around the manual notification stage, it
wasn't working in either case anyway. Since it's literally the same as
the nightly hook, I don't personally feel the need to test the release
branch but I can if the reviewer would like.
certbot's standalone code contains confusing references to things like
`SSLSocket` which we were hoping to deprecate in
https://github.com/certbot/certbot/issues/10284. are they relevant?
they're sure not!
certbot's standalone plugin only supports HTTP-01 so comments about
things like `ACMETLSServer` and the completely unused `certs` variable
can be deleted
furthermore, the type of the different variables named things like
`http_01_resources` were wrong in multiple places. as can be seen in
certbot's standalone code, the type is
`Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]`. this is also
[the type used in acme.standalone's
tests](723fe64d4d/acme/src/acme/_internal/tests/standalone_test.py (L78-L81))
despite the file's type annotations saying it takes a different type. i
think the incorrect type annotations were never caught because mypy
can't fully make sense of our overly complex server classes here
finally, `from __future__ import annotations` was added to make [forward
references in type
annotations](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#forward-references)
easier
This is a feature people didn't have before and won't miss if it fails.
We can always raise it later, but let's reduce it for now to stop people
worrying about the big red warning.
This is one solution to https://github.com/certbot/certbot/issues/10327.
It won't test an ARI check during a dry run, since it will just avoid
the mismatch problem by checking for dry run first and returning before
checking ARI. This PR will make the big error (actually a warning, but
red and scary) go away though.
I thought https://github.com/certbot/certbot/pull/9804/ was abandoned
but the author just missed my comment. I would like to accept that PR to
get it in, but in the process of updating the PR I wrote a nicer
changelog entry, so I would like to add that.
When a CA fails to issue a certificate after finalisation Certbot
currently prints the following unhelpful message:
```
An unexpected error occurred:
acme.errors.IssuanceError
```
This PR makes Certbot print the ACME error object from the order, as
such
```
An unexpected error occurred:
CAA error :: Invalid CAA: CAA prohibits issuance
```
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [x] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `master` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [x] Add or update any documentation as needed to support the changes
in this PR.
- [x] Include your name in `AUTHORS.md` if you like.
Fixes#10312. This is perhaps overly detailed, but I was hoping that by
giving a viable path forward it would forestall requests to change it
back, add a flag to ignore ari, or otherwise change the behavior. Very
open to suggestions on wording/content/length/etc.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Previously, we were constructing an ACME client for ARI checking that
used the global value for `server`, not the one recorded in a lineage's
renewal file.
This resulted in errors in the logs and failure to observe ARI for
lineages that used a non-default `--server` (e.g. staging or non-Let's
Encrypt CAs).
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
This depends on a pending Pebble pull request and so will fail
integration tests until/unless that lands:
https://github.com/letsencrypt/pebble/pull/501
However, I'd appreciate some eyes on this PR in this regard: is the
interface we're using in Pebble useful and appropriate? If not, we can
adjust the Pebble PR.
Inspired based on conversation on
https://github.com/certbot/certbot/pull/10307, but note that this just
tests the general case; it does not test the "default server differs
from lineage server" case yet; when I try adding that I get some bugs
that may reflect a problem in #10307 I need to fix (or may reflect that
I need to inhibit the `--server` flag rather than trying to override it
late in the command line).
fixes https://github.com/certbot/certbot/issues/10308
my thinking here was if the spec forbids checking ARI for expired certs,
this check should happen directly in the renewal_time function. if we do
that, what's its most useful response? error? return None? return a
datetime in the past?
i feel the latter is most helpful. tell the caller to renew now rather
than erroring out or giving it no suggestion about when it should renew
it probably doesn't matter much, but i think this would be nice to have
for 4.1.0 as it fixes a (minor) spec compliance issue in our ARI
implementation that is being released
[Recent changes](https://github.com/certbot/certbot/pull/10272/) to
`renewal.should_autorenew` assumed that if
`RenewableCert.configuration.renew_before_expiry` was set, that means
the user set it. That's wasn't true; we were throwing in a default value
if the user didn't set it. But there's no reason for that, especially
since we now set the default renewal time dynamically. Also, we were
writing out a commented `# renew_before_expiry = 30 days` without any
further documentation, in a file that we tell users they [shouldn't
really be
editing](https://eff-certbot.readthedocs.io/en/latest/using.html#modifying-the-renewal-configuration-file).
We now do neither of those things.
this is a follow up from https://github.com/certbot/certbot/pull/10286
and related to https://github.com/certbot/certbot/issues/10302
sorry i initially missed this! in #10286 our tests were just yelling at
me about the different augeas package needed, but python headers and a
compiler are also needed for things to work with an updated version of
python-augeas
i don't believe we need this change in our macOS instructions because:
1. homebrew doesn't split up python packages the way many linux distros
do. there is no equivalent python-dev package
2. if you're using homebrew, you already have a compiler because
[homebrew requires command line tools for
xcode](https://docs.brew.sh/Installation#macos-requirements)
3. "it works on my machine"
Follow-up to #10241. The acme module code is mostly the same, except the
switch to return a tuple containing Retry-After.
This includes the CLI-side work to call out to the new `renewal_time`
method when checking for renewal.
I moved `should_autorenew` from `storage.py` into `renewal.py`, where it
fits better (and also this solves an import cycle problem). To make the
edits more visible I split this into one commit for the move and [one
commit for the subsequent
edits](4e137d9b00 (diff-fad906e31304c767d620bfd243f4c7adf1e63a3420fd634ee57a0f6651c182cf)).
This does not yet attempt to store the Retry-After info, or failure
retries, in renewal configs. I figured since that's a pretty big chunk
of work and design on its own, I wanted to get interim feedback as is. I
think this PR would be okay to land with the current default crons /
systemd timers that run twice a day. I think we should implement storage
of retry information before increasing the frequency of runs. And if the
team would like to hold off on landing any ARI until that storage is
done, I'm good with that too. 👍🏻
it looks like https://github.com/certbot/certbot/pull/10098 introduced a
couple bugs into this file:
1.
[RSAPrivateKeys](https://cryptography.io/en/latest/hazmat/primitives/asymmetric/rsa/#cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey)
don't have a `public_bytes` method
2. `cryptography.x509` wasn't imported and
[load_pem_x509_certificate](https://cryptography.io/en/latest/x509/reference/#cryptography.x509.load_pem_x509_certificate)
takes bytes, not a string
i think avoiding this is unfortunately difficult as this file has no
tests, but it was useful for me just now when testing
https://github.com/certbot/certbot/pull/10283 so i wanted to fix it up
i also changed the script to initially create the account without an
email address as the fake@example.com email causes registration with
LE's staging server to fail early in execution
with the changes in this PR changes, if you:
1. change the value of
[DOMAIN](0075104805/acme/examples/http01_example.py (L57))
to a domain pointing at your machine
2. as root, activate your certbot dev environment, and run `python
acme/examples/http01_example.py `
it will fail late in the script with:
```
Traceback (most recent call last):
File "/home/brad/certbot/acme/examples/http01_example.py", line 237, in <module>
example_http()
~~~~~~~~~~~~^^
File "/home/brad/certbot/acme/examples/http01_example.py", line 223, in example_http
regr = client_acme.update_registration(
regr.update(
...<3 lines>...
)
)
File "/home/brad/certbot/acme/src/acme/client.py", line 101, in update_registration
updated_regr = self._send_recv_regr(regr, body=body)
File "/home/brad/certbot/acme/src/acme/client.py", line 373, in _send_recv_regr
response = self._post(regr.uri, body)
File "/home/brad/certbot/acme/src/acme/client.py", line 392, in _post
return self.net.post(*args, **kwargs)
~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "/home/brad/certbot/acme/src/acme/client.py", line 766, in post
return self._post_once(*args, **kwargs)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
File "/home/brad/certbot/acme/src/acme/client.py", line 781, in _post_once
response = self._check_response(response, content_type=content_type)
File "/home/brad/certbot/acme/src/acme/client.py", line 630, in _check_response
raise messages.Error.from_json(jobj)
acme.messages.Error: urn:ietf:params:acme:error:invalidContact :: The provided contact URI was invalid :: Unable to update account :: invalid contact: contact email has forbidden domain "example.org"
```
if you also change [this email
variable](0075104805/acme/examples/http01_example.py (L223))
to a valid email address, the script will run successfully
Fixes#10266.
See example deprecation in
https://github.com/certbot/josepy/pull/207/files
I can add stacklevel=2, though I find that usually I just look at the
whole stack anyway when debugging, myself, so it doesn't really matter.
This was added in https://github.com/certbot/certbot/pull/6091 to make
tests pass in EPEL and older ubuntus, 7 years ago. It is probably no
longer needed.
| pytest version | min. Python version |
|---------------|---------------------|
|8.0+ | 3.8+|
| 7.1+ | 3.7+ |
| 6.2 - 7.0 | 3.6+ |
| 5.0 - 6.1 | 3.5+ |
| 3.3 - 4.6 | 2.7, 3.4+ |
That version is [no longer
supported](https://docs.pytest.org/en/stable/backwards-compatibility.html).
Probably therefore we can just get rid of this.
Addresses #10264, though I could not actually find a way to fix that
particular issue. So, fixes#10264 is not actually accurate, but I would
like github to link them.
Fixes#10252.
See further discussion here: https://github.com/pypa/pip/issues/11457
We are doing option:
> Alternatively, enable the --use-pep517 pip option, possibly with
--no-build-isolation. The --use-pip517 flag will force pip to use the
modern mechanism for editable installs. --no-build-isolation may be
needed if your project has build-time requirements beyond setuptools and
wheel. By passing this flag, you are responsible for making sure your
environment already has the required dependencies to build your package.
Once the legacy mechanism is removed, --use-pep517 will have no effect
and will essentially be enabled by default in this context.
Major changes made here include:
- Add `--use-pep517` to use the modern mechanism, which will be the only
mechanism in future pip releases
- Change to `/src` layout to appease mypy, and because for editable
installs that really is the normal way these days.
- `cd acme && mkdir src && mv acme src/` etc.
- add `where='src'` argument to `find_packages` and add
`package_dir={'': 'src'},` in `setup.py`s
- update `MANIFEST.in` files with new path locations
- Update our many hardcoded filepaths
- Update `importlib-metadata` requirement to fix
double-plugin-entry-point problem in oldest tests
This PR adds `certbot-dns-cdmon` to the list of third-party plugins in
the documentation.
`certbot-dns-cdmon` enables DNS-01 challenge automation for domains
managed with cdmon's DNS.
PyPI: https://pypi.org/project/certbot-dns-cdmon/
Fixes https://github.com/certbot/certbot/issues/10233
This was a stand-in for
`cryptography.hazmat.primitives.asymmetric.types.CertificateIssuerPrivateKeyTypes`,
which we can now use directly.
We need this to create issues to track work like "update venv.py to
address upcoming pip build system deprecation" since we no longer have a
blank issue template.
Fixes
https://github.com/certbot/certbot/issues/9835#issuecomment-2717096178,
where our `RewriteEngine on` directive inserted at the beginning of a
virtualhost was overridden a `RewriteEngine Off` directive later. This
PR does the easy thing of placing `RewriteEngine on` in our
post-insert.
The credentials configuration file is at ~/.aws/credentials.
Also, when running on root it uses the root home (so /root/.aws). This
was from my test at an ubuntu server.
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `master` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [ ] Add or update any documentation as needed to support the changes
in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
Previously we defaulted to renewing at 30 days before expiry, and
allowed users to customize the config file to set a different value.
Instead, we should renew when 1/3 of the lifetime is left, or for
shorter certificates (<10 days), when 1/2 of the lifetime is left.
This still allows explicitly configured values to take precedence.
---------
Co-authored-by: Will Greenberg <ifnspifn@gmail.com>
Give better instructions on running all unittests, and on running
specific test cases.
Replace `python` with `python3` in venv setup invocations because some
systems don't have a plain `python` command.
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
We strongly encouraged providing an email address because we wanted
people to get expiration notices to ensure that even if their Certbot
install broke, they could fix it before their site goes down.
Now that Let's Encrypt is getting rid of expiration notices
(https://letsencrypt.org/2025/01/22/ending-expiration-emails/), we can
remove some of the encouragement, providing a smoother user experience.
i wanted this for testing
https://github.com/certbot/certbot/issues/10190
alex started working on this in
https://github.com/certbot/certbot/pull/9207 years ago, but pebble
didn't end up doing a release containing his work while he was still
regularly contributing to certbot. this has now changed though
before this PR, our integration tests only worked on amd64 linux
systems. with this PR, i've successfully run our integration tests on
all combinations of the architectures amd64 and arm64 and the OSes linux
and macos
---------
Co-authored-by: ohemorange <erica@eff.org>
The token is now owned by the team account and can simply be
regenerated, so we don't need the info about perhaps doing that. Plus,
there are now more clear instructions on the wiki. And the date was
updated.
Recognizes the profiles map in the "meta" section of directory.
Allows sending a "profile" field in order objects.
Adds an optional "profile" parameter to new_order in client.py.
Related to #10194.
Uploading for tests;
These deprecations are a precursor to #10174
In addition to the previously discussed `acme` functions, the `certbot`
functions were deprecated as they are primarily used for testing and
support. Marking them deprecated now will allow them to be removed in
the next major release, as they will no-longer be used.
Adding automation for team triage meetings for when PRs or Issues are
assigned. You can see an example in the "Test" channel.
---------
Co-authored-by: ohemorange <erica@eff.org>
Fixes https://github.com/certbot/certbot/issues/10177.
We were using `.upper()` when validating the config but not when
actually creating the object. Now we call it in both places. I updated a
test to work as a regression test here.
The v3 api was sunset at the end of July 2023. This PR cleans up code
related to api v3.
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `main` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [ ] Add or update any documentation as needed to support the changes
in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
This is a stopgap measure until we upgrade to the newer (but
backwards-incompatible) versions of cloudflare's python library (see
#9938)
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
I don't love the `Any` in that `Callable`, but I can't find a way to fix
it. In practice, it's either going to be `str` or `None`, but we pass an
`options` that's typed as `List[str] | str | None`, and one of the
functions has a header with a strict `str`. I tried various unions of
things and it wasn't working and I decided it's not worth it.
```
$ mypy --strict certbot-apache/certbot_apache/_internal/configurator.py
Success: no issues found in 1 source file
```
- Better labels upon an issue going stale will help triage better. There
other PRs with "needs update" that are manually put and therefore we
can't explicitly filter for stalebot.
- For management purposes, being able to view how many issues are
auto-closed helps as well.
We could also leave the `jose.JWKRSA` call as-is and add
`--implicit-reexport`, or explicitly export `JWKRSA` in `josepy`, but I
think just cleaning the calls up is nice.
```
$ mypy --strict certbot-apache/certbot_apache/_internal/tests/util.py
Success: no issues found in 1 source file
```
```
$ mypy --strict certbot-dns-google/certbot_dns_google
Success: no issues found in 5 source files
```
I can change the `setattr`s to `# type: ignore [method-assign,
unused-ignore]` if that's preferred.
```
$ mypy --strict certbot-dns-cloudflare/certbot_dns_cloudflare
Success: no issues found in 5 source files
```
If we do `type: ignore` but don't set `--strict`, mypy gets mad. Flake8
doesn't like this but luckily we don't use that here (yet?). The other
option is to add `# type: ignore [method-assign, unused-ignore]`; I can
change it to that if that's preferred.
```
$ mypy --strict certbot-dns-rfc2136/certbot_dns_rfc2136
Success: no issues found in 5 source files
```
`dnspython` would be perfectly happy to accept a string once the
algorithm is passed through, but our `_RFC2136Client` object will only
accept `dns.name.Name` objects, so let's make it happy.
Decided that imports should be in ascii, not caseless, order. If we've
done otherwise elsewhere I'll change it.
The cast isn't the best but that's what we get for using a dict holding
various objects -- the thing marked by `vhost` is going to be a
virtualhost so 🤷
`re.findall` can
[return](https://docs.python.org/3/library/re.html#re.findall) either a
list of strings or a list of tuples of strings, depending what you
specify in the regex. In our case it's going to be a list of strings.
```
$ mypy --strict certbot-apache/certbot_apache/_internal/http_01.py
Success: no issues found in 1 source file
$ mypy --strict certbot-apache/certbot_apache/_internal/obj.py
Success: no issues found in 1 source file
```
PEP 526 says to declare types of unpacked tuples beforehand:
https://peps.python.org/pep-0526/#global-and-local-variable-annotations.
Could have just declared it in apache, but improved the acme return type
while I was at it.
Once again, `typing.Pattern` is deprecated in favor of `re.Pattern` so
changing that while parametrizing the type
in https://github.com/certbot/certbot/pull/9124 we had the problem of
certbot-nginx's `Addr.fromstring` method possibly returning None which
is not possible in the `Addr` method in the certbot base class or in
certbot-apache. we fixed this by telling mypy the common
`Addr.fromstring` method returns an `Optional[Addr]` (despite it
actually always returning an `Addr`) and then unnecessarily complicating
certbot-apache's code a bit. the need for extra complexity with this
approach is going even further in
https://github.com/certbot/certbot/pull/10151 where we have to use
`cast` to assure mypy that the type isn't actually `Optional`. i
personally don't like all this
Fixes#10000.
To create this PR, I looked through `constants.py` for defaults set to
`None`. If the action for the cli flag was `store_true` and there wasn't
other custom manual default specification, I changed it to report
`False`, and added a comment in `constants.py`. Adding `(default:` in
the help text suppresses listing of the actual default (done by
`cli_utils.py:CustomHelpFormatter`). Also added a comment for `redirect`
which is described manually since I noticed it while I was going
through.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
If we do `type: ignore` but don't set `--strict`, mypy gets mad. Flake8
doesn't like this but luckily we don't use that here (yet?). The other
option is to add `# type: ignore [method-assign, unused-ignore]`; I can
change it to that if that's preferred.
```
$ mypy --strict certbot-dns-digitalocean/certbot_dns_digitalocean
Success: no issues found in 5 source files
```
```
$ mypy --strict certbot-apache/certbot_apache/_internal/parser.py
Success: no issues found in 1 source file
```
`typing.Pattern` is deprecated in python 3.9 in favor of using
`re.Pattern` directly, and also wants to be subscripted with its type.
`python-augeas` types can be found in
a1e84a7e58/augeas/__init__.py
Fixes#10011
When we take a server block with no ssl addresses in and and enable ssl,
if it has any listens on the http port, use those host addresses when
creating a directive to listen on ssl. Addresses with no port and on
other ports will be ignored.
---------
Co-authored-by: Will Greenberg <willg@eff.org>
`typing.Type` is deprecated in favor of built-in `type`. In strict
mode,`find_ancestors` needs to be more specific about what it actually
returns, due to covariance and generics and such.
```
$ mypy --strict certbot-apache/certbot_apache/_internal/dualparser.py
Success: no issues found in 1 source file
```
Pasted from the old one. Maybe we can just rename it but this is what
github's web interface led me to create.
I want to make sure that they at least create the template so that they
read it. If they then choose to ignore it that's fine, but it should
always pop up. Basically I want to keep the old behavior. Open to
alternatives.
We could also play around with the new issue forms:
https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms
Or label this one the "bug" template, and create a second one that is
blank but has the header text paragraph. I haven't seen a way to make
something appear in all templates, including the "blank" one, other than
just turning off blank templates.
fixes https://github.com/certbot/certbot/issues/10131
this seems simple enough, but i also requested alex's review as a quick
sanity check if he doesn't mind providing one
i've verified this fixes the problem and that PKCS#8 was used in certbot
3.0.1
gen_ss_cert()'s signature contains deprecated pyOpenSSL API, so here we
deprecate it in favor of a new function that does the same thing, except
with only cryptography types: make_self_signed_cert
fixes#10105
this PR updates our minimally required cryptography and pyopenssl
versions as well as updating our policy for choosing minimum dependency
versions generally
before this PR, we were trying to keep compatibility with packages
available in EPEL 9 using the python 3 version available in RHEL 9.
after the discussion in #10105 we decided not to do this anymore
because:
* EPEL 9 may not want to update to certbot 3.0+ anyway because of our
backwards incompatible changes from certbot 2.x
* RHEL 9 appstream repos now contain newer versions of many of our
dependencies for newer versions of python
* alternate installation methods for RHEL 9 based users including our
snaps and pip are available
on a call we then discussed what distro repositories we should track
instead of EPEL 9. our docs previously said Debian sid/unstable, but we
felt this as unnecessary because Debian sid can and does change very
quickly. if we wanted a new dependency there, Debian could probably
accommodate it
we also considered RHEL 10 + EPEL 10, however, these repos are not even
stable yet and certbot and many of its dependencies are not yet packaged
there at all
for these reasons, plus many of the reasons we decided to upgrade past
EPEL 9 with the default python 3 version there, we decided that at least
for now, we will remove any linux distro considerations when choosing
minimal dependency versions of certbot
as i wrote in the contributing docs, we may choose to reconsider this
plan if there are requests for us to do so, but based on the information
above, we are not sure this will ever even happen and removing this
constraint significantly simplifies development of certbot
this hopefully at least helps the problem hit at
https://github.com/certbot/certbot/pull/10126#discussion_r1909714276
i took this approach because in my experience, linux specific shell
commands have crept into our scripts repeatedly over the years so i
think just having macOS devs use the linux versions is much more
reliable. it's what i've personally been doing for years now
when reviewing https://github.com/certbot/certbot/pull/10126 and running
`tools/pinning/oldest/repin.sh` using a freshly created dev environment,
i was repeatedly given the message
> The "poetry.dev-dependencies" section is deprecated and will be
removed in a future version. Use "poetry.group.dev.dependencies"
instead.
i believe this section was generated automatically by poetry's tooling
when it created the initial boilerplate file for us, but we don't use
it, so i just deleted the section which makes the warnings disappear
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `main` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [x] Add or update any documentation as needed to support the changes
in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
This is needed because the Python + OpenSSL bundled in core24 don't
include an OpenSSL FIPS provider, which causes crashes on host systems
with OpenSSL 1.1.1f (e.g. Ubuntu Pro 20.04). For some reason, core24's
OpenSSL also looks in a non-standard location for the provider, which
also causes crashes on systems with OpenSSL 3.x (e.g. RHEL 9). If you
need FIPS functionality in certbot, install via pip.
This seems to be better style. The assert False statements are
automatically removed by Python when running in the optimized mode,
which could hide test failures.
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `main` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [ ] Add or update any documentation as needed to support the changes
in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
Co-authored-by: Mads Jensen <atombrella@users.noreply.github.com>
this PR hopefully improves two things that i hit while working on #10035
1) i found that repinning our dependencies took ~6 minutes!
digging into it a bit, the biggest culprit i found was the inclusion of
`--no-cache-dir` here which seemed to cause poetry to redownload the
same packages over and over in a single run. this comes from
https://github.com/certbot/certbot/pull/9453 which fixed a problem i
(but not alex) was having with a major performance penalty. i removed
the flag here and instead included instructions on clearing poetry's
caches in case anyone ever hits this in the future. with this change,
the script now takes about 40 seconds on my laptop
2) every run of this script ended with the output:
```
Warning: poetry-plugin-export will not be installed by default in a
future version of Poetry.
In order to avoid a breaking change and make your automation
forward-compatible, please install poetry-plugin-export.
explicitly. See https://python-poetry.org/docs/plugins/#using-plugins
for details on how to install a plugin.
To disable this warning run 'poetry config warnings.export false'.
```
setting `POETRY_WARNINGS_EXPORT=false` fixes this which i believe is
safe to do because of
2c8609464c/certbot/setup.py (L53-L56)
i hit this when working on https://github.com/certbot/certbot/pull/10076
where i found that updating all our dependencies no longer worked
because of new deprecations in pyopenssl. this pr fixes that
There are a quite a lot of imports that are unused.
F541 is Unnecessary f-interpolation without placeholders
E711 is incorrect use of == for boolean and None comparisons
## Pull Request Checklist
- [x] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `main` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [ ] Add or update any documentation as needed to support the changes
in this PR.
- [x] Include your name in `AUTHORS.md` if you like.
---------
Co-authored-by: Mads Jensen <atombrella@users.noreply.github.com>
Because of the change from using setuptools.pkg_resources to using
importlib, we no longer need a runtime dependency on setuptools. It is
still required, however, for running setup.py.
* restore incorrect regex changes to CHANGELOG.md
* Update _release.sh regex to switch only first instance of main in changelog
(cherry picked from commit 0e225dcba2)
Fixes#9990
If the python oneliner to check certbot's version succeeded, exit_code
would never be set, which would cause our exit_code check to fail. Use
a check that handles unset exit_code
* Print an error if outdated snap plugins detected
With Certbot 3.0 comes a bump to Python 3.12, so if any snap plugins
are still located in a python3.8 directory, print an error informing
the user.
* tox nitpicks
* personal nitpick
* review fixups
* Update certbot/certbot/_internal/snap_config.py
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Use LOGGER.warn instead of error
* warn-->warning
* warn-->warning
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
macOS-12 is [being deprecated](https://github.com/actions/runner-images/issues/10721) on Azure, so update to the latest available version.
* Upgrade macOS azure tests to use macOS-15
* switch standard azure tests to using python 3.12
* restore mac and linux cover tests to oldest and newest version style, and add explanation that that's what we're doing.
We're a few years behind the curve on this one, but using "master" as a
programming term is a callous practice that explicitly uses the
historical institution of slavery as a cheap, racist metaphor. Switch to
using "main", as it's the new default in git and GitHub.
This is another and very minor piece of https://github.com/certbot/certbot/issues/9988.
We've done nothing to warn/migrate installations using the old `certbot-route53:auth` plugin name and installations like that still exist according to https://gist.github.com/bmw/aceb69020dceee50ba827ec17b22e08a. We could try to warn/migrate these users for a future release or decide it's niche enough that we'll just let it break, but I think it's easy enough to keep the simple shim around.
This PR just moves the code raising a deprecation warning into `_internal` as part of cleaning up all deprecation warnings I found in https://github.com/certbot/certbot/issues/9988. I manually tested this with a Certbot config using the `certbot-route53:auth` plugin name and renewal worked just fine.
The `gnupg` package from Homebrew only installs a `gpg` binary, not a `gpg2` binary. I had previously worked around this by manually creating an alias, but I think we can do better.
GPG version 1 is ancient and [hasn't seen a release since 2006](https://gnupg.org/download/release_notes.html). Additionally, `gpg` has referred to GPG 2 in Ubuntu since at least 20.04 which is the oldest non-EOL'd version as of writing this so I think this change is safe to make.
Fixes#9872, originally merged in #9956.
To upgrade to python3.12 as 3.8 is reaching EOL, we need to upgrade the core snap that certbot is based on. The latest version is core24, so we're going with that for longevity. We will want to notify third party snaps to make changes as well. They can release their snaps to a version higher than certbot's, and their users will not be upgraded until the matching (or greater) version of certbot is released. They should do this as otherwise including these changes will break their plugins.
Key documents for this migration are https://snapcraft.io/docs/migrate-core22 and https://snapcraft.io/docs/migrate-core24. The discussion at https://forum.snapcraft.io/t/upgrading-classic-snap-to-core24-using-snapcraft-8-3-causes-python-3-12-errors-at-runtime/ is also relevant to understanding some changes, which may become unnecessary in future versions of snapcraft.
* Migrate primary certbot snap to core24 and python 3.12
* Migrate plugin snaps to core24 and python 3.12
* Migrate to core24 in build_remote
* Run snap tests using python 3.12
* Unstage pyvenv.cfg and set PYTHONPATH
---------
Co-authored-by: Erica Portnoy <ebportnoy@gmail.com>
Co-authored-by: Erica Portnoy <erica@eff.org>
Recently our test environments were upgraded to use Docker 26, which
enabled ipv6 loopback by default in containers. This caused tests to
start failing due to an nginx test config which was the sole listener
for ipv6.
This simply removes that ipv6 listen directive in the config, and the
archived version we use for testing.
while working on https://github.com/certbot/certbot/issues/9938, i updated our dependencies which updated mypy introducing new errors that mypy wanted me to fix. i think this makes the regularly necessary process of updating our dependencies too tedious and we should instead pin our linters that do this to a specific version and update them manually as desired. we already do this with pylint in the lines above my changes in this PR for the same reason
This fixes a bug where, when a user requests a cert interactively, the
CSR's SANs are not listed in the order that the user has in mind. This
is because, during the input validation, the _scrub_checklist_input
method does not produce a list of tags (which represents the domain
names the user has requested a cert for) in the order of in the given
indices. As a result, the CN of the resulting cert, as well as the
directory name used to store the certs, may not always be what the user
has expected, which should be the first item chosen from the interactive
prompt.
Pebble 2.5.1 supports OCSP stapling, so we can finally replace all boulder tests/harnesses with the much simpler pebble setup.
Closes#9898
* Remove unused `--acme-server` argument
Since this argument is never set and always defaults to 'pebble', just
remove it to simplify assumptions about which test server's being used.
* Remove boulder option from integration tests
Now that pebble supports all of our test cases, we can move off of
the much more complicated boulder test harness.
* pebble_artifacts: bump to latest pebble release
* pebble_artifacts: fix download path
* certbot-ci: unzip pebble assets
* CI: rip out windows tests/jobs
* tox.ini: rm outdated Windows comment
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* ci: rm redundant integration test
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* acme_server: raise error if proxy and http-01 port are both set
* acme_server: rm vestigial preterimate commands stuff
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Azure recently dropped the `docker-compose` standalone executable (aka
docker-compose v1), and since it's not receiving updates anymore, let's
get with the times and update to v2 as well.
* Reset requirements.txt path
* Add requirements.txt path
* Test config path
* Change docs path
* Amend paths for successful builds
* Place copyright for epub
- Will amend copyright parameter at a later date
* Make reconfigure use staging server
* lint and imports
* Unset the account if it's been set in preparation for a dry run
* Add unit tests for checking we switch to staging and don't accidentally modify anything else
* add docstring
* Add test to make sure a requested new account id is saved
* update changelog
* set noninteractive mode for dry run
* error when account or server is set by the user
* switch to checking for changed values in account and server
* recommend using renew instead of certonly for forbidden fields
* change link to renew-reconfiguration
* Set the delegated field in Lexicon config to bypass subdomain resolution (#9821)
The Lexicon-based DNS plugins use a mechanism to determine which actual segment of the input domain is actually the DNS zone in which the DNS-01 challenge has to be initiated (eg. `subdomain.domain.com` or `domain.com` for input `subdomain.domain.com`): they tries recursively to configure Lexicon and initiate authentication from the most specific to most generic domain segment, and select the first segment where Lexicon stop erroring out.
This mechanism broke with #9746 because now the plugins call Lexicon client instead of the underlying providers, and the client makes guess on the actual domain requested. Typically for `subdomain.domain.com` it will actually try to authenticate against `domain.com`, and so the mechanism above does not work anymore.
This PR fixes the issue by using the `delegated` field in Lexicon config each time the plugin needs it. This field is designed for this kind of purpose: it will instruct Lexicon what is the actual DNS zone domain instead of guessing it.
I tested the change with one of my OVH account. The expected behavior is re-established and the plugin is able to test `subdomain.domain.com` then `domain.com` as before.
Fixes#9791Fixes#9818
(cherry picked from commit cf4f07d17e)
* add changelog entry for 9821 (#9822)
(cherry picked from commit 7bb85f8440)
---------
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
The Lexicon-based DNS plugins use a mechanism to determine which actual segment of the input domain is actually the DNS zone in which the DNS-01 challenge has to be initiated (eg. `subdomain.domain.com` or `domain.com` for input `subdomain.domain.com`): they tries recursively to configure Lexicon and initiate authentication from the most specific to most generic domain segment, and select the first segment where Lexicon stop erroring out.
This mechanism broke with #9746 because now the plugins call Lexicon client instead of the underlying providers, and the client makes guess on the actual domain requested. Typically for `subdomain.domain.com` it will actually try to authenticate against `domain.com`, and so the mechanism above does not work anymore.
This PR fixes the issue by using the `delegated` field in Lexicon config each time the plugin needs it. This field is designed for this kind of purpose: it will instruct Lexicon what is the actual DNS zone domain instead of guessing it.
I tested the change with one of my OVH account. The expected behavior is re-established and the plugin is able to test `subdomain.domain.com` then `domain.com` as before.
Fixes#9791Fixes#9818
* helpful: fix handling of abbreviated ConfigArgparse arguments (#9796)
* helpful: fix handling of abbreviated ConfigArgparse arguments
ConfigArgparse allows for "abbreviated" arguments, i.e. just the prefix
of an argument, but it doesn't set the argument sources in these cases.
This commit checks for those cases and sets the sources appropriately.
* failing to find an action raises an error instead of logging
* Update changelog
* Add handling for short arguments, fix equals sign handling
These were silently being dropped before, possibly leading to instances
of `NamespaceConfig.set_by_user()` returning false negatives.
(cherry picked from commit 11e17ef77b)
* Fix finish_release.py (#9800)
* response is value
* rename vars
(cherry picked from commit a96fb4b6ce)
* Merge pull request #9762 from certbot/docs/yaml-config
Add YAML files for Readthedocs requirements
(cherry picked from commit 44046c70c3)
* Update Lexicon requirements to stabilize certbot-dns-ovh behavior (#9802)
* Update minimum Lexicon version required for certbot-dns-ovh
* Add types
* FIx mypy
* Fix lint
* Fix BOTH lint and mypy
(cherry picked from commit 5cf5f36f19)
* simplify code (#9807)
(cherry picked from commit 6f7b5ab1cd)
* Include linting fixes from 8a95c03
---------
Co-authored-by: Will Greenberg <willg@eff.org>
Co-authored-by: Alexis <alexis@eff.org>
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
* helpful: fix handling of abbreviated ConfigArgparse arguments
ConfigArgparse allows for "abbreviated" arguments, i.e. just the prefix
of an argument, but it doesn't set the argument sources in these cases.
This commit checks for those cases and sets the sources appropriately.
* failing to find an action raises an error instead of logging
* Update changelog
* Add handling for short arguments, fix equals sign handling
These were silently being dropped before, possibly leading to instances
of `NamespaceConfig.set_by_user()` returning false negatives.
* Drop Python 3.7 support
* Fix lint and test
* Check for venv generation
* Update requirements
* Update oldest constaints and compatibility tests runtime
* Refactor Lexicon-based DNS plugins and upgrade minimal version of Lexicon
* Relax filterwarning to comply with envs where boto3 is not installed
* Update pinned dependencies
* Use our previous method to deprecate part of modules
* Safe import internally
* Add changelog
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Migrate entrypoint logic from pkg_resources to importlib.metadata
* Usage of importlib_metadata up to Python 3.9 to align API behavior to Python 3.10
---------
Co-authored-by: Adrien Ferrand <adrien.ferrand@amadeus.com>
Co-authored-by: Adrien Ferrand <adrien.ferrand@arteris.com>
* Migrate pkg_resources API related to resources to importlib_resources
* Fix lint and mypy + pin lexicon
* Update filterwarnings
* Update oldest tests requirements
* Update pinned dependencies
* Fix for modern versions of python
* Fix assets load in nginx integration tests
* Fix a warning
* Isolate static generation from importlib.resource into a private function
---------
Co-authored-by: Adrien Ferrand <adrien.ferrand@amadeus.com>
* dns-google: fix condition to don't use private dns zones
* update MD
* Fix condition
* fix condition
* update testdata
* fix identation
* update tests
* update changelog
* Update dns_google.py
* add test for split horizon dns google
* add dnsName to managed zones
* update quickstart and remove os import
* simplify theme use
* list sphinx_rtd_theme as extension
Our docs builds failed last night, presumably because #9754 updated `sphinx_rtd_theme` which changed some unknown thing.
Looking into it, our usage of this project was very unconventional. Following the code comment I deleted in this PR to https://docs.readthedocs.io/en/stable/faq.html#i-want-to-use-the-read-the-docs-theme-locally, simple instructions are given to put the following in your `conf.py` file:
```
extensions = [
...
'sphinx_rtd_theme',
]
html_theme = "sphinx_rtd_theme"
```
I did this instead of the more complicated logic we were using and all builds passed locally. I also triggered a build on readthedocs with these changes which also passed.
This takes care of the dependabot alerts those with access can see at https://github.com/certbot/certbot/security/dependabot.
Pinning back `cython` is needed because without it, our full test suite will fail when trying to build `pyyaml` on ARM systems.
* Do not call deprecated datetime.utcnow() and datetime.utcfromtimestamp()
* Ignore DeprecationWarnings from importing dependencies
$ python3 -Wdefault
Python 3.12.0b4 (main, Jul 12 2023, 00:00:00) [GCC 13.1.1 20230614 (Red Hat 13.1.1-4)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import pkg_resources
/usr/lib/python3.12/site-packages/pkg_resources/__init__.py:121: DeprecationWarning: pkg_resources is deprecated as an API
warnings.warn("pkg_resources is deprecated as an API", DeprecationWarning)
>>> import pytz
/usr/lib/python3.12/site-packages/pytz/tzinfo.py:27: DeprecationWarning: datetime.utcfromtimestamp() is deprecated and scheduled for removal in a future version. Use timezone-aware objects to represent datetimes in UTC: datetime.fromtimestamp(timestamp, datetime.UTC).
_epoch = datetime.utcfromtimestamp(0)
* Used pytz.UTC consistently for clarity
* repin current
* repin oldest
* csr must have version set to zero
* only set PIP_USE_PEP517 for macOS
* experiment with brew update git failure workaround
* Revert change to NamespaceConfig's constructor
NamespaceConfig's argument sources dict is now set with a method,
and raises a runtime error if one isn't set when set_by_user() is
called.
* Actually update CHANGELOG to reflect the set_by_user changes
* linter appeasement
* configuration: update docs, add test
This test ensures that calling `set_by_user` without an initialized
sources dict raises a RuntimeError.
* Rewrite helpful_test to appease the linter
* Use public interface to access argparse sources dict
* HelpfulParser builds ArgumentSources dict, stores it in NamespaceConfig
After arguments/config files/user prompted input have been parsed, we
build a mapping of Namespace options to an ArgumentSource value. These
generally come from argparse's builtin "source_to_settings" dict, but
we also add a source value representing dynamic values set at runtime.
This dict is then passed to NamespaceConfig, which can then be queried
directly or via the "set_by_user" method, which replaces the global
"set_by_cli" and "option_was_set" functions.
* Use NamespaceConfig.set_by_user instead of set_by_cli/option_was_set
This involves passing the NamespaceConfig around to more functions
than before, removes the need for most of the global state shenanigans
needed by set_by_cli and friends.
* Set runtime config values on the NamespaceConfig object
This'll correctly mark them as being "runtime" values in the
ArgumentSources dict
* Bump oldest configargparse version
We need a version that has get_source_to_settings_dict()
* Add more cli unit tests, use ArgumentSource.DEFAULT by default
One of the tests revealed that ConfigArgParse's source dict excludes
arguments it considers unimportant/irrelevant. We now mark all arguments
as having a DEFAULT source by default, and update them otherwise.
* Mark more argument sources as RUNTIME
* Removes some redundant helpful_test.py, moves one to cli_test.py
We were already testing most of these cases in cli_test.py, only
with a more complete HelpfulArgumentParser setup. And since the hsts/no-hsts
test was manually performing the kind of argument adding that cli
already does out of the box, I figured the cli tests were a more natural
place for it.
* appease the linter
* Various fixups from review
* Add windows compatability fix
* Add test ensuring relevant_values behaves properly
* Build sources dict in a more predictable manner
The dict is now built in a defined order: first defaults, then config
files, then env vars, then command line args. This way we eliminate the
possibility of undefined behavior if configargparse puts an arg's entry
in multiple source dicts.
* remove superfluous update to sources dict
* remove duplicate constant defines, resolve circular import situation
* letstest: -ubuntu18.04 +centos9stream +debian11
* letstest: username for centos 9 stream is ec2-user
This is mentioned on https://centos.org/download/aws-images/
* ensure mod_ssl is installed
in centos 9 stream, apache has to be restarted after mod_ssl is
installed, or the snakeoil certificates will not be present and
apache won't start.
this also removes nghttp2 being installed as the relevant bug
is long fixed.
* dns-rfc2136: add test coverage for PR #9672
* fix compatibility with oldest dnspython
* rename test to be more descriptive
Co-authored-by: ohemorange <ebportnoy@gmail.com>
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
This is, to my knowledge, an entirely inconsequential PR to add support for entirely novel challenge types.
Presently in the [`challb_to_achall` function](399b932a86/certbot/certbot/_internal/auth_handler.py (L367)) if the challenge type is not of a type known to certbot an error is thrown. This check is mostly pointless as an authenticator would not request a challenge unknown to it. This check does however forbid any plugins from supporting entirely novel challenges not of the key authorisation form.
* support unknown ACME challenge types
* add to changelog
* update tests
---------
Co-authored-by: Brad Warren <bmw@eff.org>
* remove pointless paragraph about --server and wildcards
* docs: update help text for --dry-run and --staging
* docs: update "Changing the ACME Server" for --dry-run
* add note about webserver reloads
* Optionally sign initial SOA query
Added configuration file option to enable signing of the initial SOA query when determining the authoritative nameserver for the zone. Default is disabled.
* Better handling of sign_query configuration and fix lint issues
* Update str casting to match 5503d12395
* Update certbot/CHANGELOG.md
Co-authored-by: alexzorin <alex@zorin.au>
* Update certbot/CHANGELOG.md
Co-authored-by: alexzorin <alex@zorin.au>
* Update dns_rfc2136.py
Updated with feedback from certbot/certbot#9672
---------
Co-authored-by: alexzorin <alex@zorin.au>
In addition to the speed improvements in CI, the speed improvements locally with both this https://github.com/certbot/certbot/pull/9666 which this builds on is even more significant. After it's been run once so it's had a chance to set up the different virtual environments, `tox` locally now takes 39 seconds on my laptop when it used to take 137 seconds.
Fixes#6127.
* Added lineage name validity check
* Verify lineage name validity before obtaining certificate
* Added linage name limitation to cli help
* Update documentation on certificate name
* Added lineage name validation to changelog
* Use filepath seperators to determine lineagename validity
* Add unittest for private choose_lineagename method
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* add some missing types
* install pkg-config
* install pkg-config for docker too
* add pkg-config to plugins
* pkg-config when cryptography may need to be built
* deps cleanup
* more comments
* more tweaks
Fixes https://github.com/certbot/certbot/issues/7921.
In all cases when we run `pip_install.py`, we first run `pipstrap.py`. This PR combines these two steps for convenience and to make always doing that less error prone. This will also help me with some of the `tox.ini` refactoring I'm planning to do.
I ran the full test suite on everything and tested the release script changes locally.
This change shouldn't have any effect on cryptography's setup because they install `certbot[test]` which depends on pip, setuptools, and wheel.
* always pipstrap
* use pip_install.py during releases
This is a first step towards implementing the plan I described at https://github.com/certbot/certbot/issues/7909#issuecomment-1448675456 which got a +1 from both Erica and Will. Similar changes for our other packages will be made in followup PRs to try and make this easier to review.
It may be helpful to look at https://github.com/certbot/certbot/pull/7600 when reviewing this PR where we did something similar in the past.
The value of `ignore-paths` in `.pylintrc` should work on Windows based on https://pylint.readthedocs.io/en/latest/user_guide/configuration/all-options.html#ignore-paths and the fact that on macOS/linux, changing path delimiters to `\` still causes these directories to be ignored.
I started testing this for mypy as well, but mypy doesn't current pass for us on Windows so I didn't bother and took this opportunity to remove it from the default environments in `tox.ini`. I'll update https://github.com/certbot/certbot/issues/7803 to mention that the value of `exclude` in `mypy.ini` may need to be tweaked if anyone works on that issue.
* make acme tests internal
* no mypy-win
Adrien and I added this is in https://github.com/certbot/certbot/pull/6590 in response to https://github.com/certbot/certbot/issues/6582 which I wrote. I now personally think these tests are way more trouble than they're worth.
In almost all cases, the versions pinned in `tools/requirements.txt` are used. The two exceptions to this that come to mind are users using OS packages and pip. In the former, the version of our dependencies is picked by the OS and do not change much on most systems. As for pip, [we only "support it on a best effort basis"](https://eff-certbot.readthedocs.io/en/stable/install.html#alternative-2-pip).
Even for pip users, I'm not convinced this buys us much other than frequent test failures. We have our tests configured to error on all Python warnings and [we regularly update `tools/requirements.txt`](https://github.com/certbot/certbot/commits/master/tools/requirements.txt). Due to that, assuming our dependencies follow normal conventions, we should have a chance to fix things in response to planned API changes long before they make their way to our users. I do not think it is necessary for our tests to break immediately after an API is deprecated.
I think almost all other failures due to these tests are caused by upstream bugs. In my experience, almost all of them will sort themselves out pretty quickly. I think that responding to those that are not or planned API changes we somehow missed can be addressed when `tools/requirements.txt` is updated or when someone opens an issue. I personally don't think blocking releases or causing our nightly tests to fail is at all worth it here. I think removing this frequent cause of test failures makes things just a little bit easier for Certbot devs without costing us much of anything.
* Add async interface for finalization to acme.client.ClientV2
Add `begin_order_finalization()`/`poll_finalization()` to
`acme.client.ClientV2`, which are directly analogous to
`answer_challenge()`/`poll_authorizations()`. This allows us to
finalize an order and then later poll for its completion as separate
steps.
* Address code review feedback
Rename `begin_order_finalization` -> `begin_finalization` and tweak
wording of changelog entry
Right now if you to_json() an `OrderResource` and later deserialize
it, the `AuthorizationResource` objects don't come back through the
round-trip (they just get de-jsonified as frozendicts and worse, they
can't even be passed to `AuthorizationResource.from_json` because
frozendicts aren't dicts). In addition, the `csr_pem` field gets
encoded as an array of integers, which definitely does not get
de-jsonified into what we want.
Fix these by adding an encoder to `authorizations` and encoder and
decoder to `csr_pem`.
`lock_test.py` is a weird, heavily customized, standalone testing relic that's giving me trouble because the name currently conflicts with `certbot/tests/lock_test.py`. Moving `certbot/tests` inside the Certbot package as discussed at https://github.com/certbot/certbot/issues/7909#issuecomment-1448675456 would avoid this, however, this is at least somewhat blocked on getting that test code passing lint and mypy checks again because we run those checks on the entirety of the Certbot package 🙃 Since `lock_test.py` could probably stand to be rewritten/refactored anyway, I took this approach.
What I did is I rewrote something largely equivalent to `lock_test.py` inside Certbot's unit tests. I chose not to do this in `certbot-ci` because its not necessary to have an ACME server available. We're no longer explicitly testing things with the nginx plugin here like we were in `lock_test.py`, however, we are checking that `prepare` is called on the plugin at the right time and I added comments about the importance of checking that we lock the directory during the call to `prepare` in the Apache and nginx test code.
As a bonus, this fixes https://github.com/certbot/certbot/issues/8121.
Now that we're using pytest more aggressively, I think we should start transitioning our tests to that style rather than continuing to use unittest. This PR removes some unnecessary uses of unittest I found.
I kept the test classes (while removing the inheritance from unittest.TestCase) where I felt like it added structure or logical grouping of tests.
I verified that pytest still finds all the tests in both this branch and master by running commands like:
```
pytest $(git diff --name-only master | grep -v windows_installer_integration_tests)
```
* generate multiarch images for non-architecture tags
* lock docker build to legacy docker buider, and bugfix
* rename deploy.sh to deploy_by_arch.sh
* Update documentation related to multiarch Docker
* Consistent IFS value with respect to other scripts
Co-authored-by: humanoid2050 <humanoid2050@monolith>
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Add deprecation warning when update_symlinks is run
* Remove information about update_symlinks from help
* ignore our own warning and remove unused imports
* Update changelog
* run unittest2pytest
The command used here was `unittest2pytest -nw acme/tests certbot*/tests`.
* fix with pytest.raises
* add parens to fix refactoring
* <= not <
I want to use isort as part of https://github.com/certbot/certbot/issues/9572 because I want to do it programmatically, however, I felt like the config needed to be tweaked a bit due to it not understanding what is and is not our own code.
This PR updates the isort config so it recognizes our own modules and runs `isort .` from the root of the repo to update everything.
* update isort config
* run "isort ."
* Add command to update config files without issuing/renewing cert
* toss up a vague untested skeleton
* remove duplicated code
* set certname in config
* consistent name, no zope
* import copy
* reconsitute is in renewal
* import renewal
* import cli
* fix lint errors
* call choose_configurator_plugins for its side effect of writing to config
* Set certonly in choose config plugins as we do for renew
* rewrite by piggybacking on existing side effects of a dry run instead
* do not allow domains to be set while reconfiguring
* remove unused cert_manager.reconfigure
* remove unused imports
* Add comments and messages
* add cli information
* start adding tests
* remove test code
* get certname before setting up plugins
* get plugin from lineage if not set on cli
* import copy
* always reconstitute
* only load cert once
* add error message
* improve comment
* mock everything out for tests
* test functionality is working!
* add tests for adding and modifying hooks
* test that we don't modify the config if the dry run fails
* improve documentation
* add webroot to reconfigure common options
* lint and clean up intermediate artifacts
* mock validate_hooks for windows
* print success message with updated parameters
* Improve success message
* add message for no changes have been made
* improve changed message to show before as well
* syntax
* Add changes will apply at the next renewal message
* lint
* lint really likes dict.items() for some reason
* run the deploy hook
* turn off dry run to test deploy hook
* patch list_hooks call for tests
* factor out reporting results code
* Remove reporting of which values were changed
* add flag to run deploy hook despite doing a dry run, and recommend setting that to yes when running reconfigure and modifying the deploy hook
* missing () around multi-line string
* test if the two dicts are equal instead of finding the actual changes, thus avoiding having to deal with webroot_map being a list
* refer to --deploy-hook instead of deploy hook
* use renewal configuration instead of configuration information
* mention that the deploy hook will use the active cert not the test one
* disable lint and remove new from language asking about running a deploy hook
* pluralize run deploy hook(s)
* Add test for reporting results when there is a webroot map
* update changelog
* Update error message about modifying domains on the certificate
* update changelog
* Add basic integration tests
* Just set -a rather than redoing the whole testing infrastructure
* used webroot in integration test since it's already installed
* file contents are accessed twice now
---------
Co-authored-by: Alex Zorin <alex@zorin.au>
In #9127, where @osirisinferi added the `show_account` verb, I made a call not to include the thumbprint in the output of `certbot show_account`.
In hindsight, and after a community member asked for this feature, I think it's better to include it.
It is useful on occasion and `show_account` is fairly specialized anyway. It's only really good for getting your account URL for rate limit increases, checking your contacts, and (now) and doing *magic* with the thumbprint for stateless/distributed HTTP-01 responders.
Without this feature, a clever user might figure out their thumbprint by doing a `certonly --manual --preferred-challenges http` request, but most users would probably be lost.
* show_account: display account thumbprint
* use local key for display
* certbot-ci: boulder will now only supports port 80 for http-01
* forgot to actually use the http_01_port argument
* print the port the proxy listens on
* try allow binding to privileged ports
* Replace probot/stale app with a Github Action
This creates a Github Actions workflow which seems to be the supported
way of automarking issues as stale. Adds a dry-run flag to test it out.
* small fixups
* cron typo
* disable unnecessary permissions
* use friendlier name
* fix cover tox envs
* make test work on all Pythons
* Remove unused import
Co-authored-by: alexzorin <alex@zorin.id.au>
Co-authored-by: alexzorin <alex@zorin.id.au>
* acme.messages.Error: add mutability
As of Python 3.11, an exception caught within a `with` statement will
update the __traceback__ attribute. Because acme.messages.Error was
immutable, this was causing a knock-on exception, causing certbot to
exit abnormally. This commit hacks in mutability for acme.messages.Error
Fixes#9539
* Add CHANGELOG entry
Based on my design [here](https://docs.google.com/document/d/1jGh_bZPnrhi96KzuIcyCJfnudl4m3pRPGkiK4fTo8e4/edit?usp=sharing).
Fixes https://github.com/certbot/certbot/issues/4634 and https://github.com/certbot/certbot/issues/4635.
- [x] Deprecate `NamespaceConfig.csr_dir`,`NamespaceConfig.key_dir`, ~~`constants.CSR_DIR` and `constants.KEY_DIR`~~. (`constants` is `_internal` so we can just delete it eventually).
- [x] Update `certbot.crypto_util.generate_csr` and `.generate_key` to make `csr_dir` and `key_dir` optional, respectively.
- [x] Change `certbot._internal.client.Client.obtain_certificate` to no longer include `csr_dir` and `key_dir` to the `.generate_csr` and `.generate_key` calls, respectively.
- Automatically delete unwanted lineage items:
- [x] In `certbot._internal.storage.RenewableCert`, add a function to truncate the lineage history according to the criteria (keep the current and the 5 prior certificates).
- [x] Add a test suite for `truncate`
- [x] In `certbot._internal.renewal.renew_cert`, call the lineage truncation function after the symlinks have been updated for the renewal.
* Stop writing new files to /csr and /keys
* storage: add lineage truncation
* remove unused code
* deprecate keys_dir and csr_dir
* update CHANGELOG
* just keep 5 prior certificates, dont be clever with expiry
* docs: remove reference to /archive and /keys
* filter {csr,key}_dir deprecations directly in tests
Fixes#9500
Also print the path to the file with errors for the error "Error parsing credentials configuration" of `dns_common.py`. This makes debugging this error much easier.
I wanted to do this because we were notified that https://ubuntu.com/security/notices/USN-5638-3/ affects our snaps. This probably doesn't affect us, but rebuilding to be safe seems worth it to me personally.
I started to just trigger a new v1.32.0 release build, but I don't want to overwrite our 2.0 Docker images under the `latest` tag.
Changelog changes here are similar to what has been done for past point releases like https://github.com/certbot/certbot/pull/8501.
I also cherry picked #9474 to this branch to help the release process pass.
* add changelog
* Use a longer timeout for releases (#9474)
This is in response to the thread starting at https://github.com/certbot/certbot/pull/9330#issuecomment-1320416069.
In addition to this, I plan to add the following text to the step of the release instructions that tells you to wait until Azure Pipelines for the release has finished running:
> Some jobs such as building our snaps can take a long time to complete, however, if the process seems hung, you can cancel the build and then rerun the failed jobs. To do this, click on the build for the release in the link above, make sure you're logged into Azure Pipelines, and then use the cancel/rerun buttons in the top right of the web page.
(cherry picked from commit 30b4fd59a5)
* nginx: capitalise product names in warning message properly
* nginx: don't crash on encountering lua directives, warn instead
* add tests
* undo excess newline
* fix oldest tests: use old camelCase function name
* add missing newline in new testdata
* add tests for _by_lua, which should parse fine
This is in response to the thread starting at https://github.com/certbot/certbot/pull/9330#issuecomment-1320416069.
In addition to this, I plan to add the following text to the step of the release instructions that tells you to wait until Azure Pipelines for the release has finished running:
> Some jobs such as building our snaps can take a long time to complete, however, if the process seems hung, you can cancel the build and then rerun the failed jobs. To do this, click on the build for the release in the link above, make sure you're logged into Azure Pipelines, and then use the cancel/rerun buttons in the top right of the web page.
At the time this section was written, it was all about the introduction of support for ECDSA and how users can start taking advantage of that support.
Now that we use ECDSA by default, this piece of documentation probably should serve a new purpose. My idea here is to document the new behavior that we have in 2.0: new key type on new certificates, old certificates will keep their existing key type.
Users may now be going in the reverse direction with their changes ("I got an ECDSA certificate but I need RSA because I have an old load balancer appliance!") so I have also updated some section titles to be less about ECDSA and more about Key Types in general.
Fixes#9442.
This PR:
* Deletes the 2.0 pre-release pipeline
* Causes 1.x releases to be released to Docker Hub without updating the latest tag, PyPI, and the candidate and stable channels of the snap store
* Causes 2.x releases to be released to Docker Hub, PyPI, the beta channel of the snap store, and our Windows installer
We could potentially look into how to continue to do 1.x Windows installer releases through GitHub releases and tech ops tooling, but I personally don't think it's worth it right now.
This PR DOES NOT do anything about progressive snap releases. I think we can revisit this when/if we decide (how) to do them.
(cherry picked from commit 09af133af3)
This PR:
* Deletes the 2.0 pre-release pipeline
* Causes 1.x releases to be released to Docker Hub without updating the latest tag, PyPI, and the candidate and stable channels of the snap store
* Causes 2.x releases to be released to Docker Hub, PyPI, the beta channel of the snap store, and our Windows installer
We could potentially look into how to continue to do 1.x Windows installer releases through GitHub releases and tech ops tooling, but I personally don't think it's worth it right now.
This PR DOES NOT do anything about progressive snap releases. I think we can revisit this when/if we decide (how) to do them.
* main: set more permissive umask when creating work_dir
This'll guarantee our working dir has the appropriate permissions,
even when a user has a strict umask
* update changelog
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* certbot-apache: use httpd for newer RHEL derived distros
A change in RHEL 9 is causing apachectl to error out when used
with additional arguments, resulting in certbot errors. The CentOS
configurator now uses httpd instead for RHEL 9 (and later) derived
distros.
* Single CentOS class which uses the apache_bin option
* soothe mypy
* Always call super()._override_cmds()
Fixes#7206.
I think it's about time we did this:
- `dnssec-keygen` on new distros doesn't support the HMAC algorithms anymore, so our instructions don't work.
- The oldest distros we support are Debian Buster (`9.11.5.P4+dfsg-5.1+deb10u7`) and CentOS 7 (`9.11.4-26.P2.el7_9.9`), which ship `tsig-keygen` and support `HMAC-SHA512`.
* Use the TSIG keyring for the initial SOA request
Helps allow the use of keys in BIND ACLs to help certbot update the correct zone. Previously TSIG was only used for zone updates, rather than for both the authoritative SOA request and zone update.
* Update CHANGELOG.md
* Update AUTHORS.md
* Workaround for mypy failure due to dnspython stubs
As per https://github.com/certbot/certbot/pull/9408#issuecomment-1257868864
Co-authored-by: Alex Zorin <alex@zorin.id.au>
* dont superfluously ask whether to renew, when changing key type
* reorder conditions
this prevents "Certificate not yet due for renewal" being printed
* and replace superfluous mock
* mock renewal.should_renew
* Remove external mock dependency
This also removes the "external-mock" test environment
* remove superfluous ignores
* remove mock warning ignore from pytest.ini
* drop deps on mock in oldest, drop dep on types-mock
Co-authored-by: Alex Zorin <alex@zorin.id.au>
There is no need for two interconneced (pipe) processes.
The regular expression in the grep part is not strict enough in some cases (presence of long_description.
sed does not seem to support perl regular expressions ("\s").
Some Python developers prefer single quotes to double qoutes. Some even go so far as to adapt generated templates (setup.py).
This update will (hopefully) fix this all.
This was tested on Ubuntu 20.04.5 LTS (Focal Fossa) and macOS 12.5.1 (Monterey).
* acme: remove Client and BackwardsCompatibleClientV2
* remove ClientTestBase and some unused variables
* add ClientV2.get_directory
* tweak ToS callback code
* acme: update example to use ClientV2.get_directory
* simplify ToS callback further into one step
* further removal of acmev1-related code
- remove acme.client.ClientBase
- remove acme.mixins.VersionedLEACMEMixin
- remove acme.client.DER_CONTENT_TYPE
- remove various ACMEv1 special cases
- remove acme.messages.ChallengeResources.combinations
* remove .mixins.ResourceMixin, fields.resource, fields.Resource
and resource field from various .message classes.
* simplify acme.messages.Directory:
- remove Directory.register
- remove HasResourceType and GenericHasResourceType
- remove ability to look up Directory resources by anything other
than the exact field name in RFC8555 (section 9.7.5)
* remove acme.messages.OLD_ERROR_PREFIX and support the old prefix
* remove acme.mixins
* reorder imports
* add comment to Directory about resource lookups
* s/new-cert/newOrder/
* get rid of `resource` sillyness in tests
* remove acmev1 terms-of-service support from directory
* deprecate more attributes in acme
* Deprecate .Authorization.combinations by renaming the field and
deprecating in getters/setters
* Silence deprecation warnings from our own imports of acme.mixins
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* deps: remove pyjwt dependency
* pinning: strip extras from dependencies
`poetry export` outputs in requirements.txt format, which is now
apparently producing "dep[extra]==...". We are using this output
as the constraints file for pip and pip's new resolver does not
permit extras in the constraints file.
This change filters out the extras specifiers.
* repin current dependencies
* fix new pylint complaints
* silence lint about distutils.version
We have already deprecated the function and it'll be removed in
2.0.
* docs: set sphinx language to 'en'
this is emitting a warning and failing the build
* Revert "pinning: strip extras from dependencies"
This reverts commit 11268fd23160ac53fd8dad7a2ff15e453678e159.
* pin poetry back to avoid extras issue
* repin
* fix new mypy complaints in acme/
* docs: how to override the trusted CA certificates
* Update certbot/docs/using.rst
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Add Code Signing action for Windows Installer
* Clean up variable names and input
* Amend and add to documentation per PR guidelines
* Update tools/finish_release.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update tools/finish_release.py
Amend typo
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Amend release script for better work flow
- SCP commands to upload and download unsigned & signed installers from CSS
* Collapse spaces
* Update tools/finish_release.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Create new windows signer function
* Update Windows Installer Script
- Update change log
- add new function for signing and document
- @TODO Streammline SSH session
* Remove Azure and Github release methods
- Methods moved to CSS
- Reduced to a ssh function that triggers the process on a CSS
* Amend Chnagelog and Remove Unneeded Deps
* Update tools/finish_release.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Add Verison Fetch Function
- For the purpose of snap releases
- Add back package to dev extras for function
* Chaneg path in ssh command
* Amend release script
* Amend the ssh command for CSS
* Update tools/finish_release.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update script with proper path and subprocess call
* Update ssh command
* Correct typo in path
* Fix typo in path
* Update certbot/CHANGELOG.md
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Remove missed conflict text
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* try the easy thing of just doing what the error message says
* temporarily add deploy stage to extended tests to see if it uploads properly
* follow instructions on https://forum.snapcraft.io/t/snapcraft-authentication-options/30473
* just run the packaging jobs for speed
* fix formatting
* import changes from test- branch and revert temporary changes
* Update instructions in deploy-stage.yml
It was previously 5.5 hours, which was just to have an exception thrown
before Azure's 6 hour timeout. Generally we aren't seeing this step take
more than 45 minutes, so 90 minutes seems like more than enough.
* Change `query_registration()` to use `_get_v2_account()`
* Improve `_get_v2_account()`
Required for proper working of `certbot.main.update_registration()`. This
function updates the `regr.body` locally instead of passing the fields
which need to be updated to `acme.client.update_registration()` as a
separate argument in the `update` parameter.
* Revert "Improve `_get_v2_account()`"
This reverts commit e88a23ad76b6dc092645a870b3b5f99bd4fbd095.
* Improve `_get_v2_account() (version 2)
Instead of e88a23a, this change should be more compatible with older
ACMEv1 accounts used through symlinking ACMEv2 account dirs to the
existing ACMEv1 account dirs.
It should also still be compatible with `certbot.main.update_registration`.
* Move and slightly update CHANGELOG entry
* Handle CAA failure on finalize_order during renewal (#9251)
* Fix CAA error on renewal test
* Attempt to fix failing test in CI
* Retry errors with subproblems in obtain_certificate_from_csr with allow_subset_of_names
Only retry if not all domains succeeded
* Back out renewal changes
* Fix linting error line too long
* Update log message for more general case and only log on retry
* Changelog entry
* Add retry logic to order creation
* Changelog entry wording
* Fix acme error handling when no subproblems provided
* Fix test name
* Use summarize domain list to display list of failed domains
* Tidy up incorrect client tests
* Remove unused var and output all failed domains
* Add logging to failed authorization case
* use _retry_obtain_certificate for failed authorizations
* Fix typo failing in CI
* Retry logic comments
* Preserve original error
* Move changelog entry to latest version
* If an installer is provided to certonly, restart after cert issuance
* Add myself to AUTHORS.md
* Handle certonly's "installer" error case
* Handle interactive case, use lazy interpolation
* fix trailing whitespace
* fix whitespace in error message, re-raise exception
* Handle cases where user specified an authenticator but no installer
* make tox happy
* Clarify comment in selection.py
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Add tests for the certonly installer changes
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Add documentation on interactions between multiple views in BIND and the dns_rfc2136 plugin
* Missing ; in example config
* Make lines shorter
* Missed one long line, and move Examples up in the documentation
* Apply suggestions from code review
Co-authored-by: alexzorin <alex@zor.io>
Co-authored-by: alexzorin <alex@zor.io>
* Add subproblems to errors (#7046)
* Fix can't assign attribute
* Tidy up string representations of errors and add decoders for subproblems / identifiers
* Add missing attributes to docstring
* Move change to 1.27.0 in changelog
@osirisinferi pointed out [in chat](https://opensource.eff.org/eff-open-source/pl/y5whp5ny378wuedi8gd7995qbo) that the way this entry was written, suggested that `--new-key` might affect whether `--reuse-key` is set or not.
I think the second sentence was the main culprit, so I've nixed it and replaced it with a reminder about our other flags.
This maybe calls out more for a documentation section but let's fix this quickly before the release.
* Add message to account reg. error
* Changelog
* Remove forced lowercase first char
* Catch errors raised by acme library
* Fix mypy and add some comments
* Add some tests
* Move changelog entry to current version
* Address comments
* Address additional comments
Put everything in this commit instead of using the "Commit suggestion"
feat on Github, which would resolve in 4 different tiny commits.
* Skip ToS agreement question if ToS value is None
* Add changelog entry
* Typo in CHANGELOG
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Typo in CHANGELOG
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* certbot-ci: fix challtestsrv address for boulder-v2
The port is no longer exposed on the Docker host.
* vary the challtestsrv URL by acme server
* fix mypy
* fix comment
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Remove cast for jose.fields.
https://github.com/certbot/certbot/pull/9073 references this.
* Some of them can't be removed, though.
* Fix josepy type hints of json
* Increase josepy pinning version.
Note that the repin scripts have not been used.
* Run repin scripts.
* Fix constraints
* Remove Windows 2016 environment, generate 64 bit installer
* Add note to changelog
* Use win_amd64 as installer suffix
* Bump PYTHON_BITNESS to 64
* Require 64 bit Windows for the installer_build job
* Update certbot install path
* update windows test name
* Base installer suffix on PYTHON_BITNESS again
* Update changelog to request users uninstall old version
* improve handling and ux of unexpected key type migration
* update unit tests
* update integration tests
* if --cert-name and --key-type are set, dont prompt
* Add challenge info to `--debug-challenges`
* Expand/add tests
* Add changelog entry
* Make tests Python 3.6 and 3.7 compatible
* Don't use `config.namespace`
* And don't use `config.namespace` in tests too
* Expand tests to check for token/thumbprint
* Add test for the DNS-01 challenge
Changed the Apache authenticator to the manual authenticator. Doesn't
seem to make a difference to the tests, but makes more sense if the
DNS-01 challenge is being used.
* Reword changelog entry
* Mention feature in --help output
* Better variable assignment in test
Co-authored-by: alexzorin <alex@zor.io>
* Better variable assignment in test
Co-authored-by: alexzorin <alex@zor.io>
* Remove unnecessary `verbose_count` assignment
Co-authored-by: alexzorin <alex@zor.io>
* Use terminology from RFC 8555
* Compress the two new tests into one
* s/world wide web/internet
* Move new code into separate function
* Remove superfluous newline with mixed challs
Co-authored-by: alexzorin <alex@zor.io>
I think test_apache2.sh still has value as it allows us to test our Apache plugin with the Apache layouts found on different OSes. Unfortunately, many of the OSes we're currently testing against don't have Python 3.7+ packaged yet we still support these OSes through things like snap where we bundle our own version of Python.
To allow us to continue testing on these OSes, I switched to installing Python through pyenv. I also took the opportunity to clean up the scripts, removing a lot of code, failing more quickly, and simplifying failure logic in test_apache2.sh.
The reason I want to do this is many of the targets of `test_sdists.sh` use Python 3.6 which [has reached its EOL](https://www.python.org/dev/peps/pep-0494/#lifespan). We could instead just stop running the test on these systems or install a newer version of Python 3 outside of OS packaging, but instead I decided to look into why we have these tests to begin with.
I introduced these tests many years ago in https://github.com/certbot/certbot/pull/4089 as a fix for https://github.com/certbot/certbot/issues/4044. Essentially the problem was the way packagers ran tests and the way we ran tests were slightly different. This difference could cause test failures when distros tried to run tests on our packages.
Since I did this, [we've switched to telling packagers to run tests using `pytest` like we do](5e76669c50/certbot/docs/packaging.rst (notes-for-package-maintainers)) and we've greatly reduced our reliance on OS packaging through things like `snap`.
Because of this, I think we should stop running this test, reducing our reliance on the heavy "test farm tests", and simplifying our CI pipeline. I think future problems here is quite unlikely and even if we have them, it should only affect tests on our non-primary distribution mechanisms which I think is a very minor concern.
When reviewing this PR, it's probably worth noting that I just replaced `targets.yaml` with the contents of `apache2_targets.yaml` since the Apache 2 tests are the only runs we're running with this change.
* Revert setuptools-rust pin
This was a temporary workaround to fix
https://github.com/certbot/certbot/issues/9111, but it looks like the
the issue resolved itself
* Make mypy happy
There was an unused ignore statement, and Validator.certificate was
unnecessarily casting strings as bytes for an X509 digest method.
* Pin setuptools-rust to prevent build-dep hiccups in the future
* Add support for revoking ecdsa keys without --cert-name.
Co-Authored-By: commonism <commonism@users.noreply.github.com>
* Move alg to acme_client.ClientNetwork instantiating in acme_from_config_key
* Fix argument for RS256/ES256
* Support also ES384 and ES512 signing algorithms.
* Work in progress
* Work in progress
* Work in progress
* Work in progress
* Fix issues around nullability of VirtualHost.path, may discuss that during review
* Work in progress
* Fix remaining types
* Various lint fixes
* Reconfigure tox and mypy to disallow untyped defs globally
* Cleanup compatibility tests
* Use cast for unused v2 logic
* Improve types
* Remove unused comment
* Fix coverage
* Better types
* Fix another type
* Update certbot-apache/certbot_apache/_internal/apacheparser.py
Co-authored-by: alexzorin <alex@zor.io>
* Update certbot-apache/certbot_apache/_internal/assertions.py
Co-authored-by: alexzorin <alex@zor.io>
* Fix type
* Various fixes
* Refactor imports
* Keep naming convention consistent on TypeVars
* Improve types
* Improve types
* Remove remaining Sequence[str] in the project
Co-authored-by: alexzorin <alex@zor.io>
* Ignore SOA TTL in favor of explicit TTL argument
`domain.ttl` should be `None` so that the `self.ttl` argument in
`add_txt_record()` is not ignored (`domain.ttl` takes precedence).
* Document mitigation for dns-digitalocean ignoring the 30 second TTL.
* Add generic methods to save some casts, and fix lint
* Update current and oldest pinning
* Fix classes
* Remove some todos thanks to josepy 1.11.0
* Cleanup some useless pylint disable
* Finish complete typing
* Better TypeVar names
* Upgrade pinning and fix some typing errors
* Use protocol
* Fix types in apache
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Fixes https://github.com/certbot/certbot/issues/8983
Python 3.6 is now EOL: https://endoflife.date/python
This is normally a good time to create warnings about Python 3.6 deprecation the Certbot upcoming release 1.23.0 so that its support is removed in 1.24.0.
We have to say here that EPEL maintainers asked us to keep maintaining support of Python 3.6 because Python 3.7 will never be shipped to CentOS 7. This support would be needed in theory up to 2 more years, basically until CentOS 7 EOL in 2024-06-30. It has been said that we could support as a best effort until a reasonable need on Certbot side requires to drop Python 3.6. See https://github.com/certbot/certbot/issues/8983 for more information.
However some of us (including me) consider that there is already a reasonable need right now. Indeed, keeping the support on Python 3.6 while the Python community globally moves away from it will pin implicitly some Certbot dependencies to the last version of these dependencies supporting Python 3.6 as the upstream maintainers decide to make the move. At any point in a future time, one of these dependencies could require an urgent upgrade (typically a critical uncovered vulnerability): then we would require to drop Python 3.6 immediately without further notice instead of following an organized deprecation path.
This reason motivates to proactively deprecate then drop the Python versions once they are EOL. You can see the discussion in Mattermost starting from [this post](https://opensource.eff.org/eff-open-source/pl/ntzs9zy1fprjmkso3xrqspnoce) to get more elements about the reasoning.
* Deprecate Python 3.6 support.
* Ignore our own PendingDeprecationWarning
* Improve assertions in certbot-apache tests.
Replacements inspired by flake8-assertive.
* Fix test failures
* assertEqual is not for None :D
* Pass all tests :)
* Fetch and print account contacts from ACME server
* Add tests
* Add changelog entryAdd changelog entry
* Add account URI and thumbprint output
Only show these items when verbosity > 0
* Add test case for account URI and thumbprint
* Move changelog entry to new placeholder
* Add test for `cb_client.acme` (coverage)
* Address comments
* Update changelog
* Few small word changes
* Add server to error messages
* Remove phone contact parts
* Add types in all DNS plugins
* Order imports
* Fix type
* Update certbot-dns-route53/certbot_dns_route53/_internal/dns_route53.py
Co-authored-by: alexzorin <alex@zor.io>
* Clean up imports
Co-authored-by: alexzorin <alex@zor.io>
The `# self.comment = comment` caught my eye while working on #9071 as well as the intermediate variables, which aren't really needed. As a result, I reformatted the code slightly in those places.
* Remove comment in AugeasCommentNode.__init__
* Replace some intermediate varibles with return-statements in apache augeas parser.
* more clean-up
* Added --issuance-timeout command line option
* clarification of command line option name,docstring and add tests
* fix test case for python36
* improved the command line options
This was deprecated in version 2.1 and cryptography will be
removing it soon. The replacement function is available in all
versions of cryptography that certbot supports (2.1+)
* Certificate issuing on Window while having web.confing and more then one domain in request
* add a test
* update changelog
Co-authored-by: Serghei Trufkin <Serghei.Trufkin@Technosoft.md>
* docs: describe how to modify renewal config
* Apply suggestions from code review
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* reword warning about manual modifications
* explain the flags in the --force-renewal command
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Fixes https://github.com/certbot/certbot/issues/9058.
The changes to the CI config are equivalent to the ones made in https://github.com/certbot/certbot/pull/8460.
Other than ignoring some warnings raised by botocore, the main additional work that had to be done here was switching away from using `distutils.version.LooseVersion` since the entire `distutils` module was deprecated in Python 3.10. To do that, I took a few different approaches:
* If the version strings being parsed are from Python packages such as Certbot or setuptools, I switched to using [pkg_resources.parse_version](https://setuptools.pypa.io/en/latest/pkg_resources.html#parsing-utilities) from `setuptools`. This functionality has been available since [setuptools 8.0 from 2014](https://setuptools.pypa.io/en/latest/history.html#id865).
* If the version strings being parsed are not from Python packages, I added code equivalent to `distutils.version.LooseVersion` in `certbot.util.parse_loose_version`.
* The code for `CERTBOT_PIP_NO_BINARY` can be completely removed since that variable isn't used or referenced anywhere in this repo.
* add python 3.10 support
* make some version changes
* don't use looseversion in setup.py
* switch to pkg_resources
* deprecate get_strict_version
* fix route53 tests
* remove unused CERTBOT_PIP_NO_BINARY code
* stop using distutils in letstest
* add unit tests
* more changelog entries
* Start more types
* Second run
* Work in progress
* Types in all acme module
* Various fixes
* Various fixes
* Final fixes
* Disallow untyped defs for acme project
* Fix coverage
* Remote unecessary type ignore
* Use Mapping instead of Dict as input whenever it is possible
* Update acme/acme/client.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update acme/acme/client.py
Co-authored-by: alexzorin <alex@zor.io>
* Various fixes
* Fix code
* Fix code
* Update acme/acme/client.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update acme/acme/challenges.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update acme/acme/client.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Fix deactivate_registration and simplify signature of update_registration
* Do not leak personal data during account deactivation
* Clean more Dicts
* New fix to not leak contact field in the account deactivation payload.
* Add ignore for python 3.6 type check
* Revert "Add ignore for python 3.6 type check"
This reverts commit da7338137b798e3ace34de15ed12f76ec3cf3888.
* Let's find a smarter way than "type: ignore"
* Update certbot/certbot/_internal/account.py
Co-authored-by: alexzorin <alex@zor.io>
* Fix an annotation
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: alexzorin <alex@zor.io>
* Generate a web.config file to serve properly challenge files with IIS
* Fix cleanup, add test
* FIx lint
* Do not overwrite existing web.config. Delete only web.config when it has been created by Certbot and is unmodified.
* Fix lint
* Update certbot/certbot/_internal/plugins/webroot.py
Co-authored-by: alexzorin <alex@zor.io>
* Add log
* Check for POSIX_MODE before web.config deletion attempt.
* Add documentation
* Update certbot/CHANGELOG.md
Co-authored-by: alexzorin <alex@zor.io>
* Update certbot/docs/using.rst
Co-authored-by: alexzorin <alex@zor.io>
It seems that all required pre-compiled wheels to install Certbot on Python 3.9 on Windows are present.
This PR upgrades Windows tests on Python 3.9 and repackages the installer on this version of Python.
This PR upgrades the pinned version of the dependencies. Version `1.9.0` of josepy is used so errors related to JWK serialization with EC keys (see https://github.com/certbot/josepy/issues/109) are fixed for Certbot.
@osirisinferi noticed [in chat](https://opensource.eff.org/eff-open-source/pl/sa85u4n71tywfpc15c1wu59wae) that "NEXT STEPS:" was ignoring `--quiet` and was being printed unconditionally.
I think it ended up being written this way in #8860 because I was trying not to avoid dumping ANSI escapes and newlines into the log file and confused myself in the process.
This change makes things a bit more explicit in separating presentation/message.
* fix 'NEXT STEPS' being printed to stdout during -q
* fix tests
Issuing a certificate with --quiet was crashing during the donation
atexit call because it was trying to use the /dev/null fd after the
displayer context manager had already closed it.
Due to macOS having some complications about Certbot from Homebrew being
in the PATH, the instructions we have in the Automated Renewal section
do not work for them. Instead, send those users to the instruction
generator.
While the previous approach of testing the functionality of snapctl
worked, the snapd developers told us they could not guarantee its
reliability.
---
As with #8955, I tested this on Debian 9, 10 and CentOS 7, 8, Stream.
* Fix some typos (found by codespell)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
* Remove typo fixes for some files which should not be modified
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Fixes https://github.com/certbot/certbot/issues/6844.
This PR does two things:
1. Changes ACMEv1 deprecation warnings from `PendingDeprecationWarning` to `DeprecationWarning`.
2. Changes the ACMEv1 deprecation warnings to be on references to the class themselves. This is the approach taken in https://github.com/certbot/certbot/pull/8989, the PRs linked there, and the `cryptography` code in the code comment. I think this approach warns in more cases and I updated our unit tests to avoid hitting these warnings.
* add ip address support to acme saving
* remove client-site check for ip address
* using right prefix for san parsing
* remove type hint for backward compatibility
* remove bare ip blocking check from main_test
* upppercase
* lint tix
* add additional tests for new IP support
* support for ipv6 bare address
* make apache and nginx plugin raise error for certs with ip address
* linting
* add pem file's last newline char
* gen_ss_cert ip support and comment fixup
* fix test coverage
* indent fix and assetTrue to assetIN
* indent mistake, made a note where class end
* acme lib now receive IPs as separate list
* fix typos
* type 2
* fix tests
* Deny IP address on certbot/certbot side as LE don't support it
* remove excess empty line to rerun tox
* comment indent and typo fix
Apply suggestions from code review
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* trim unused functions
* trim unused import
* make raw san list extraction as separate function
* Apply suggestions from code review
mostly comment suggestions here
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* apply patches suggested on review.
* remove excessive empty lines
* update CHANGELOG.md
* added acme lib update about ipaddress support in CHANGELOG.md
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Raise separate error when a hostname is being used for `dns_rfc2136_server`
* Explicitly say IP address instead of hostname in docs
* Don't catch ValueError, but actually check the server value
* Add tests
* Add CHANGELOG entry
This PR removes all zope dependencies from plugins configuration.
It also lets Sphinx upgrade to the next major version by removing the plugin dedicated to zope interfaces documentation. As a consequence, the deprecated zope interfaces are not documented anymore.
* Cleanup zope dependencies in plugins and upgrade sphinx
* Update pinnings
As a follow-up to #8971, this PR removes all references to the old Zope interfaces, except the ones used to deprecate them and prepare for their removal.
In the process, some documentation and tests about the `Display` objects are simply removed since they are not relevant anymore given that they are removed from the public API.
* Cleanup some interfaces.IInstaller
* Cleanup IConfig doc
* Allmost complete removal
* Remove useless tests
* Fixes
* More cleanup
* More cleanup
* More cleanup
* Remove a non existent reference
* Better type
* Fix lint
Fixes#8899
This PR removes the pinning upper limit of mypy currently set to <0.900 and adds the required types-* stub packages to make recent versions of mypy work.
* Unpin mypy
* Improve type in TempHandler
* Add types
I want this for #8949.
I think this is quite verbose, but purposefully so as an intervention to try prevent users from hitting this problem. It's more of a "How-To Guide" than a "Reference Guide" (in the lingo of https://documentation.divio.com).
* docs: add "Deleting Certificates" to user guide
* try a less convoluted explanation
about what the installer did in the first place
* add a warning early on: read the full thing
* erica's copy changes
* rewrite as a how-to guide
* rewrite self-signed step 2 for mental model++
* rewrite intro to "safely deleting certificates"
[Snapcraft 5.0](https://forum.snapcraft.io/t/release-notes-snapcraft-5-0/25751) implemented creating build IDs based on the project's contents instead of the directory path in https://github.com/snapcore/snapcraft/pull/3554. This is a feature we initially wanted, but it broke our workaround added in https://github.com/certbot/certbot/pull/8719. Our workaround is broken because now that the build ID is based on the project's contents, copying the project to a temporary directory has no effect.
This PR removes the workaround from https://github.com/certbot/certbot/pull/8719 and instead constructs a random build ID that it provides to snapcraft. This provides us with even more randomness to avoid build ID conflicts while avoiding having to copy the project to a temporary directory before every build.
* improve-remote-build
* use lowercase letters
* BF: apache cfg parsing - relax assumption that value cannot contain =
* Remove failing test_update_runtime_vars_bad_output
* Add test Define statements: with = in value, and an empty value
* update CHANGELOG
Co-authored-by: Alex Zorin <alex@zorin.id.au>
`distro.linux_distribution` was deprecated (https://github.com/python-distro/distro/pull/296) in the release of `distro` at the end of last week. The deprecation is causing the `nopin` nightly tests to fail.
This change migrates Certbot off that function.
As far as I can tell, the Arch Linux edge case described in the code comments no longer happens, but better to be safe than sorry I think.
* stop using deprecated distro.linux_distribution
* update comment
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
This PR is a new approach for fixing #8732 based on the discussions occurred in the first PR #8877.
This PR upgrades python-augeas to the latest version, and avoids tests failure of Windows because of this upgrade. To do so it leverages the [tox multi-platform feature](https://tox.readthedocs.io/en/latest/example/platform.html) and modifications to `tools/venv.py` in order to not install and not test `certbot-apache` on Windows.
* Unpin python-augeas and upgrade current pinnings
* Do not install certbot-apache in Windows dev environments
* Introduce tox specific win packages + remove certbot compatibility on windows
* Add libaugeas to sphinx build
* Redefine lint and mypy targets
* Keep the lint and mypy environments
* acme: deprecate ACMEv1 client classes
Adds pending deprecations to:
- acme.client.Client
- acme.client.BackwardsCompatibleClientV2
Adds a warning to Certbot when a v1 server is detected.
* move thsi change from 1.17 to 1.18
* revert some whitespace changes
While bumping pinned packages in #8928, we came across a new version of pylint (2.9.3). Upgrading to this version requires some changes to Certbot's code, which is what this change is about.
* pylint: upgrade pinned verson and fix new lints
* maxsplit should be 1, not -1, for rsplit
* docs: explain the situation with --manual renewal
* note that the non-hook command can't be cronned
* add xref to #renewing-certificates
* update manual description in the plugins table
* redirect manual users towards other plugins
* refer to authentication hook scripts in table
In the apache2 package on Debian-based distros, the default
000-default.conf virtual host does not include a ServerName.
Depending on the FQDN hostname of the machine and DNS setup, Apache
assigns a name to this unnamed vhost at runtime. As a result, the
Apache config end up with vhosts that have duplicative names.
Previously, Certbot did not identify that the nameless vhost could be
a match for the requested identifier, which would, depending on
configuration load order, cause the authenticator to fail.
This change causes Certbot to include all unnamed vhosts on top of
matched vhosts, during authentication. If no vhosts matched, the
existing behavior remains the same.
* apache: configure nameless vhosts during auth
* vhost is only unnamed if ServerName is not set
* also fix test to only match ServerName
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* cli: vary renewal advice for hookless manual certs
1. Don't print that the certificate will be automatically renewed,
because it won't be.
2. Add a "NEXT STEP" telling the user that they will need to manually
re-issue the certificate in order to renew it.
* kill superfluous comma
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* clarify wording of the next step
* fix the test
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Also, update `dev-cli.ini` example to use new flag.
Although https://github.com/bw2/ConfigArgParse/pull/216 allowed setting a `count` action value in a config file, our default detection system won't let us use that functionality. While we should eventually fix that, for now, let developers have a cli.ini with a higher logging level by adding this flag.
Note that this flag is intended to work the same way adding `-vvv`s does; that is, as a modifier to the pre-set level, rather than setting the absolute level. The number it is set to is equivalent to the number of `v`s that would otherwise have been passed, with "2" as the current maximum effective number of levels (warning --> info --> debug).
* Add --verbose-level flag for devs to set in cli.ini
* Update dev-cli.ini to use new flag
* use poetry 1.2.0a1
* pin pip normally
* use normal constraints file with pipstrap
* remove unused STRIP_HASHES var
* Check for old poetry versions
* keep pip, setuptools, and wheel pinned in oldest
* remove strip hashes
* pin back pip
* fix new lint error
* Deprecate usage of IConfig as a singleton in Certbot
* Fix local oldest requirements
* Add changelog
* Add tests for certbot.crypto_util.init_save_* functions
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Add instructions for setting up a cronjob in the docs
* Be more specific about where the cron entry will be created
Co-authored-by: alexzorin <alex@zorin.id.au>
* Correct &s to &s
Co-authored-by: alexzorin <alex@zorin.id.au>
* Correct other & to &
Co-authored-by: alexzorin <alex@zorin.id.au>
* De-weasel the double-scheduled-task comment
Co-authored-by: alexzorin <alex@zorin.id.au>
* Have users create directory hooks instead of command line hooks
* Use sudo in command
Co-authored-by: alexzorin <alex@zorin.id.au>
* tell windows users to ignore these instructions instead of telling them they won't work
* Use the same commands that we have in the general instructions
Co-authored-by: alexzorin <alex@zorin.id.au>
* later printing of renewal and install retry advice
Move printing of advice for automated renewal, and retrying installation
in case of failure, towards the end of `run` and `certonly`.
Also adds some renewal advice for the --csr case (no autorenewal).
* update renewal advice for preconfigured-renewal
* rewrite in terms of "NEXT STEPS" for run/certonly
* fix lint
* re-add "Could not install certificate"
* update --csr renewal advice
* rewrite non-preconfigured-renewal renewal advice
* Remove no names found in configuration files because it sounds like an error but actually it is fine
* fix test
* Pose question more grammatically and specifically, and remove extra space
* fix lint
Co-authored-by: Alex Zorin <alex@zorin.id.au>
1.10.0 was a bad release and this breaks our oldest Boulder tests.
I bumped the version to 1.10.0 in #8852 to get access to a new public display_util API, but that was the release with the broken deprecation of `--manual-public-ip-logging-ok`. So let's bump it to 1.10.1.
Streamline and reorganize Certbot's CLI output.
This change is a substantial command-line UX overhaul,
based on previous user research. The main goal was to streamline
and clarify output. To see more verbose output, use the -v or -vv flags.
---
* nginx,apache: CLI logging changes
- Add "Successfully deployed ..." message using display_util
- Remove IReporter usage and replace with display_util
- Standardize "... could not find a VirtualHost ..." error
This changes also bumps the version of certbot required by certbot-nginx
and certbot-apache to take use of the new display_util function.
* fix certbot_compatibility_test
since the http plugins now require IDisplay, we need to inject it
* fix dependency version on certbot
* use better asserts
* try fix oldest deps
because certbot 1.10.0 depends on acme>=1.8.0, we need to use
acme==1.8.0 in the -oldest tests
* cli: redesign output of new certificate reporting
Changes the output of run, certonly and certonly --csr. No longer uses
IReporter.
* cli: redesign output of failed authz reporting
* fix problem sorting to be stable between py2 & 3
* add some catch-all error text
* cli: dont use IReporter for EFF donation prompt
* add per-authenticator hints
* pass achalls to auth_hint, write some tests
* exclude static auth hints from coverage
* dont call auth_hint unless derived from .Plugin
* dns fallback hint: dont assume --dns-blah works
--dns-blah won't work for third-party plugins, they need to be specified
using --authenticator dns-blah.
* add code comments about the auth_hint interface
* renew: don't restart the installer for dry-runs
Prevents Certbot from superfluously invoking the installer restart
during dry-run renewals. (This does not affect authenticator restarts).
Additionally removes some CLI output that was reporting the fullchain
path of the renewed certificate.
* update CHANGELOG.md
* cli: redesign output when cert installation failed
- Display a message when certificate installation begins.
- Don't use IReporter, just log errors immediately if restart/rollback
fails.
- Prompt the user with a command to retry the installation process once
they have fixed any underlying problems.
* vary by preconfigured_renewal
and move expiry date to be above the renewal advice
* update code comment
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* update code comment
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* fix lint
* derve cert name from cert_path, if possible
* fix type annotation
* text change in nginx hint
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* print message when restarting server after renewal
* log: print "advice" when exiting with an error
When running in non-quiet mode.
* try fix -oldest lock_test.py
* fix docstring
* s/Restarting/Reloading/ when notifying the user
* fix test name
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* type annotations
* s/using the {} plugin/installer: {}/
* copy: avoid "plugin" where possible
* link to user guide#automated-renewals
when not running with --preconfigured-renewal
* cli: reduce default logging verbosity
* fix lock_test: -vv is needed to see logger.debug
* Change comment in log.py to match the change to default verbosity
* Audit and adjust logging levels in apache module
* Audit and adjust logging levels in nginx module
* Audit, adjust logging levels, and improve logging calls in certbot module
* Fix tests to mock correct methods and classes
* typo in non-preconfigured-renewal message
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* fix test
* revert acme version bump
* catch up to python3 changes
* Revert "revert acme version bump"
This reverts commit fa83d6a51c.
* Change ocsp check error to warning since it's non-fatal
* Update storage_test in parallel with last change
* get rid of leading newline on "Deploying [...]"
* shrink renewal and installation success messages
* print logfile rather than logdir in exit handler
* Decrease logging level to info for idempotent operation where enhancement is already set
* Display cert not yet due for renewal message when renewing and no other action will be taken, and change cert to certificate
* also write to logger so it goes in the log file
* Don't double write to log file; fix main test
* cli: remove trailing newline on new cert reporting
* ignore type error
* revert accidental changes to dependencies
* Pass tests in any timezone by using utcfromtimestamp
* Add changelog entry
* fix nits
* Improve wording of try again message
* minor wording change to changelog
* hooks: send hook stdout to CLI stdout
includes both --manual and --{pre,post,renew} hooks
* update docstrings and remove TODO
* add a pending deprecation on execute_command
* add test coverage for both
* update deprecation text
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: Alex Zorin <alex@zorin.id.au>
Co-authored-by: alexzorin <alex@zor.io>
This is part of https://github.com/certbot/certbot/issues/8782. I took it on now because the currently pinned version of `pylint` doesn't work with newer versions of `poetry` which I wanted to upgrade as part of https://github.com/certbot/certbot/issues/8787.
To say a bit more about the specific changes in this PR:
* Newer versions of `pylint` complain if `Popen` isn't used as a context manager. Instead of making this change, I switched to using `subprocess.run` which is simpler and [recommended in the Python docs](https://docs.python.org/3/library/subprocess.html#using-the-subprocess-module). I also disabled this check in a few places where no longer using `Popen` would require significant refactoring.
* The deleted code in `certbot/certbot/_internal/renewal.py` is cruft since https://github.com/certbot/certbot/pull/8685.
* The unused argument to `enable_mod` in the Apache plugin is used in some over the override classes that subclass that class.
* unpin pylint and repin dependencies
* disable raise-missing-from
* disable wrong-input-order
* remove unused code
* misc lint fixes
* remove unused import
* various lint fixes
Fixes#8824
This PR makes the installer first delete (if exist) the previous `pkg` directory in the Certbot installation in order to avoid dependencies conflicts when a new version of Certbot (with new versions of dependencies) is intaller other an existing one.
I took the simplest approach here, which is to delete specifically the directories known to create conflicts, instead of more complex approaches that involve to factor in some way the complete uninstaller logic. This is because the complexity added without a clear improvement does not worth it in my opinion. More specifically:
* factorizing in some way the uninstaller section in the NSIS template make the installer use any potential new logic of a new installation of Certbot instead of the one applying for the current installation, and may create unexpected errors during installation or at runtime
* calling the existing `uninstaller.exe` would be better, but I could not find a proper way to let NSIS wait for the actual end of the uninstall logic, and again may create unexpected errors during installation or at runtime
* Cleanup Certbot pkg dir before installing to avoid dependencies conflicts
* Add a changelog
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Apache test farm tests started failing last night due to a change in pyenv. See https://dev.azure.com/certbot/certbot/_build/results?buildId=3948&view=logs&j=f67c2a39-2c4f-5190-915f-6f32a7a4306f&t=96f0f394-f513-5158-f5e7-a26e55aeadbf&l=26943.
I managed to fix that in d94f20f8b7, however, the OSes the tests were failing on were Debian 9 and Ubuntu 16.04. [Debian 9 reached its end-of-life in July 2020](https://wiki.debian.org/DebianReleases) and [Ubuntu 16.04 reached its end of standard support in April 2021](https://wiki.ubuntu.com/Releases). As shown at the same links, Debian 9 still has support from the LTS team and Ubuntu 16.04 has ESM support. Do we still want to support either of these OSes?
If so, we can use the commit I linked in the first sentence of the last paragraph, but I think supporting the OSes through their standard support is good enough. The Certbot team has enough on their plate and especially when the OSes are so old that we can't even use their packaged version of Python anymore which complicates our tests, I think we can just drop support and move on.
I don't have a strong opinion here though so if someone else does, let me know what you'd like to see or make the PR yourself based on the changes in my linked commit and I'll merge it.
You can see the tests passing with this change at https://dev.azure.com/certbot/certbot/_build/results?buildId=3955&view=results.
* Remove apache tests on old OSes
* remove unused pyenv code
I think we should use our `pip_install*` scripts wherever we can and I'm not quite sure yet if I'd call `repoze.sphinx.autointerface` unmaintained.
* use pip_install_editable
* update sphinx comment
Since Saturday the CI pipeline is failing due to several Sphinx errors. See https://dev.azure.com/certbot/certbot/_build/results?buildId=3928&view=logs&j=d74e04fe-9740-597d-e9fa-1d0400037dfd&t=dde413a4-f24c-59a0-9684-e33d79f9aa02
First, the build of certbot-dns-google is failing because of a particular configuration. It seems that this configuration has been written here to activate the support of the RST instruction `.. code-block:: json` in documentation. However, it does not seem to be necessary for a similar situation in certbot-dns-route53 documentation. So let's try to remove it and fix the Sphinx builds.
Second, Sphinx builds were not pinning dependencies, so Sphinx 4.x (that has been released yesterday) started to be used in the pipeline. Sadly this new version is not compatible with the plugin `repoze.sphinx.autointerface`, used to extract documentation from `zope.interface`. So I fixed the pinning and also explicitly pin Sphinx to 3.5.x for now.
Technically speaking the second action is sufficient to fix the first error, but I keep the dedicated solution because it improves the documentation in my opinion.
This situation could be fixed by not requiring `repoze.sphinx.autointerface`, but this is possible only if we remove `zope.interface` from Certbot. Luckily I started the work few days ago ;).
* Remove explicit lexer call in certbot-dns-google doc builds.
* Write a valid JSON file in the documentation
* Apply constraints to sphinx build environments
* Pin Sphinx to 3.5.4
* Update dependencies
* Pin traitlets
Fixes https://github.com/certbot/certbot/issues/8832.
[These instructions are creating confusion among users](https://github.com/certbot/certbot/issues/8832) and [frustration among packagers](https://pagure.io/fesco/issue/2570) for whom the warning at the top of the OS packaging section doesn't apply. Because of this, I think we should remove them in favor of our instruction generator and snap/docker/pip instructions.
I also told Fedora packagers that we could probably do this in response to them continuing to improve their Certbot packages which they've done through things like the renewal timer that is now enabled by default.
Fixes https://github.com/certbot/certbot/issues/8781.
This PR makes our test farm tests into a normal package so it and its dependencies can be tracked and installed like our other packages.
Other noteworthy changes in this PR:
* Rather than continuing to place logs in your CWD, they're placed in a temporary directory that is printed to the terminal.
* `tests/letstest/auto_targets.yaml` was deleted rather than renamed because the file is no longer used.
* make a letstest package
* remove deleted deps
* fix letstest install
* add __init__.py
* call main
* Explicitly mention activating venv
* rerename file
* fix version.py path
* clarify "this"
* Use >= instead of caret requirement
* Update assertTrue/False to Python 3 precise asserts
* Fix test failures
* Fix test failures
* More replacements
* Update to Python 3 asserts in acme-module
* Fix Windows test failure
* Fix failures
* Fix test failure
* More replacements
* Don't include the semgrep rules
* Fix test failure
* Move version.py to tests/letstest since it's used by test_sdists.sh
* Delete unused components of certbot-auto
* Remove test_leauto_upgrades.sh and references to it
* Remove test_letsencrypt_auto_certonly_standalone.sh and references to it
* Remove outstanding references to certbot-auto
* Remove references to letsencrypt-auto
* find certbot in the correct directory
* delete letsencrypt-auto-source line from .isort.cfg since that directory no longer contains any python code
* remove (-auto) from certbot(-auto)
* delete line from test
* Improve style for version.py
Fixes#8802.
Also removed the unused `kgs` cruft while I was here, since it's leftover from the [initial release commit](3c08b512c3) and I'm pretty sure we don't use that anymore.
* Expand manual DNS challenge instructions
* Less jargon
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Less is more
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Make more clear where to look at Googles Toolbox
* Reshuffle text
* Show verify instructions only on last dns-01 challenge
* Swap domain and value
* Remove '(also)'
* Fix DNS verify message for mixed challenge types
* Add a lengthy comment about why there's a full stop after `{domain}`
* Typo
Co-authored-by: ohemorange <ebportnoy@gmail.com>
In https://github.com/certbot/certbot/pull/8748#discussion_r605457670 we discussed about changing the dict used to set OS options for Apache configurators into a dedicated object.
* Create _OsOptions class to configure the os specific options of the Apache configurators
* Fix tests
* Clean imports
* Fix naming
* Fix compatibility tests
* Rename a class
* Ensure restart_cmd_alt is set for specific OSes.
* Add docstring
* Fix override
* Fix coverage
This is one of the things that newer versions of `pylint` complains about.
* git grep -l super\( | xargs sed -i 's/super([^)]*)/super()/g'
* fix spacing
I think this PR improves tools/snap/build_remote.py's output in a number of ways such as:
* Logs of snap builds were being deleted because they weren't being copied out of the temporary directory added in https://github.com/certbot/certbot/pull/8719.
* The lock should now always be acquired before printing output when multiple processes are running which helps prevent processes mixing their output with each other.
* Output is never buffered which ensures that repeated calls to `print` from the same process while it holds the output lock is kept together.
* The case where we printed output about the "chroot problem" and stopped retrying the build has been deleted because with the fix in https://github.com/certbot/certbot/pull/8719, we should be able to recover in this case.
* If the build failed for any reason, we dump as much output about the problem as we can. I think most times we won't need to read this output, but I personally prefer it being there in case we want it for some reason. Due to this change, I also simplified `_build_snap` and `_dump_results` a bit since `_build_snap` handles printing logs as needed.
* print more output
* lock when printing output
* clarify purpose of lock
* preserve logfiles
* python better
* consistently flush output
* remove workspaces dict
* rename variable
* remove unused variable
* don't use all which exits early
* fix typo
Built on top of #8748, this PR reenables mypy strict mode and adds the appropriate corrections to pass the types checks.
* Upgrade mypy
* First step for acme
* Cast for the rescue
* Fixing types for certbot
* Fix typing for certbot-nginx
* Finalize type fixes, configure no optional strict check for mypy in tox
* Align requirements
* Isort
* Pylint
* Protocol for python 3.6
* Use Python 3.9 for mypy, make code compatible with Python 3.8<
* Pylint and mypy
* Pragma no cover
* Pythonic NotImplemented constant
* More type definitions
* Add comments
* Simplify typing logic
* Use vararg tuple
* Relax constraints on mypy
* Add more type
* Do not silence error if target is not defined
* Conditionally import Protocol for type checking only
* Clean up imports
* Add comments
* Align python version linting with mypy and coverage
* Just ignore types in an unused module
* Add comments
* Fix lint
* Work in progress
* Finish type control
* Isort
* Fix pylint
* Fix imports
* Fix cli subparser
* Some fixes
* Coverage
* Remove --no-strict-optional (obviously...)
* Update certbot-apache/certbot_apache/_internal/configurator.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update certbot/certbot/_internal/display/completer.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Cleanup dns_google
* Improve lock controls and fix subparser
* Use the expected interfaces
* Fix code
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Fixes#8773
I took option 2 from the issue mentionned above (importing `typing-extensions` on dev dependencies) to avoid modifying certbot runtime requirements given that what needs to be added is useful for mypy only.
I did not change the Python version used to execute the linting and mypy on the standard tests, given that the tox `docker_dev` target already checks if the development environment is working for Python < 3.8.
Let's begin. All pipelines are defined in `.azure-pipelines`. Currently there are two:
*`.azure-pipelines/main.yml` is the main one, executed on PRs for master, and pushes to master,
*`.azure-pipelines/advanced.yml` add installer testing on top of the main pipeline, and is executed for `test-*` branches, release branches, and nightly run for master.
*`.azure-pipelines/main.yml` is the main one, executed on PRs for main, and pushes to main,
*`.azure-pipelines/advanced.yml` add installer testing on top of the main pipeline, and is executed for `test-*` branches, release branches, and nightly run for main.
Several templates are defined in `.azure-pipelines/templates`. These YAML files aggregate common jobs configuration that can be reused in several pipelines.
@@ -64,7 +64,7 @@ Azure Pipeline needs RW on code, RO on metadata, RW on checks, commit statuses,
RW access here is required to allow update of the pipelines YAML files from Azure DevOps interface, and to
update the status of builds and PRs on GitHub side when Azure Pipelines are triggered.
Note however that no admin access is defined here: this means that Azure Pipelines cannot do anything with
protected branches, like master, and cannot modify the security context around this on GitHub.
protected branches, like main, and cannot modify the security context around this on GitHub.
Access can be defined for all or only selected repositories, which is nice.
```
@@ -91,11 +91,11 @@ grant permissions from Azure Pipelines to GitHub in order to setup a GitHub OAut
then are way too large (admin level on almost everything), while the classic approach does not add any more
permissions, and works perfectly well.__
- Select GitHub in "Select your repository section", choose certbot/certbot in Repository, master in default branch.
- Select GitHub in "Select your repository section", choose certbot/certbot in Repository, main in default branch.
- Click on YAML option for "Select a template"
- Choose a name for the pipeline (eg. test-pipeline), and browse to the actual pipeline YAML definition in the
:warning: __Pipeline has failed__: [Link to the build](https://dev.azure.com/$(Build.Repository.ID)/_build/results?buildId=$(Build.BuildId)&view=results)\n\n\
---"
curl -i -X POST --data-urlencode "payload={\"text\":\"${MESSAGE}\"}" "$(MATTERMOST_URL)"
Invalid identifiers requested :: Cannot issue for "adfsfasdf.asdfasdf": Domain name does not end with a valid public suffix (TLD)
Ask for help or search for solutions at https://community.letsencrypt.org. See the logfile /var/log/letsencrypt/letsencrypt.log or re-run Certbot with -v for more details.
```
placeholder:Tell us what you see!
validations:
required:true
- type:textarea
id:expected
attributes:
label:Expected behavior
description:Certbot's behavior differed from what I expected because.
placeholder:"What was expected?"
validations:
required:true
- type:textarea
id:logs
attributes:
label:Relevant log output
description:Here is a Certbot log showing the issue (if available). Logs are stored in `/var/log/letsencrypt` by default. Feel free to redact domains, e-mail and IP addresses as you see fit.
about:If you're having trouble using Certbot and aren't sure you've found a bug or request for a new feature, please first try asking for help here. There is a much larger community there of people familiar with the project who will be able to more quickly answer your questions.
- [ ] The Certbot team has recently expressed interest in reviewing a PR for this. If not, this PR may be closed due our limited resources and need to prioritize how we spend them.
- [ ] If the change being made is to a [distributed component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout), add a description of your change to the `newsfragments` directory. This should be a file called `<title>.<type>`, where `<title>` is either a GitHub issue number or some other unique name starting with `+`, and `<type>` is either `changed`, `fixed`, or `added`.
* For example, if you fixed a bug for issue number 42, create a file called `42.fixed` and put a description of your change in that file.
- [ ] Add or update any documentation as needed to support the changes in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
zoracon: (This is an old comment below, not sure how accurate this is anymore.
Since Github seems to lean more towards Markdown these days, it's still probably accurate)
This file serves as an entry point for GitHub's Contributing
Guidelines [1] only.
@@ -19,19 +22,15 @@ to the Sphinx generated docs is provided below.
Hi! Welcome to the Certbot project. We look forward to collaborating with you.
If you're reporting a bug in Certbot, please make sure to include:
- The version of Certbot you're running.
- The operating system you're running it on.
- The commands you ran.
- What you expected to happen, and
- What actually happened.
If you're reporting a bug in Certbot. Please open an issue: https://github.com/certbot/certbot/issues/new/choose.
If you're having trouble using Certbot and aren't sure you've found a bug, please first try asking for help at https://community.letsencrypt.org/. There is a much larger community there of people familiar with the project who will be able to more quickly answer your questions.
If you're a developer, we have some helpful information in our
[Developer's Guide](https://certbot.eff.org/docs/contributing.html) to get you
started. In particular, we recommend you read these sections
started. In particular, we recommend you read these sections:
- [EFF's Public Projects Code of Conduct](https://www.eff.org/pages/eppcode)
- [Finding issues to work on](https://certbot.eff.org/docs/contributing.html#find-issues-to-work-on)
Explanation on supported versions [here](https://github.com/certbot/certbot/wiki/Architectural-Decision-Records#-update-to-certbots-version-policy-and-end-of-life-support-on-previous-major-versions)
| Major Version | Support Level |
| ------- | ------------------ |
| >= 4.0 | Full Support |
| 3.x | Discretionary Backports |
| <=2.x | None |
## Reporting a Vulnerability
Security vulnerabilities can be reported using GitHub's [private vulnerability reporting tool](https://github.com/certbot/certbot/security/advisories/new).
Some files were not shown because too many files have changed in this diff
Show More
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
