Skip to content

Conversation

@alexzorin
Copy link
Contributor

For #380 (comment).

Keeping track of when a challenge is in the processing state means we will be able to then include a Retry-After response header for authorizations, as described in:

To check on the status of an authorization, the client sends a POST-
as-GET request to the authorization URL, and the server responds with
the current authorization object. In responding to poll requests
while the validation is still in progress, the server MUST return a
200 (OK) response and MAY include a Retry-After header field to
suggest a polling interval to the client.

One extra side effect here is that clients will no longer be able to cause Pebble to queue up a pending challenge multiple times for processing by a VA. This seems like a good thing anyway.

@alexzorin
Copy link
Contributor Author

alexzorin commented May 8, 2022

On second thought, this is still racey. I guess we have to set the status earlier, when we hold the lock to check that the current status is pending.

I tried a different approach, by taking a short lock to check the status again, right before we send the challenge to the VA. It's a bit duplicative but I can't think of anything better 🤷 .

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.

LGTM, but I'd like a second set of eyes on this, since I'm actually not very familiar with Pebble's in-memory datastore and the locking necessary to work with it safely. @jsha?

@moratori
Copy link
Contributor

moratori commented Jun 9, 2022

@jsha I wonder if you could review this PR.

Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Thanks very much!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants