Skip to content

wfe, csr: Add IP address identifier support & integration test #8187

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
May 27, 2025
Merged

wfe, csr: Add IP address identifier support & integration test #8187

merged 15 commits into from
May 27, 2025

Conversation

jprenken
Copy link
Contributor

@jprenken jprenken commented May 14, 2025 •
edited
Loading

Permit all valid identifier types in wfe.NewOrder and csr.VerifyCSR.

Permit certs with just IP address identifiers to skip sa.addIssuedNames.

Check that URI SANs are empty in csr.VerifyCSR, which was previously missed.

Use a real (Let's Encrypt) IP address range in integration testing, to let challtestsrv satisfy IP address challenges.

Fixes #8192
Depends on #8154

@jprenken jprenken marked this pull request as ready for review May 16, 2025 02:28
@jprenken jprenken requested a review from a team as a code owner May 16, 2025 02:28
@jprenken jprenken requested a review from beautifulentropy May 16, 2025 02:28
@jprenken jprenken marked this pull request as draft May 16, 2025 18:27
@jprenken jprenken changed the title wfe: Add IP address identifier support wfe, csr: Add IP address identifier support & integration tests May 16, 2025
@jprenken jprenken changed the title wfe, csr: Add IP address identifier support & integration tests wfe, csr: Add IP address identifier support & integration test May 16, 2025
aarongable
aarongable reviewed May 20, 2025
View reviewed changes
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Since this is in draft mode and I know you have more changes coming, just some comments on the new integration test itself.

@aarongable aarongable mentioned this pull request May 20, 2025
Merged
@jprenken jprenken marked this pull request as ready for review May 20, 2025 22:57
@jprenken jprenken requested a review from aarongable May 20, 2025 22:58
jprenken added a commit that referenced this pull request May 20, 2025
@jprenken
bdns, va: Remove DNSAllowLoopbackAddresses
09ba3d1 
We no longer need a code path to resolve reserved IP addresses during integration tests, since we started using a public IP for tests in #8187.

Depends on #8187
@jprenken jprenken mentioned this pull request May 20, 2025
Merged
aarongable
aarongable reviewed May 21, 2025
View reviewed changes
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

I think your "depends on" link in the PR description should be updated; that points to a bug not another PR.

@jprenken jprenken requested a review from aarongable May 22, 2025 06:12
aarongable
aarongable previously approved these changes May 22, 2025
View reviewed changes
@jprenken jprenken requested a review from aarongable May 22, 2025 18:08
@jprenken jprenken merged commit 103ffb0 into main May 27, 2025
12 checks passed
@jprenken jprenken deleted the wfe-ips branch May 27, 2025 20:17
jprenken added a commit that referenced this pull request May 28, 2025
@jprenken
bdns, va: Remove DNSAllowLoopbackAddresses (#8203)
dea81c7 
We no longer need a code path to resolve reserved IP addresses during
integration tests.

Move to a public IP for the remaining tests, after #8187 did so for many
of them.

Depends on #8187
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aarongable aarongable aarongable approved these changes

@beautifulentropy beautifulentropy beautifulentropy approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Teach the WFE to plumb ipAddress identifiers to the RA
3 participants
@jprenken @aarongable @beautifulentropy