This field is optional to maintain backwards compatibility. Note that
`AnnotatedChallenge` inherits from `jose.ImmutableMap`, which has a
[check in
__init__](4b74747670/src/josepy/util.py (L125-L131))
that all slots are provided. That check would not allow us to do a
backwards-compatible addition, so I implemented an `__init__` for each
of these subclasses that fills the fields without calling the parent
`__init__`, and so doesn't hit an error when `identifier` is absent.
I chose to use `acme.messages.Identifier` rather than
`certbot._internal.san.SAN` here because these are wrapped ACME types,
so they should use the ACME representation. Also, `AnnotatedChallenge`
is passed to plugins, so we need to pass a type that the plugins can
understand.
Additionally, `domain` is marked as deprecated.
Part of #10346
/cc @bmw, who noticed the issue with `AnnotatedChallenge`
[here](https://github.com/certbot/certbot/pull/10468#issuecomment-3403294394)
and provided additional feedback
[here](https://github.com/jsha/certbot/pull/2#issuecomment-3534895793).
Note that there's still some work to do to finish excising `domain`
assumptions from this portion of the code.
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
in https://github.com/canonical/snapcraft/pull/5720, snapcraft made a
change. `snapcraft status certbot` output changed from something like
this:
```
Track Arch Channel Version Revision Progress
latest amd64 stable 5.1.0 5057 -
candidate ↑ ↑ -
beta 5.2.1 5214 -
edge 5.2.0.dev0 5210 -
arm64 stable 5.1.0 5058 -
candidate ↑ ↑ -
beta 5.2.1 5215 -
edge 5.2.0.dev0 5211 -
armhf stable 5.1.0 5056 -
candidate ↑ ↑ -
beta 5.2.1 5213 -
edge 5.2.0.dev0 5212 -
```
to this:
```
Track Arch Channel Version Revision Progress
latest amd64 stable 5.1.0 5057 -
latest amd64 candidate ↑ ↑ -
latest amd64 beta 5.2.1 5214 -
latest amd64 edge 5.2.0.dev0 5210 -
latest arm64 stable 5.1.0 5058 -
latest arm64 candidate ↑ ↑ -
latest arm64 beta 5.2.1 5215 -
latest arm64 edge 5.2.0.dev0 5211 -
latest armhf stable 5.1.0 5056 -
latest armhf candidate ↑ ↑ -
latest armhf beta 5.2.1 5213 -
latest armhf edge 5.2.0.dev0 5212 -
```
when its output is captured like it is in finish_release.py in the lines
above the code i'm modifying here
not matching on the beginning of lines makes this pattern a little less
strict, but based on the rest of the pattern and the output here, i
personally think this is fine
after carefully verifying this works with the current state of things, i
went ahead and finished the release with this change and it worked just
fine. instead, this PR proposes a way to fix things going forward
if you dislike the general idea of this PR, feel free to just close it,
but i'm scheduled to release the next version of certbot a week from
today and i personally didn't like how
[newsfragments](https://github.com/certbot/certbot/tree/main/newsfragments)
is so empty despite us having done a lot of work on certbot lately
this PR just adds a simple newsfragment highlighting/teasing the work
jsha has been leading on support for IP address certificates which i
imagine would be of interest to some people in the community
```
$ towncrier build --draft --version 5.2.0
Loading template...
Finding news fragments...
Rendering news fragments...
Draft only -- nothing has been written.
What is seen below is what would be written.
## 5.2.0 - 2025-11-25
### Added
- Support for Python 3.14 was added.
([#10477](https://github.com/certbot/certbot/issues/10477))
### Changed
- While nothing significant should have changed from the user's perspective,
we've been doing a lot of internal refactoring in preparation for soon adding
support for IP address certificates to Certbot.
([#10468](https://github.com/certbot/certbot/issues/10468),
[#10478](https://github.com/certbot/certbot/issues/10478))
```
The Apache configuration `Include`d in automatically created
`[sitename]-le-ssl.conf` files was redefining the `vhost_combined`
`LogFormat`, but contrary to the comment before the redefinition, did
not include the virtual host server name in the log format. This is
particularly confusing because this redefinition is hard to find when
debugging logging issues, as log formats are not related to SSL/TLS
configuration, and the included configuration file is outside of
`/etc/apache2`.
Additionally, a `vhost_common` `LogFormat` was defined, but not used
anywhere.
The `LogFormat` directives were introduced in commit
68f85d9f1a. Several other directives that
do not directly pertain to configuring SSL/TLS were added in that
commit, and have gradually been removed over the years. This should be
the last such removal.
Delete the `LogFormat` directives from the Apache configuration files
(both old and current), and update the `ALL_SSL_OPTIONS_HASHES`.
Fixes#9769 File 'options-ssl-apache.conf' included in autocreated
'[sitename]-le-ssl.conf' has potentially problematic vhost_combined
LogFormat
Closes#10348.
## 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),
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.
this pr is in response to https://words.filippo.io/compromise-survey/.
ohemorange and i read this late on a friday to (speaking for myself at
least) much panic as it has some very strong words to say about the
github actions trigger pull_request_target which we use. looking into
the issue more, i also found that the popular static analysis tool
[zizmor](https://github.com/zizmorcore/zizmor) flags any github actions
workflow that uses the pull_request_target trigger with the message:
```
error[dangerous-triggers]: use of fundamentally insecure workflow trigger
pull_request_target is almost always used insecurely
```
this only added to my concern
the general problem with pull_request_target is that it runs with
additional privileges (e.g. potential write access, access to secrets)
in an environment containing values that can be set by an attacker.
these values include things such as references to the arbitrary code
contained in the triggering pr and pr titles which have been used to
perform shell injection attacks. not carefully treating these values
like the untrusted data it is while executing code in the privileged
environment given to pull_request_target has resulted in many supply
chain attacks
that's not to say that pull_request_target CAN'T be used securely.
zizmor even has [an
issue](https://github.com/zizmorcore/zizmor/issues/1168) brainstorming
how to not warn about all uses of the trigger as some are clearly fine
and the only way to accomplish what the user wants. i'm going to argue
that our uses of the trigger are ok
looking through the links provided by filippo's blog and [zizmor's
docs](https://docs.zizmor.sh/audits/#dangerous-triggers), i think we can
break down attacks used against pull_request_target into roughly 2
categories:
1. shell injection: "Nx S1ingularity" and "Ultralytics" from filippo's
blog
2. checking out and running a PR's code: "Kong Ingress Controller" and
"Rspack" from filippo's blog and https://ptrpa.ws/nixpkgs-actions-abuse
from zizmor docs
i think none of our pull_request_target workflows have these problems.
none of them use a shell (the [zizmor
issue](https://github.com/zizmorcore/zizmor/issues/1168) i linked
earlier suggests that any pull_request_target workflow that uses a run
block should always be flagged as insecure). instead, our workflows just
call action-mattermost-notify which can be [pretty easily
audited](https://github.com/mattermost/action-mattermost-notify/blob/2.0.0/src/main.js)
(as all the other files in the repo are boilerplate). passing possible
attacker controlled values directly to an action written in another
language is one of the approaches for mitigating script injection
[recommended by
github](https://docs.github.com/en/actions/reference/security/secure-use#use-an-action-instead-of-an-inline-script).
our workflows also do not check out the triggering pr's code
despite all that, i took this opportunity to cleanup and harden things a
bit. i reduced the permissions for each workflow and confirmed they each
still work on my fork. i also pinned the mattermost action to an exact
version and added some inline documentation
with these changes, our github workflows trigger few to no
warnings/errors when checked with zizmor,
[octoscan](https://github.com/synacktiv/octoscan), and [openssf
scorecard](https://github.com/ossf/scorecard)
if this pr is approved, i'll make similar changes to our josepy repo
In #10478 we added a `san.SAN` class, with two subclasses `san.DNSName`
and `san.IPAddress`, so we can carry type information about identifiers
through the Certbot code. This PR plumbs through those types in most
Certbot-internal code. Note that this does not change the `acme` module,
which uses `messages.Identifier`. It also tries to leave alone the code
paths into plugins.
This does not add a CLI flag to request an IP address certificate. That
will be in a followup PR.
Part of #10346
Contains san.DNSName, san.IPAddress, and a parent class san.SAN.
Split out from #10468 as a standalone PR. To see examples of how it's
intended to be used, please see that PR.
The constructor for DNSName incorporates the same validation done in
`enforce_domain_sanity`, and the tests from `enforce_domain_sanity` are
copied here as well. The goal is to delete `enforce_domain_sanity`
entirely as part of #10468.
In support of #10346.
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.
2024-09-26 14:48:10 -07:00
855 changed files with 8559 additions and 7941 deletions
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), edit the `master` section of `certbot/CHANGELOG.md` to include a description of the change being made.
- [ ] 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.
- Certbot team members SHOULD look at: [link](${{ env.MERGED_URL }})
- Certbot team members MAY also want to look at: [link](${{ env.UPDATED_URL }})
- Want to Discuss something today? Place it [here](https://docs.google.com/document/d/17YMUbtC1yg6MfiTMwT8zVm9LmO-cuGVBom0qFn8XJBM/edit?usp=sharing) and we can meet today on Zoom.
- The key words SHOULD and MAY in this message are to be interpreted as described in [RFC 8147](https://www.rfc-editor.org/rfc/rfc8174).
## Updates In the Past Week
- Most commented in the last week: [link](${{ env.UPDATED_URL }})
- Updated (assigned) PRs in the last week: [link](${{ env.ASSIGNED_PRS }})
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.
