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

[Paul Kyzivat <[e-mail]>]

Michael,

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.

>> (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? 
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. This would mean that the cancel-key would be 
checked against locks that used a different hash, but that shouldn't 
hurt anything.

>> (3) Minor:
>> [...]
> 
> Issue 2 should be discussed first.
> 
> The hash function used for HMAC in Section 4 is unrelated to <scheme>
> and unrelated to the check algorithm from Section 3.5.
> 
> On the other hand, the hash function from <scheme> is not used to
> create <c-key-string> elements for the Cancel-Key header field.

I agree that this comment is moot given the answer to (2).

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

	Thanks,
	Paul