Repository navigation
ra/sa: render the attempted challenge as "processing" - #9096
Open
bhaskargurram-ai wants to merge 2 commits into
Open
bhaskargurram-ai wants to merge 2 commits into
bhaskargurram-ai wants to merge 2 commits into
Conversation
RFC 8555 Section 7.1.6 says a challenge transitions from "pending" to "processing" once the client responds to it. Boulder never rendered that state: because challenges are stored as part of the authorization rather than as separate objects, a challenge stayed "pending" until the authorization was finalized as "valid" or "invalid". SetAuthzProcessing (added in letsencrypt#8838) already marks the authz's beganProcessing column before dispatching validation, but that column has no effect on the rendered objects. Extend it so that the RA also tells the SA which challenge is being validated: - Replace the SetAuthzProcessing request type with a dedicated SetAuthzProcessingRequest carrying the authz ID and the attempted challenge type. Field 1 is unchanged, so the RPC remains wire-compatible with the old AuthorizationID2 request in both directions. - Have the SA write the attempted challenge type into the authz2 "attempted" column in the same UPDATE that sets beganProcessing. The field is optional, so an RA which predates it still works. - When modelToAuthzPB sees an attempted challenge on an authz which is still pending, render that challenge with status "processing" (and no validation results) while leaving the authz status "pending", since RFC 8555 has no "processing" status for authorizations. - Have the RA reject a second attempt at a challenge which is already processing with the same "conflict" error the SA returns, rather than letting CheckPending turn it into a "malformed" error. - Forward SetAuthzProcessing in the in-memory SA used by the RA tests, which previously did not implement it. All of the new behavior remains gated behind the existing SetAuthzProcessing feature flag, so nothing changes until the flag is enabled in the RA. Fixes letsencrypt#9032
aarongable
reviewed
Oct 19, 2026
Address review feedback: when the authz has an attempted challenge, populate that single challenge directly instead of checking every challenge type in the bitmap loop. Move the "processing" check and the validated time into populateAttemptedFields, and add a TODO to require the attempted challenge type once the SetAuthzProcessing flag is removed.
Author
|
@aarongable Thanks, done in d96e0d4. The attempted challenge is now built directly outside the loop, the processing check and the validated time moved into populateAttemptedFields, and I added the TODO next to the optional check in SetAuthzProcessing. I also set a finalized status on the two models in TestPopulateAttemptedFieldsBadJSON, since a zero-value model is now treated as pending and returns early. |
aarongable
reviewed
Oct 20, 2026
Comment on lines
+839
to
+840
| if am.Attempted != nil { | ||
| if (am.Challenges>>*am.Attempted)&1 == 1 { |
Contributor
There was a problem hiding this comment.
Either combine these into a single conditional, or remove the second conditional (it should be impossible for am.Attempted to indicate a value that isn't set in the am.Challenges bitmask, but I haven't looked closely to check how firmly we hold that invariant).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RFC 8555 §7.1.6 says a challenge moves from "pending" to "processing" once the
client responds to it. Boulder never rendered that state: challenges are stored
as part of the authorization rather than as separate objects, so a challenge
stayed "pending" until the authz was finalized as "valid" or "invalid".
#8838 added
SetAuthzProcessing, which setsauthz2.beganProcessingbeforevalidation is dispatched, but that column has no effect on rendered objects.
Following @aarongable's suggestion in #9032, this extends it so the RA also
tells the SA which challenge is being validated, and the SA renders that
challenge as "processing" while the authz stays "pending":
SetAuthzProcessingnow takes a dedicatedSetAuthzProcessingRequestcarrying the authz ID and the attempted challenge type. Field 1 is unchanged
from
AuthorizationID2, so the RPC stays wire-compatible in both directionsduring a deploy; the new field is optional and an RA that doesn't send it
gets the old behaviour.
attemptedcolumn in the sameUPDATE that sets
beganProcessing.modelToAuthzPBrenders an attempted challenge on a still-pending authz as"processing" (with no validation results). The authz status stays "pending",
since RFC 8555 has no "processing" status for authorizations. The
combination is done in the SA rather than the WFE because
beganProcessingisn't part of the authz model/proto, and
attempted != NULLon a pendingauthz is only ever produced by
SetAuthzProcessing.with the same "conflict" error the SA returns, so clients that re-POST keep
getting the 409 introduced by ra/sa: Prevent parallel validation attempts #8838 (and
TestParallelValidationConflictkeeps passing) instead of a "malformed" 400 from
CheckPending.test/inmem/sanow forwardsSetAuthzProcessing; it didn't before, so RAtests couldn't exercise the flag.
Everything is still gated behind the existing
SetAuthzProcessingfeatureflag. No config, schema, or flag changes.
Open questions for reviewers:
behaviour). While processing they're shown the same way; happy to render
them as "pending" instead if you'd rather follow §7.1.4 literally.
shared constant if preferred.
Tests: new
TestAuthzModelProcessing, extendedTestSetAuthzProcessing,TestPerformValidationSetsChallengeProcessing,TestPerformValidationChallengeProcessing, and WFE challenge/authz handlercases asserting the rendered "processing" status. Protos regenerated with the
pinned toolchain (protoc 3.20.1, protoc-gen-go v1.36.5, protoc-gen-go-grpc
v1.5.1).
Fixes #9032