A quick note: Smallstep's implementation of ACME client and server already supports the new extension.

Smallstep client code that supports both the default and non-default payloads: https://github.com/smallstep/certificates/blob/1b2d999e4607bbe4796dce2a0f0f3c7a29cec463/ca/acmeClient.go#L243-L267

CC @golang/security

Change https://go.dev/cl/608975 mentions this issue: acme: support challenges that require the ACME client to send a non-empty JSON body in a response to the challenge.

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

Would it make sense to add a field to the Challenge type instead of having a whole new method?

We could also avoid adding a field entirely, and add an internal check for Challenge.Type == "hardware-module". This would be somewhat opaque, but doesn't require an API change.

Would it make sense to add a field to the Challenge type instead of having a whole new method?

This works too.

If I understand correctly, the change will look like the below. Does it make sense?

1. Add a field to Challenge.

type Challenge struct {
  ...
  // If a challenge requires a non-empty response (e.g. "device-attest-01"),
  // the client must populate Response
  Response sometype
}

2. Code that uses the new challenge type will look like this:

  // retrieve a challenge from the server
  var challenge acme.Challenge
  challenge = ...

  // construct the response from challenge.Token
  challenge.Response = ...

  // inform the server that the challenge is ready
  acmeClient.Accept(ctx, challenge)

3. Client.Accept() is modified to handle non-empty response:

  if chal.Response is not initialized {
    res, err := c.post(ctx, nil, chal.URI, json.RawMessage("{}"), ...)
  } else {
    res, err := c.post(ctx, nil, chal.URI, chal.Response, ...)
  }

We could also avoid adding a field entirely, and add an internal check for Challenge.Type == "hardware-module". This would be somewhat opaque, but doesn't require an API change.

I may be missing something here... How does the client pass the response to the server in this case?

Thank you for the review and suggestions!

BTW this ACME extension as specified in the draft is already widely implemented and deployed in iOS and MacOS, so even if the draft evolves further (which is unlikely; it's AFAIK at the IETF last-call stage), there will still be a desire to support it for this large client base.

I may be missing something here... How does the client pass the response to the server in this case?

Whoops, my bad I was thinking of the wrong extension. Adding a field to Challenge makes the most sense. The description in #68674 (comment) sounds accurate to me.

OK, so it sounds like maybe adding

// Payload is the JSON-formatted payload that the client sends
// to the server to indicate it is ready to respond to the challenge.
// When unset, it defaults to an empty JSON object: {}.
Payload json.Message

to Challenge is all we need?

OK, so it sounds like maybe adding

Payload json.Message

to Challenge is all we need?

This looks like the best path forward.
Just a minor clarification: should the type be json.RawMessage?

The current proposal is to add the following field to acme.Challenge:

// Payload is the JSON-formatted payload that the client sends
// to the server to indicate it is ready to respond to the challenge.
// When unset, it defaults to an empty JSON object: {}.
Payload json.RawMessage

Is that right?

I would like for the doc comment to be a little more prescriptive. Perhaps point at the extension spec and/or suggest why one would want to set this field. Could someone more familiar with the uses of this write that text?

Also, I see the extension proposal hasn't made any visible progress since version 1 was uploaded on 2023-07-24. Do we expect this to actually get ratified? Alternatively, is it in widespread enough use that it's de facto standard?

I would like for the doc comment to be a little more prescriptive. Perhaps point at the extension spec and/or suggest why one would want to set this field. Could someone more familiar with the uses of this write that text?

Yup I can add some additional text.

Also, I see the extension proposal hasn't made any visible progress since version 1 was uploaded on 2023-07-24. Do we expect this to actually get ratified? Alternatively, is it in widespread enough use that it's de facto standard?

It looks like the most recent revision was sent last month (draft-acme-device-attest-03), given there has been minimal feedback, it's likely to get moved to last-call at the next IETF meeting in November. The acme WG has taken a somewhat more reasonable approach to specification standardization, usually waiting for at least a couple working implementations before fully ossifying things.

Have all remaining concerns about this proposal been addressed?

Add the following field to acme.Challenge:

// Payload is the JSON-formatted payload that the client sends
// to the server to indicate it is ready to respond to the challenge.
// When unset, it defaults to an empty JSON object: {}.
Payload json.RawMessage

Alternatively, is it in widespread enough use that it's de facto standard?

Yes, the current draft is already implemented by IOS and MacOS.

Based on the discussion above, this proposal seems like a likely accept.

Add the following field to acme.Challenge:

// Payload is the JSON-formatted payload that the client sends
// to the server to indicate it is ready to respond to the challenge.
// When unset, it defaults to an empty JSON object: {}.
Payload json.RawMessage

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.

Add the following field to acme.Challenge:

// Payload is the JSON-formatted payload that the client sends
// to the server to indicate it is ready to respond to the challenge.
// When unset, it defaults to an empty JSON object: {}.
Payload json.RawMessage