Re: [Gen-art] Gen-ART Last Call review of draft-baeuerle-netnews-cancel-lock-05

[Michael Bäuerle <[e-mail]>]

Paul Kyzivat wrote:
> On 7/7/17 11:47 AM, Michael Bäuerle wrote:
> > Paul Kyzivat wrote:  
> > >
> > > [...]
> > > General Comments:
> > >
> > > I have not attempted to validate the security properties of this
> > > document - leaving that to a security review.
> > >
> > > I have also not attempted to verify the operational suitability of this
> > > mechanism because I don't have the experience needed to do so.
> > >
> > > Issues:
> > >
> > > Major: 1
> > > Minor: 3
> > > Nits:  0
> > >
> > > (1) MAJOR:
> > >
> > > [Problem with ABNF syntax of header fields]  
> > 
> > Hi Paul,
> > 
> > Julien Élie has proposed a new syntax definition.  
> 
> I was copied on his mail proposing a change to RFC5536, and that is 
> tentative. *This* document would of course require a similar change. Or 
> else it could extend that change, via something like
> 
> 	news-fields =/ cancel-lock / cancel-key
> 
> But that can only be done if there is actually a draft that revises 5536 
> that can then be referenced. Bottom line is that it seems there needs to 
> be some coordination of that fix with this draft.

It's clear that the Cancel-Lock draft needs to be changed. My comment
was about the potential erratum for RFC 5536 and the question:
Is it (in general) possible to reference a modified ABNF definition in
an erratum?

If this is possible, then the current:

   fields =/ *( cancel-lock / cancel-key )

can be changed to:

   news-fields =/ cancel-lock / cancel-key

and should then reference the new ABNF from the erratum.
If not, what about a less formal definition like this:

   The Cancel-Lock and Cancel-Key header fields MUST be treated as
   though it were news header fields as defined in Section 3 of
   [RFC5536].

Rewriting RFC 5322 and RFC 5536 should be avoided if possible.

> > > (2) Minor:
> > >
> > > In Section 3.5, step 1 says to hash the key using the algorithm from its
> > > scheme. But IIUC the scheme describes the algorithm that has already
> > > been used when constructing the Cancel-Key header.  
> > 
> > No. There are two different operations that use a hash function.
> > 
> > The recommended algorithm described in Section 4 calculates:
> > |
> > | K = HMAC(uid+mid, sec)
> > 
> > K is then used for <c-key-string> in the Cancel-Key header field only
> > with Base64 as transfer encoding (no further hash operation is used):
> > 
> >     c-key-string = Base64(K)
> > 
> > HMAC can be based on a different hash function than the one in <scheme>
> > (because a serving agent doesn't need to know how K was calculated for
> > the check algorithm defined in Section 3.5).
> > 
> > A value of e.g. "sha256" for <scheme> in a Cancel-Key header field
> > doesn't mean that SHA256 was used to generate this field, but that the
> > corresponding Cancel-Lock header field (in the original article) contains
> > the value of <c-key-string> hashed with SHA256 (and finally Base64
> > encoded).
> > 
> > The hash function defined by <scheme> is used to generate <c-lock-string>
> > elements for the Cancel-Lock header field:
> > 
> >     c-lock-string = Base64(hash(c-key-string))
> >                            ^^^^
> > This hash function is the one specified with <scheme> (in both matching
> > elements <c-lock> and <c-key> in the Cancel-Lock and Cancel-Key header
> > fields of the two articles involved).
> >   
> >> IIUC the serving
> >> agent need not hash the provided key. Rather it only uses the scheme to
> >> select the locks to compare the hash against.  
> > 
> > <scheme> is used for both purposes.
> > 
> > Step 1 is defined in Section 3.5 as:
> > |
> > | 1. The <c-key-string> part of the <c-key> element is hashed using
> > |    the algorithm defined by its <scheme> part.
> > 
> > This is the operation for <c-lock-string> listed above.
> > The hash operation is required here because if the elements from the
> > Cancel-Key header field would be directly comparable, everybody could
> > forge them (copy from the public original article).
> > The hijack/abuse attack against <c-key-string> elements on their way
> > through the network (as discussed for the review from David Mandelberg)
> > is not the same. The relevant difference is who can *create* valid
> > <c-key-string> elements.
> > 
> > In Step 2 <scheme> is used again to skip <c-lock-string> elements that
> > were created using different hash functions. The compare operation from
> > Step 2 (with first operand from Step 1) is:
> > 
> >     Base64(hash(c-key-string)) == c-lock-string
> > 
> > The Base64 encode part is not explicitly noted in Step 1.
> > Base64 decoding the second operand instead would work too:
> > 
> >     hash(c-key-string) == Base64_decode(c-lock-string)
> > 
> > To be unambiguous and consistent with the definition of <c-lock-string>
> > from Section 2.1, the words for Step 1 in Section 3.5 maybe should
> > explicitly cover the Base64 encode operation. Suggestion:
> > 
> >     1. The <c-key-string> part of the <c-key> element is hashed using
> >        the algorithm defined by its <scheme> part and then Base64
> >        encoded.  
> 
> Hmm. That wasn't so clear to me, though on rereading I can understand it 
> that way.
> 
> But in that case, what is the point of passing the scheme around at all? 

The <scheme> part of <c-key> elements serve the following purposes:
- Backward compatibility
  Existing implementations use it this way since many years
- Define the hash algorithm to apply on <c-key-string> elements
- Select corresponding <c-lock-string> elements to compare against

> IIUC doing so doesn't add any additional security or privacy. You could 
> simply pass the hashed value in cancel-key. Then the verifying agent 
> wouldn't do any hashing.

In this case the creator of the Cancel-Key header field would not prove
that he really knows K.

> This would mean that the cancel-key would be 
> checked against locks that used a different hash, but that shouldn't 
> hurt anything.

Yes, it is unlikely that comparing against <c-lock-string> elements,
that used a different hash algorithm, will hurt.

But changing the general syntax of the header fields would break
backward compatibility to existing implementations. This should be
avoided with highest priority.

> [...]
> I didn't see a response from you on (4).

Sorry for the delay, I was busy.

| (4) Minor:
| 
| In Section 8.1/8.2 the syntax/semantics of the Owner/Change
| controller is unspecified. But IANA is charged with allowing/rejecting change 
| requests only from the "same" owner. How is IANA expected to verify 
| this?

If it is an experimental algorithm, I see no need for strong authentication
of external contributors.

For the algorithms on the standards track, the IESG is defined as owner
(may authenticate itself to IANA with a signature).

| What if a change is required but the original owner is no longer 
| available? 
| E.g., the owner might be specified by email address. Later the mail 
| server for that email address may no longer exist.

In this case the part "[...] where the owner of the registration [...]
has moved out of contact [...]" can be applied.

-- 
Michael Bäuerle