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

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.

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

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

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

### 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:"

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

### 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"

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

Received on Wednesday, 7 June 2023 23:47:40 UTC