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
On Fri, Feb 24, 2023 at 3:16 PM Brian Campbell <bcampbell@xxxxxxxxxxxxxxxx> 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@xxxxxxxx> 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.
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
