Re: Secdir last call review of draft-ietf-httpbis-digest-headers-11

Hi Peter,

Thank you for the review. Responses in line

On Fri, Mar 24, 2023 at 12:06 AM Peter Yee via Datatracker <noreply@ietf.org>
wrote:

> Reviewer: Peter Yee
> Review result: Has Issues
>
> 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.
>

You're correct that digest on its own provides weak protection from
purposeful or malicious intent to manipulate it. You're also correct that
signatures can help protect against that and the digest fields are just
another piece of input to the signature (rather than having any special
role or distinct meaning).

I disagree that the algorithm status is not useful. Common HTTP deployment
models involve many interactions where there is no decent secondary
integration mechanism. For example, a cache that persists to disk, or "just
in time" processing at the edge. The choice of algorithm is dependent on
the needs of the application using HTTP, which includes the data in content
and the context of its use. Sometimes a weak algorithm might be sufficient
to cover those needs. Other times, the risk of something like a hash
collision could make an algorithm a severely bad choice. We can't hope to
explain all of these in the draft itself, so have chosen to highlight the
considerations and provide a clear status to implementations that need to
pick an algorithm for their needs. We also need to accept that most
implementers will lack the domain knowledge to make the correct selection.
It seems remiss to present a set of algorithms in a way that appears to
give them a level playing field, when the community does not think that. In
this light, hoping that a selection was "likely good enough" makes me
uncomfortable. Nudging people towards stronger algorithms seems to have
little downside.

To put things in a different context, some other integrity mechanisms used
regularly on the Internet, such as Subresource Integrity [1], provide
stronger normative language on the choice of algorithm. Given the legacy of
this draft, the WG consensus is that this document strikes the correct
balance between highlighting security considerations and allowing
applications to make their own decisions specific to their needs.



> 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.
>

Thanks!


>
> 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".
>
>
Thank you for these. I've applied almost all of them to the editor's copy
and they improve the readability of the document.

Cheers,
Lucas

[1] - https://www.w3.org/TR/SRI/#cryptographic-hash-functions