Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

derive_key: Add default-off, unstable derive_key_u8ref API call #424

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jac-cbi
Copy link

@jac-cbi jac-cbi commented Aug 28, 2024

This can be enabled with the unstable feature flag, i_know_what_i_am_doing.

Developers attempting to deploy blake3 via the Rust crate often encounter derive_key(&str, ...) which forces the INFO parameter to be a valid UTF-8 string.

Please see the discussion in issue 13, in particular, this comment.

The recommended course of action for those with non-UTF-8 INFO is to use hash and keyed_hash and open code their own derive_key which takes an &[u8].

This is not good for two reasons: First, it is quickly seen that this forces deviation from the Blake3 paper for how derive_key should be performed, as hash doesn't let you set the flags field. Attempting to use the underlying hash_all_at_once fails because it's not exported. Second, the developer is now forced into the position of maintaining their own patches on top of the blake3 repo. This is a burden on the developer, and makes following blake3 upstream releases much less likely.

This patch proposes a reasonable compromise. For developers who require &[u8] INFO field, they can enable the rust feature flag i_know_what_i_am_doing to expose the API derive_key_u8ref. This enables developers to use upstream blake3 directly, while still discouraging sloppy behavior in the default case.

This can be enabled with the unstable feature flag, `i_know_what_i_am_doing`.

Developers attempting to deploy blake3 via the Rust crate often encounter
`derive_key(&str, ...)` which forces the INFO parameter to be a valid UTF-8
string.

Please see the discussion in issue [13](BLAKE3-team#13),
in particular, this [comment](BLAKE3-team#13 (comment)).

The recommended course of action for those with non-UTF-8 INFO is to use `hash`
and `keyed_hash` and open code their own `derive_key` which takes an `&[u8]`.

This is not good for two reasons:  First, it is quickly seen that this forces
deviation from the Blake3 paper for how `derive_key` should be performed, as
`hash` doesn't let you set the `flags` field.  Attempting to use the underlying
`hash_all_at_once` fails because it's not exported.  Second, the developer is
now forced into the position of maintaining their own patches on top of the
blake3 repo.  This is a burden on the developer, and makes following blake3
upstream releases *much* less likely.

This patch proposes a reasonable compromise.  For developers who require `&[u8]`
INFO field, they can enable the rust feature flag `i_know_what_i_am_doing` to
expose the API `derive_key_u8ref`.  This enables developers to use upstream
blake3 directly, while still discouraging sloppy behavior in the default case.

Signed-off-by: Jason Cooper <[email protected]>
@jac-cbi
Copy link
Author

jac-cbi commented Aug 28, 2024

Apologies in advance, I visit GitHub just rarely enough to forget its ins and outs. I've no idea what "1 workflow awaiting approval" means except that I might've done it wrong :-/

If you need me to edit anything just let me know and I'll resend

@oconnor663
Copy link
Member

workflow awaiting approval

You didn't do anything wrong, that's just a change GitHub made a couple years back where they stopped automatically running CI on PRs from new contributors. Apparently some people were "contributing" cryptocurrency mining code to random repos and abusing GitHub compute. Something like that. Anyway, I've clicked the run button :)

@oconnor663
Copy link
Member

oconnor663 commented Aug 29, 2024

The recommended course of action for those with non-UTF-8 INFO is to use hash and keyed_hash and open code their own derive_key which takes an &[u8].

I agree that this is a thing you can do, and even a thing that I suggested in that thread :) but I might not go quite as far as "recommended course of action" without hearing more about what a specific caller is trying to accomplish.

hash doesn't let you set the flags field

Indeed, nothing lets you set that field, and I'd be surprised if we ever exposed a public API for it. To kind of repeat and summarize my wall of text from #13, the goal of derive_key is to help uncoordinated callers who happen to share key material from accidentally having bad interactions (reusing each other's nonces with the same key, publishing each other's intermediate secrets etc). Of course if two callers can coordinate with each other, then domain separation is easy: you prefix all your inputs with "foo", and I'll prefix all my inputs with "bar", and boom we're domain separated. You don't need any special modes or features for that. It's when there's no coordination that you have to rely on requirements and conventions to keep you safe. And so the value of derive_key is tied to its requirements and conventions, that plus the fact that we internally domain-separate it from the other APIs that don't have the same requirements. We'd lose the second half of that if there was any public API where you could set arbitrary values for the flags field.

For developers who require &[u8] INFO field

My suspicion for a long time is that needing arbitrary bytes is usually a side effect of violating the "hardcoded" requirement of the derive_key context string. Could you tell me more about your use case?

I should mention that we do provide a C API that takes bytes, blake3_hasher_init_derive_key_raw. The issue there is that ordinary ASCII/UTF-8 C strings are null-terminated, but strings in most other languages are not, and we didn't want to force bindings to heap allocate and copy strings just to add the null terminator. However, Rust doesn't have the same problem. As you probably already know, std::str::from_utf8 doesn't require a null terminator and doesn't do any heap allocation, and you can cheaply convert a &[u8] to a &str. If you're completely sure the bytes are valid UTF-8, you can even do a zero-cost unsafe conversion. But if you're not sure...I'd like to hear more about why you're not sure.

One scenario I could imagine (not suggesting this is your scenario, just making stuff up here) is using BLAKE3 to implement API-compatible replacement for an existing HKDF construction that currently takes its info/ikm/salt parameters as raw bytes. I'd suggest that derive_key is not a good fit for building such a replacement, precisely because it imposes requirements on the context string that the previous API did not impose.

@jac-cbi
Copy link
Author

jac-cbi commented Aug 29, 2024

The recommended course of action for those with non-UTF-8 INFO is to use hash and keyed_hash and open code their own derive_key which takes an &[u8].

I agree that this is a thing you can do, and even a thing that I suggested in that thread :) but I might not go quite as far as "recommended course of action" without hearing more about what a specific caller is trying to accomplish.

hash doesn't let you set the flags field

Indeed, nothing lets you set that field, and I'd be surprised if we ever exposed a public API for it. To kind of repeat and summarize my wall of text from #13, the goal of derive_key is to help uncoordinated callers who happen to share key material from accidentally having bad interactions (reusing each other's nonces with the same key, publishing each other's intermediate secrets etc). Of course if two callers can coordinate with each other, then domain separation is easy: you prefix all your inputs with "foo", and I'll prefix all my inputs with "bar", and boom we're domain separated. You don't need any special modes or features for that. It's when there's no coordination that you have to rely on requirements and conventions to keep you safe. And so the value of derive_key is tied to its requirements and conventions, that plus the fact that we internally domain-separate it from the other APIs that don't have the same requirements. We'd lose the second half of that if there was any public API where you could set arbitrary values for the flags field.

Ack.

For developers who require &[u8] INFO field

My suspicion for a long time is that needing arbitrary bytes is usually a side effect of violating the "hardcoded" requirement of the derive_key context string. Could you tell me more about your use case?

Sure :) Some quick background: The paper is written, and I have a few contacts doing some review right now. The paper will be published (ar, at a minimum, arxiv), and the code will be open source. (I started writing the code, which led to this question ;-) )

Without rewriting the paper, I'm creating an ephemeral cryptographic protocol (think generic ZRTP with modern lessons-learned incorporated) designed to maintain a context between two peers over extended periods of time, like years. Over many connections, and many runs of the process, without losing cryptographic state sync. Since the contexts last so long (there is session key rotation, keys are derived from a shared secret, using derive_kdf), I wanted to not just domain separate by application (static string), but also by context.

Once the context is established, the info buffer is const for the life of that context. However, in order to incorporate context-distinguishing, non-private data, I'm looking to concatenate some unique info about the environment of the context establishment. Things like public IP / port of both peers with epoch time in seconds. Most of these are binary data, and I'd prefer to avoid silly workarounds like hashing the data, then converting to a string of hex. Cortex-M4 is in my scope of support, so unnecessary conversions are to be avoided.

I should mention that we do provide a C API that takes bytes, blake3_hasher_init_derive_key_raw. The issue there is that ordinary ASCII/UTF-8 C strings are null-terminated, but strings in most other languages are not, and we didn't want to force bindings to heap allocate and copy strings just to add the null terminator. However, Rust doesn't have the same problem. As you probably already know, std::str::from_utf8 doesn't require a null terminator and doesn't do any heap allocation, and you can cheaply convert a &[u8] to a &str. If you're completely sure the bytes are valid UTF-8, you can even do a zero-cost unsafe conversion. But if you're not sure...I'd like to hear more about why you're not sure.

I'm definitely having to bite the bullet and call some C code in the initial code, but I'd prefer to avoid it where possible. I say this with extreme sadness, as I've written in C for about 25 years, and only picked up Rust in the past three.

One scenario I could imagine (not suggesting this is your scenario, just making stuff up here) is using BLAKE3 to implement API-compatible replacement for an existing HKDF construction that currently takes its info/ikm/salt parameters as raw bytes. I'd suggest that derive_key is not a good fit for building such a replacement, precisely because it imposes requirements on the context string that the previous API did not impose.

During the design phase, I looked at HKDF, but I've not felt comfortable with how salt is required to be uniformly distributed randomness. I get why, it's the secret in the HMAC under the hood. But it seems unnecessarily complicated to me. All of the deployments concatenating what should be a salt with the info parameter seems like a red flag of a broken design.

I chose blake3 because it was the first hash algorithm I found that clearly understood password hashing is a different animal from hashing. 32-bit is a first class citizen, it has a Rust implementation, and I'm a lot more comfortable with how derive_key works under the hood vice HKDF. I just need &[u8] :-P

To summarize, is there a specific reason why blake3's derive_key() is not appropriate for my use case? Say, I was writing this in C, this issue would've never arisen: Am I doing something wrong, or is there something I missed?

Thanks!

@jac-cbi
Copy link
Author

jac-cbi commented Sep 5, 2024

@oconnor663 gentle ping?

@oconnor663
Copy link
Member

Oops, thanks for the ping.

in order to incorporate context-distinguishing, non-private data, I'm looking to concatenate some unique info about the environment of the context establishment. Things like public IP / port of both peers with epoch time in seconds.

Gotcha. I think what I'd recommend here is a two-step process along the lines of: 1) derive_key your long-lived context data as input bytes, using a hardcoded context string, producing an intermediate 32-byte key. Then 2) keyed_hash your shorter-lived inputs with that intermediate key. If your design doesn't need more than one context string for step 1, you can also replace derive_key with just hash. That hash+keyed_hash construction would be identical to what derive_key does internally. The only differences are that it doesn't use the internal domain-separation bitflags that are unique to derive_key, and that (for that reason) it's not subject to the security requirement that you must hardcode the first argument.

Thinking about your example is helping me clarify in my head what derive_key is trying to do. It looks like it's solving two different problems at once:

  1. Deriving independent subkeys from a shared secret, to help you use the same secret safely with different algorithms, or with different instances of the same algorithm that shouldn't interact for whatever reason.
  2. Hashing two arbitrary-length strings together unambiguously, especially-but-maybe-not-exclusively when one of those strings is some sort of key.

But in fact, derive_key is only intended for the first thing, not the second thing. There's nothing wrong with the second thing per se, but the hardcoded context requirement is important for the first thing, and unfortunately it's incompatible with the second thing.

I think all of this provokes a very natural follow up question, and if you'll forgive me, I'll express that question as a meme:

image

Am I really telling folks to hand-roll their own TupleHash? Well...yes...I am. Which is lame. And I'm ashamed. But in my defense, even if we said derive_key was a fine way to hash any two strings, we'd still be telling folks to figure things out for themselves as soon as they had three strings. Or consider another flavor of the same question: Do the hashes of these two structs need to be domain-separated from each other?

struct User {
    name: String,
    age: u32,
}

struct Server {
    domain: String,
    port: u32,
}

TupleHash doesn't know. It can't know. Different applications need different things. If we came up with a fully generic solution (#[derive(HashMyStruct)]) that incorporated every possible bit of local context, then many users would be dismayed to discover that they can no longer change the names of their structs or fields without breaking backwards compatibility. It's surprisingly difficult to come up with a general solution to this, much like it's surprisingly difficult to come up with a one-size-fits-all serialization format.

Another related question, how do we compare these three different approaches to the same problem:

  1. keyed_hash(key=hash(A), input=B)
  2. hash(hash(A) + hash(B))
  3. hash(A + B + len(B))

All of these are valid ways to unambiguously hash A and B together, with a hash like BLAKE3 or SHA-3 that doesn't allow length extension. I recommended the first approach because it's slightly more efficient when A rarely changes. An advantage of the second approach is that, if A and B are both very long, it's possible to start processing B before you've gotten to the end of A, or to process A and B on different threads. The third approach wins when A and B are both very short, because it's the only option that can reduce to a single call to the compression function. I think there's an argument to be made that some hybrid approach might be best, which dynamically switches between the third and the second as inputs grow. But for any given application, it's probably best to keep it simple and pick one of those three based on whatever feels right for the problem at hand?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants