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
pgpHFq8im6Bpt.pgp
Description: OpenPGP digital signature
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
