Interop issue with HKDF deriveBits operation when length is zero #370
Comments
|
For what it's worth, I've filed a bug in Chrome, since it was the one diverging, but it seems Chrome would rather avoid introducing a discontinuity in the deriveBits function at zero, in the same line of argumentation used in other related discussions (eg issues #322, #351 or #369 ); I'd let @davidben correct or complete this position statement, if needed. I'd like to read opinions from @annevk and @Frosne, representing WebKit´s and Gecko's position respectively, and see how far we are to get an agreement on whether we should change the spec or not. Finally, it'd be interesting to have @twiss feedback about the rationale used back then to merge the PR#275 mentioned before. |
|
Chrome wants to throw not just for zero, but for any truncation? Or if that's not possible, not at all? If so, investigating whether we can throw for any should probably be resolved first. Please also copy @nmahendru on future issues where WebKit's input is desired. |
|
It's hard for me to grasp this as an "interop" issue. Asking HKDF or PBKDF2 (or any deriveBits algorithm) for a zero-length output is just nonsense. |
My understanding is that the throw for any truncation would be the ideal, but it seems not many chances of getting an agreement soon (eg, it'd break wrap / unwrap). What I think it's clear is that Chrome doesn't want to throw on 0 only.
Sure. |
|
As long as we want to get to a world where 0 throws it would seem bad for Firefox and Safari to change here. |
I mean interop issue derived from the different behavior of the 3 major browsers. It seems that the spec as it is has caused this interop issue; it's been discussed many times that 0 is as non-sense as 1 or 2, so that defining 0 as s special case was a mistake. |
But the spec is now in a weird situation: we are changing WebKit and Firefox implementation of other algorithms (eg ECDH, X25519) to make the deriveBits return an empty string when passing 0, so that the value of 'length' is treated uniformly. It seems that throwing on any truncation is the most sensible thing to do, but will require time it seems (it's backwards incompatible and there are issues with wrap / unwrap), so we would need to agree how to move on: 1- assume the interop in HKDF until we agreed on the issue about throwing on any truncation |
|
This is not a truncation problem, specifying arbitrary non-0 length during HKDF and PBKDF2 is valid, in here it is just 0 that's the special case. And the spec handles it correctly now. |
Could you point where in the HKDF's RFC is stated that 0 is invalid ? If there is a clear statement of 0 being invalid for HKDF, then the spec indeed handlers both cases correctly, I agree. However, 0 remains valid for ECDH and X25519, is the spec handing those cases correctly as well ? I guess the problem is that although the API defines the same operation's signature for the 4 algorithms, each of them have substantial differences that makes the API confusing. Sorry if it's just me, but at this point I don't have a clear idea of how to solve the issue. If I've understood it correctly, you would go with 2) right ? |
No, but we've well established it is a nonsense thing to do, haven't we? PBKDF2 and HKDF algorithms in the spec already cover that.
ECDH 0 length (and subsequently X25519/448) will be handled as not valid when we agree to remove the possibility to truncate their output altogether.
I don't see a problem. PBKDF2 and HKDF are made to support arbitrary output, it's just 0 that doesn't make sense. ECDH and X... don't make sense if anything but their full output is used. It is unfortunate that they share an API method, yeah, but that doesn't mean their argument processing must be the same for every value.
Yeah, there's nothing to do in the spec for HKDF and PBKDF2 wrt the |
I suggested to update HKDF simply to be consistent with PBKDF2. Probably I should've checked more thoroughly that everyone's OK with that, but I didn't expect this to be controversial. My bad, in any case. Now that we're here, though, I struggle to see the argument for reverting part of the change and allowing to use HKDF to derive 0 bits; especially if we're going to throw for 0 length for every other algorithm as well in #351 (which to be clear is of course still up for discussion). It's a bit unfortunate that RFC5869 (for HKDF) and RFC8018 (for PBKDF2) are inconsistent in how they handle 0 bits, but I don't think we should necessarily expose that inconsistency in Web Crypto, especially since there's no real reason to ask for 0 bits. (And the alternative of consistently returning an empty ArrayBuffer for both ECDH and PBKDF2 would require a step explicitly saying "if |
No, KDF functions and Diffie-Hellman primitives have nothing to do with each other. The behavior of one has no bearing on the behavior of the other. A Diffie-Hellman primitive returns a fixed-width secret. This secret is not guaranteed to be uniform and thus truncating it is a questionable. A good API would not have had this truncation; rather this is an artifact of WebCrypto merging operations into these generic verbs, rather than just having a feature-detectable HKDF function, ECDH function, etc. Of course, if an API does truncate it, truncating to any length, including zero, of course has a natural definition. A KDF function takes a secret and an arbitrary length N, and outputs N bytes of [computationally indistinguishable from] uniform output. This output is uniform, so asking for N bytes of it will naturally get you N bytes of uniform output with all the properties one can expect from that. Yes, asking for 0 bytes of uniform output is a silly degenerate case. Asking for 1 byte of uniform output is also largely silly. Asking to sort an empty array is also a degenerate case, and yet there is a natural thing for empty sorts to do. In the same vein, there is a natural answer to asking for 0 bytes of uniform output. We don't introduce random discontinuities into our functions because that means callers need to add special cases and reason about whether they hit these discontinuities when analyzing whether a function might accidentally throw an exception, etc. All these special cases around zero stem from a misreading of the original specification, where null and zero were mixed up. It is regrettable that the new maintenance of the WebCrypto specification, as well as WPT tests, perpetuated this mistake. I was involved in the early Chromium implementation work here and I recall we spent quite a lot of effort making sure these edge cases behaved correctly (OpenSSL's APIs sometimes do weird things), so this was very much not the intent of the specification. We should fix this mistake, not carry it forward. Zero means zero. |
|
I think there's a difference between sorting an arbitrary-length array, where the input length may depend on user-provided data, and generating an arbitrary amount of bytes, where the output length is (typically) defined by some specification or hardcoded in the application, or at least is one of a fixed number of predefined values, dependent on the key size of the symmetric encryption algorithm to be used, for example. It seems implausible to me that an application would need a user-defined number of bytes, and would need to work around a discontinuity at 0. Also, if that situation happens in the case of PBKDF2, they'd need to handle the discontinuity as well, unless we add a step there saying that "if |
I mean, this very repository (WebCrypto) is such an application. If you're generating output on behalf of something else, that something else might ask you for a funky length. Correctness is easier to reason about if you don't have to worry about throwing an exception or checking a return code or whatever somewhere in your input domain. I definitely agree it is rare, because asking for zero bytes isn't very sensible, but if we're not trying to enforce sensible output sizes (in which case the minimum would be higher than zero, but I do not think we should do that), we shouldn't have a discontinuity like this.
I would generally prefer that we defer to the cryptographic primitive, certainly for significant cryptographic checks as we've discussed elsewhere. However, that does, as you note, lead us to an inconsistency between PBKDF2 and HKDF. Unlike HKDF and ECDH, which are unrelated animals, PBKDF2 and HKDF are a bit more similar. (Though still not really substitutable as one's designed for low-entropy secrets and the other isn't.) I think PBKDF2(0) should also just return the empty array. That's what BoringSSL (and OpenSSL) do. It's true that the spec does say "positive integer" here (I'm reminded of the "is zero a natural number" debate...), but I think avoiding the discontinuity is right semantics for a function like this, and I somehow doubt that line in PBKDF2 was specifically written to care one way or another about zero. |
I meant an application using Web Crypto, not an application of HKDF. Web Crypto I'd argue gets the number of bytes to produce from the application, not from the user directly. My claim was simply that the application is unlikely to pass that number directly from the user to Web Crypto, and if they do need to do that for some reason, it's not entirely unreasonable to need some bounds checking too.
OpenSSL's documentation refers to SP800-132, which doesn't have the "positive integer" wording. However, the algorithm described there seems to break for Btw, as a somewhat orthogonal point, Argon2 specifies a minimum derived key size of 4 bytes; so if we want to add that to Web Crypto at some point we wouldn't be able to get around some discontinuities (nor inconsistencies, to be fair) anyway. |
|
I think we are discussing here 2 different things; whether the deriveBits API fits in the different algorithms or not and what to do with some special cases of the 'length`argument. Although both are related, and probably the former is the root case of the later, we can handle it separately. Lets start with the zero length, which is what this specific issue is about. I can see David's point about avoiding discontinuity in the deriveBits API; also, I don´t think that the fact that PBKDF2 spec requires a "positive integer" is enough to force the WebCrypto API to throw an exception. Unless we want to protect consumers of the API against security issues or undefined behavior, we can always catch exceptions from the crypto library or introduce early checks and do what we consider more reasonably. Allowing 0 for both, HKDF and PBKDF2, returning an empty string is a sensible outcome for me. However, zero is not the only case introducing discontinuity, since the spec states that we must throw an exception for values non-multiple of 8 as well. As a matter of fact, we allow non-mutiple of 8 for ECDH and X25519; FF even has a bug about it and I've submitted a PR to add new tests to cover these cases. Hence, although I agree on avoiding treating zero as a special case, the discontinuity is already there. Now thinking on former question, about the ergonomics of the deriveBits API, and perhaps as a way to address the issue about special values of the length argument, I'd like to remind that we have recently introduced a change to consider the 'length' argument as optional. As such, we can consider that it applies only to certain algorithms, throwing otherwise. It's not uncommon for web APIs to have this behavior, considering certain combination of parameters as incompatible. So my proposal would be to define 'length' as a valid parameter for HKDF and PBKDF2, throwing an exception if used in combination of ECDH or X25519 algorithms; this could be discussed further in the issue #369. Given that it applies to HKDF and PBKDF2, we may remove any reference to the term 'truncation' and consider it just an argument of the KDF function. As such, there is no reason to treat 0 as a special case; from the WebCrypto API would accept any value and it'd be responsibility of the deriveBits caller to use the result properly. |
OK. @annevk, @nmahendru, and @Frosne, what would be your opinion about adding a step to PBKDF2 to return an empty ArrayBuffer for |
We can make the change in WebKit. But we have to update the spec too. Currently WebKit is doing what this says: |
|
We can make the change as well. |
|
OK, thanks both! I've created a PR in #380. |
The current implementation status of HKDF deriveBits operation in the 3 major browser shows a different behavior when passing zero in the 'length' argument:
This is also described by the WPT covering this case.
It's worth mentioning that the change in HKDF behavior has been introduced in the PR #275, which was defined to address the [issue #274], originally filed to report a bug in PBKDF2. For some reason we assumed both ,HKDF and PBKDF2 should have the same behavior in case of length is zero.
Both HKDF and PBKDF2 algorithms were shipped in the 3 browsers long time ago, so any change would imply a non-backwad compatible change. However, I'd like to see if we can solve this interop issue, now that we are discussing other interop issues that affect the deriveBits operation.
The text was updated successfully, but these errors were encountered: