Paul, On Mon, Sep 18, 2017, at 10:14 PM, Paul Kyzivat wrote: > I'm on tap to do the genart telechat review on > draft-baeuerle-netnews-cancel-lock-06. I see that nothing has been done > about the ABNF that was my primary concern in my prior wglc review of > -05: > > https://mailarchive.ietf.org/arch/msg/gen-art/wXKa5tadbo_xQbbUUCk9RZyB0w8 > > I'm told by the authors that Alexey asked that it be submitted this way, > but the ABNF *is* wrong, and I can't see how a new draft can be allowed > to be published with knowingly wrong ABNF. OTOH, the problem didn't > originate with this draft. Rather the problem originated with RFC5536, > and this draft has simply built on that mistake. Unfortunately the root > of the problem is with RFC5322, since its ABNF is structured in a way > that makes it really hard to define extensions such as those in RFC5536 > and this draft. So the cleanest solution I can see is to revise both > 5322 and 5536, and then this draft can build on those changes. That is a > rather involved fix but I don't see any other reasonable way. > > This prompted me to submit an erratum (#5116) on RFC5536 regarding this > issue. But errata aren't often acted on quickly when they require > revising RFCs. > > The question is: in the meantime, what to do about this draft? By > default I guess I will simply restate the issue in my genart telechat > review and leave it for the IESG discuss it. > > Anybody have a better idea?
It is on my to do list *before* the telechat. I contacted Pete Resnick and he suggested a slightly better fix to your erratum. This is just taking time on my part. Best Regards, Alexey > Thanks, > Paul > > On 7/14/17 12:58 PM, Paul Kyzivat wrote: > > Michael, > > > > Please see further comments inline. > > > > (Sorry - the indenting is getting deep, but I think we are pretty much > > done.) > > > > On 7/11/17 2:39 PM, Michael Bäuerle wrote: > >> 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? > > > > I don't know of any way to do so. Perhaps somebody else can provide a > > more definitive answer. You might try asking on rfc-interest. > > > >> 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]. > > > > I'm not sure about that. It might fly, or not. Again, I think you need > > to ask somebody more authoritative. > > > >> Rewriting RFC 5322 and RFC 5536 should be avoided if possible. > > > > I understand why you might not want to take on responsibility for that. > > But it is the right thing to do. Question is whether it needs to be done > > now. > > > >>>>> (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 studied the draft further. I finally get that it is simpler than I was > > expecting it to be. So really it is just that the lock contains > > hash(password) while the key contains the password. (The key is just an > > algorithmically generated password.) I couldn't believe you would really > > be passing the password in the clear, since that would expose it to > > on-path attack. But I guess your assumption is that this is really a > > one-time password, and that if it is correct then it will immediately be > > invalidated, and if it is wrong then it doesn't matter. > > > > So I'll withdraw my concerns on this section, except that maybe it could > > be clearer. Specifically, section 3 could be more explicit about what > > each distinct actor (poster, posting agent, moderator or injecting > > agent) does. > > > >>> [...] > >>> 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. > > > > I won't belabor this point because it will clearly be an infrequent event. > > > > Thanks, > > Paul > > > > _______________________________________________ > > Gen-art mailing list > > Gen-art@ietf.org > > https://www.ietf.org/mailman/listinfo/gen-art > > > > _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
