Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolved: probe for dnssec support in allow-downgrade mode #32598

Merged
merged 1 commit into from May 1, 2024

Conversation

rpigott
Copy link
Contributor

@rpigott rpigott commented May 1, 2024

Previously, sd-resolved unnecessarily requested SOA records for each dns label in the query, even though they are not needed for the chain of trust. Since 4769063, only the necessary records are queried when validating.

This is actually a problem in allow-downgrade mode, since we will no longer attempt a query for a record that we know is signed a priori, and will therefore never update our belief about the state of dnssec support in the recursive resolver.

Rectify this by reintroducing a query for the the root zone SOA in the allow-downgrade case, specifically to test that the resolver attaches the RRSIGs which we know must exist.

Fixes: 4769063 ("resolved: don't request the SOA for every dns label")


Fixes: #32570

@github-actions github-actions bot added resolve please-review PR is ready for (re-)review by a maintainer labels May 1, 2024
Copy link

github-actions bot commented May 1, 2024

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

Copy link
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Several superficial comments.

src/resolve/resolved-dns-transaction.c Outdated Show resolved Hide resolved
src/resolve/resolved-dns-transaction.c Outdated Show resolved Hide resolved
src/resolve/resolved-dns-transaction.c Outdated Show resolved Hide resolved
src/resolve/resolved-dns-transaction.c Outdated Show resolved Hide resolved
@yuwata yuwata added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels May 1, 2024
Previously, sd-resolved unnecessarily requested SOA records for each dns
label in the query, even though they are not needed for the chain of
trust. Since 4769063, only the necessary records are queried when
validating.

This is actually a problem in allow-downgrade mode, since we will no
longer attempt a query for a record that we know is signed a priori, and
will therefore never update our belief about the state of dnssec support
in the recursive resolver.

Rectify this by reintroducing a query for the root zone SOA in the
allow-downgrade case, specifically to test that the resolver attaches
the RRSIGs which we know must exist.

Fixes: 4769063 ("resolved: don't request the SOA for every dns label")
@github-actions github-actions bot added please-review PR is ready for (re-)review by a maintainer and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels May 1, 2024
Copy link
Member

@yuwata yuwata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superficially LGTM. But I am not familiar with the change.

@yuwata yuwata requested a review from poettering May 1, 2024 06:54
@bluca bluca added this to the v256 milestone May 1, 2024
@bluca bluca merged commit 5237ffd into systemd:main May 1, 2024
43 of 49 checks passed
@github-actions github-actions bot removed the please-review PR is ready for (re-)review by a maintainer label May 1, 2024
@rpigott rpigott deleted the resolved-downgrade-soa branch May 1, 2024 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

systemd-resolved: "DNSSEC=allow-downgrade" still fails
3 participants