Re: Genart last call review of draft-ietf-httpbis-client-cert-field-04

FWIW, I've prepared this PR with those changes
https://github.com/httpwg/http-extensions/pull/2425

A preview can be seen here
https://httpwg.org/http-extensions/cert/genart-review/draft-ietf-httpbis-client-cert-field.html

As well as a diff
https://author-tools.ietf.org/iddiff?url1=https://httpwg.github.io/http-extensions/draft-ietf-httpbis-client-cert-field.txt&url2=https://httpwg.github.io/http-extensions/cert/genart-review/draft-ietf-httpbis-client-cert-field.txt



On Fri, Feb 24, 2023 at 3:16 PM Brian Campbell <bcampbell@pingidentity.com>
wrote:

> Thank you Peter for the thorough review!
>
> I'll make those clarifications re: Client-Cert and Client-Cert-Chain in
> sec 2 and the appx. Although I think adding the root certificate to
> Client-Cert-Chain is more of a 'can' than a 'should' depending on
> deployment needs. Regardless, I'll add some text to clarify. And will, of
> course, fix all the nits/editorial comments too.
>
> You are absolutely right that there's potentially a lot of bytes on the
> wire overhead in adding the client cert and possibly the certificate chain
> to every request. However, approaches like substituting something smaller
> on subsequent requests would entail different challenges and overhead -
> particularly with respect to maintaining and keeping state. Also, the aim
> of this draft is to document existing practice while codifying specific
> details, like header names and structure, so as to hopefully facilitate
> improved and lower-touch interoperability going forward (that goal/scope
> could be more explicitly stated in the intro and I'll look to add something
> as such). And existing practice is (largely) to stick the certs in headers
> on each request.
>
>
>
> On Fri, Feb 24, 2023 at 6:40 AM Peter Yee via Datatracker <
> noreply@ietf.org> wrote:
>
>> Reviewer: Peter Yee
>> Review result: Ready with Issues
>>
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>>
>> For more information, please see the FAQ at
>>
>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>>
>> Document: draft-ietf-httpbis-client-cert-field-04
>> Reviewer: Peter Yee
>> Review Date: 2023-02-24
>> IETF LC End Date: 2023-02-23
>> IESG Telechat date: Not scheduled for a telechat
>>
>> Summary: This draft documents a standardized means of conveying a client
>> certificate and chain between a TLS-terminating reverse proxy (TTRP) and
>> an
>> origin server so as to allow the origin server to make decisions based on
>> the
>> identity of the client. This draft is fairly straightforward and not
>> difficult
>> to follow, but there are a couple of issues that could use clarification.
>> [Ready with issues.]
>>
>> Major issues: None
>>
>> Minor issues:
>>
>> The text defining the header fields for Client-Cert and Client-Cert-Chain
>> in
>> section 2 along with the example in Appendix A should make clear:
>>
>> 1) The client certificate placed in Client-Cert is not included in the
>> optional
>> Client-Cert-Chain. 2) Client-Cert-Chain should only appear when
>> Client-Cert is
>> also present. 3) That the root certificate of the chain should be added
>> to the
>> Client-Cert-Chain by the TTRP.
>>
>> Confusion might arise because the header definitions make no mention of
>> this,
>> but the example in Appendix A shows a certificate chain (Figure 1) that is
>> inclusive of a client certificate, but Figure 3 shows that the client
>> certificate is not included in the chain. The text in Appendix A says the
>> client only presents the client and intermediate certificate, so point #3
>> above
>> is needed.
>>
>> In section 2.4, second paragraph, second sentence: this seems really
>> expensive
>> to add the client cert and possibly the certificate chain to every
>> request the
>> client sends to the TTRP. The sending of those fields to the could
>> outweigh the
>> size of the request by quite a bit. Would it be possible to substitute
>> something smaller on subsequent requests between the TTRP and the origin
>> server
>> since they already require a trusted connection between them? A hash of
>> the
>> field(s) value(s)? Something else that helps combat the potential
>> overhead? I
>> realize that I'm asking about a non-trivial change to the draft, which is
>> easy
>> for me to do and difficult for the authors to implement. ;-)
>>
>> Nits/editorial comments:
>>
>> General:
>>
>> Change all occurrences of “mutually-authenticated” to “mutually
>> authenticated”.
>>
>> Specific:
>>
>> Page 6, section 2.4, 2nd paragraph, 2nd sentence: change the “are” to
>> “is”.
>>
>> Page 8, section 3.2, 3rd sentence: append a comma after “e.g.”.
>>
>> Page 8, section 4, 1st paragraph, 1st sentence: change “server side” to
>> “server-side”.
>>
>> Page 8, section 4, 2nd paragraph, 2nd sentence: append a comma after
>> “fields”.
>>
>> Page 9, 3rd paragraph, 5th sentence: append a comma after “Alternatively”.
>>
>> Page 15, section B.1, 3rd sentence: change the semicolon after “possible”
>> to a
>> comma.
>>
>> Page 15, section B.3, 1st sentence: delete one period after “etc”.
>>
>> Page 16, 1st partial paragraph: append a comma after “full”.
>>
>>
>>
>>

-- 
_CONFIDENTIALITY NOTICE: This email may contain confidential and privileged 
material for the sole use of the intended recipient(s). Any review, use, 
distribution or disclosure by others is strictly prohibited.  If you have 
received this communication in error, please notify the sender immediately 
by e-mail and delete the message and any file attachments from your 
computer. Thank you._