Last Call Review of draft-ietf-httpbis-digest-headers-11
review-ietf-httpbis-digest-headers-11-secdir-lc-yee-2023-03-23-00

Reviewer: Peter Yee
Review result: Has Issues

I have reviewed this document as part of the security directorate's ongoing
effort to review all IETF documents being processed by the IESG.  These
comments were written primarily for the benefit of the security area directors.
Document editors and WG chairs should treat these comments just like any other
last call comments.

The document specifies an HTTP integrity mechanism that replaces a prior one
specified in RFC 3230. This is done with 4 new HTTP header fields and a new
digest algorithm registry to be used with those header fields.

The reason that the review result is "Has Issues" is that I disagree with the
algorithm statuses assigned in Table 2 that are used to populate the Hash
Algorithms for HTTP Digest Fields Registry. It's possible I'm way off base
here. In the face of a malicious actor who is able to change the header fields
as an HTTP message is transferred along the way to an endpoint, none of these
algorithms is strong. That's fine. As noted in sections 1.2 and section 6.1, a
secondary mechanism such as HTTP Message Signatures
(draft-ietf-httpbis-message-signatures) would be needed to protect the field
values in an end-to-end manner. If such a mechanism is employed, then the
strength of the hash algorithm used in this specification is largely
irrelevant, as the output of the hash algorithm here is not the direct input to
the digital signature algorithm. Perhaps I do not understand how
draft-ietf-httpbis-message-signatures is used to protect the header fields
specified in this document, but I have to think the field values generated
through this specification are merely data to be input to the combined digital
signature and hash algorithms specified in
draft-ietf-httpbis-message-signatures. To reiterate, I think the statuses
specified for the registry are not terribly useful: in the case of a malicious
actor, any of the hash values can be recalculated and substituted easily. In
the case of non-malicious value mismatches, any of the hash algorithms is
likely good enough, particularly if the underlying transport attempts to ensure
that it correctly delivers data.

I did not thoroughly read through the appendices nor attempt to validate the
examples they contain. I do appreciate the inclusion of Python code to make it
easier to calculate the hash values needed to validate the examples.

The rest of this review is minor comments and nits.

General: change all "use-cases" to "use cases" for consistent with other usage
in the document.

General: append a comma after all occurrences of "e.g.".

Page 4, 1st partial paragraph, 1st whole sentence: change the comma after
"connection" to a period. Capitalize the following "in".

Page 5, section 1.2, 1st paragraph, 2nd sentence: change the comma after
"message" to a period. Capitalize the following "the".

Page 5, section 1,.2, 1st paragraph, last sentence: append "registry" after the
quoted registry name. Also consider changing the pointer from section 5 to
section 7.2, where the registry really gets defined.

Page 5, section 1.2, 2nd paragraph, 1st sentence: insert "the" before "HTTP".

Page 6, 1st paragraph, 1st sentence: change "Authorization" to lowercase
"authorization" and append a comma after it.

Page 6, section 1.3, 1st paragraph, 2nd sentence: delete the comma after
"defined".

Page 6, last paragraph: append a comma after the quoted "user agent".

Page 12, 3rd to last paragraph, 3rd sentence: append a comma after "broken".
Yeah, I like Oxford/Harvard commas.

Page 13, section 6.2, 1st paragraph, 2nd sentence: insert "an" before "HTML".
Append a comma after "player".

Page 13, section 6.3, 2nd paragraph, 1st sentence: change "digitial" to
"digital".

Page 14, section 6.4, 3rd paragraph: change to the comma after "Section" to a
period. Make "Section" lowercase. Capitalize the following "some". Change
"pre-processing" to "preprocessing".

Page 14, section 6.5, 1st paragraph, 2nd sentence: change "mulitple" to
"multiple".

Page 14, section 6.5, 1st paragraph, 3rd sentence: change the comma after
"metadata" to a period. Capitalize the following "for". Append a comma after
"instance".

Page 21, Appendix A, 1st paragraph, 1st sentence: append a comma after
"Transformations". I'm not sure that word needs to be capitalized, but HTTP
terms is not an area with which I'm overly familiar.

Page 23, Appendix B, 2nd paragraph, 3rd sentence: change "spaced" to "spaces".

Page 36, Acknowledgements: append a comma after "Yaskin".