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

_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to