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

(RHEL-26644) resolved: limit the number of signature validations in a transaction #431

Conversation

jacekmigacz
Copy link
Member

@jacekmigacz jacekmigacz commented Feb 26, 2024

No description provided.

@github-actions github-actions bot added tracker/invalid-product pr/needs-ci Formerly needs-ci pr/needs-review Formerly needs-review labels Feb 26, 2024
Copy link

github-actions bot commented Feb 26, 2024

Commit validation

Tracker - RHEL-26644

The following commits meet all requirements

commit upstream
811c030 - resolved: limit the number of signature validations in a transaction systemd/systemd@67d0ce8
9597b18 - resolved: reduce the maximum nsec3 iterations to 100 systemd/systemd@eba2911

Tracker validation

Success

🟢 Tracker RHEL-26644 has set desired product: rhel-8.10.0
🟢 Tracker RHEL-26644 has set desired component: systemd
🟢 Tracker RHEL-26644 has been approved


Pull Request validation

Success

🟢 CI - All checks have passed
🟢 Review - Reviewed by a member
🟢 Approval - Changes were approved


Auto Merge

Success

🟢 Pull Request is not marked as draft and it's not blocked by dont-merge label
🟢 Pull Request meet requirements, title has correct form
🟢 Pull Request meet requirements, mergeable is true
🟢 Pull Request meet requirements, mergeable_state is clean
🟢 Pull Request has correct target branch main
🟢 Pull Request was merged

@jamacku jamacku added this to the RHEL-8.10 milestone Feb 26, 2024
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

@jacekmigacz We usually don't reference merge commits. Please reference the following upstream commits instead:

@github-actions github-actions bot added pr/changes-requested and removed pr/needs-review Formerly needs-review labels Feb 27, 2024
It has been demonstrated that tolerating an unbounded number of dnssec
signature validations is a bad idea. It is easy for a maliciously
crafted DNS reply to contain as many keytag collisions as desired,
causing us to iterate every dnskey and signature combination in vain.

The solution is to impose a maximum number of validations we will
tolerate. While collisions are not hard to craft, I still expect they
are unlikely in the wild so it should be safe to pick fairly small
values.

Here two limits are imposed: one on the maximum number of invalid
signatures encountered per rrset, and another on the total number of
validations performed per transaction.

(cherry picked from commit 67d0ce8843d612a2245d0966197d4f528b911b66)

Resolves: RHEL-26644
According to RFC9267, the 2500 value is not helpful, and in fact it can
be harmful to permit a large number of iterations. Combined with limits
on the number of signature validations, I expect this will mitigate the
impact of maliciously crafted domains designed to cause excessive
cryptographic work.

(cherry picked from commit eba291124bc11f03732d1fc468db3bfac069f9cb)

Related: RHEL-26644
@github-actions github-actions bot added the pr/needs-ci Formerly needs-ci label Feb 27, 2024
@github-actions github-actions bot added pr/needs-review Formerly needs-review and removed pr/changes-requested pr/needs-ci Formerly needs-ci labels Feb 27, 2024
Copy link
Member

@jamacku jamacku left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot removed the pr/needs-review Formerly needs-review label Mar 7, 2024
@github-actions github-actions bot merged commit 92124e8 into redhat-plumbers:main Mar 7, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants