Re: John Scudder's No Objection on draft-ietf-httpbis-message-signatures-17: (with COMMENT)

Hi John, thanks for the review - answers inline below.

> On Jun 7, 2023, at 7:47 PM, John Scudder via Datatracker <noreply@ietf.org> wrote:
> 
> John Scudder has entered the following ballot position for
> draft-ietf-httpbis-message-signatures-17: No Objection
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
> for more information about how to handle DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-httpbis-message-signatures/

> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> # John Scudder, RTG AD, comments for draft-ietf-httpbis-message-signatures-17
> CC @jgscudder
> 
> Thanks for this thorough and well-written spec. Although I haven't done
> anything like a detailed line-by-line review of it, I noticed a few minor
> things to comment on (some are just proofreading), I hope some of these are
> helpful.
> 
> ## COMMENTS
> 
> ### General, keyword rendering
> 
> You have numerous keywords identified in the source by enclosing them in
> <tt></tt>. This works well (or well enough) to distinguish them for what they
> are in the HTML rendering, but unfortunately in the txt rendering, xml2rfc does
> exactly and only what RFC 7991 says: renders them in a constant-width font.
> Since in the txt rendering, everything else is in a constant-width font too (or
> to split hairs, really no font information is present) this means there's
> nothing at all to distinguish the keywords you've gone to the trouble of
> identifying in your source, in the txt rendering, <tt> is basically a no-op.
> 
> For the most part this is OK, context makes it clear enough, but I noticed one
> place where it creates ambiguity in the txt rendering. In Section 1.4, first
> bullet, you have
> 
>                                                For example, an
>      authorization protocol could mandate that the Authorization field
>      be covered to protect the authorization credentials and mandate
>      the signature parameters contain a created parameter,
> 
> In this case, there's nothing to cue a reader that "created" is the name of a
> particular parameter, and not an adjective being used to describe "parameter".
> 
> This isn't a huge problem even in this case, and I haven't identified any
> more-problematic instances (although I've made no effort to audit all 448 <tt>
> uses). Arguably this is something for the RSWG and not the authors at all, but
> I wanted to make sure you were aware of it. If you've only been reviewing the
> HTML rendering you may never have seen the issue.
> 

This is a good thing to point out to the tooling folks - I’ve seen this behavior with other drafts as well. For the authors’ part, we’re using Markdown to do the primary text edits, and using backticks to separate things like `created` from the surrounding text, since it’s a specific named parameter. I think previous versions of the tooling put quotes around backtick sequences in the text versions, but I’m honestly not sure. If there’s something we can do on our side to make this render better, I’m glad to.

> ### Section 1.1, quibble about "Unix time"
> 
>   The term "Unix time" is defined by [POSIX.1], Section 4.16
> 
> The string "Unix time" never appears in the reference. Though the text is
> correct as written, if closely parsed, it might be nice to write it a little
> more bluntly, as in
> 
>   The term "Unix time" refers to what [POSIX.1], Section 4.16 calls
>   "Seconds Since the Epoch."

This is a good point and we’ll fix that, along with fixing the reference as suggested by Roman’s review.

> 
> ### Section 2.5, imprecision in step (2)(1)
> 
>       1.  Check that the component identifier (including its
>           parameters) has not already been added to the signature base.
>           If this happens, produce an error.
> 
> I think a close and uncharitable reading of this step leaves "this" without a
> clear referent. It's clear enough from context what you mean, so I'm not really
> worried, but it's nice for algorithms to be as unambiguous as possible. Perhaps
> something like
> 
>       1.  If the component identifier (including its
>           parameters) has already been added to the signature base,
>           produce an error.
> 
> would do the job.
> 

Good catch, we’ll reword this to remove the ambiguity.

> ### Section 2.5, even nittier nit in (2)(5) bullet 2
> 
>           *  If the component identifier has several incompatible
>              parameters, such as bs and sf, produce an error.
> 
> Perhaps replace "several" with "two or more" or rewrite as "... has parameters
> that are mutually incompatible" or "... has parameters that are incompatible
> with one another".
> 

Thanks, I think that’s a reasonable change, we’ll incorporate it.

> ### Section 2.5, another nit
> 
>   If covered components reference a component identifier that cannot be
>   resolved to a component value in the message, the implementation MUST
>   produce an error and not create a signature base.  Such situations
>   are included but not limited to:
> 
> Last line should be something like "include, but are not limited to:"

Seems fine to me, we can add that.

> 
> ### Section 6.2, spurious "are"
> 
>   The Designated Expert (DE) is expected to ensure that the algorithms
>   referenced by a registered algorithm identifier are fully defined
>   with all parameters (such as salt, hash, required key length, etc)
>   are fixed by the defining text.  The DE is expected to ensure that
> 
> I think the "are" in "are fixed by the defining text" should be deleted.
> 

Good catch, thanks.

> ### Section 8.4
> 
>   A possible mitigation for this specific situation would be for the
>   intermediary to verify the signature itself, then modifying the
>   message to remove the privacy-sensitive information.
> 
> "then modifying" -> "and then modify"
> 

Good catch, thanks.


 — Justin

Received on Friday, 9 June 2023 18:29:03 UTC