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

Add AES & ChaCha keys #228

Merged
merged 2 commits into from
Sep 6, 2021
Merged

Add AES & ChaCha keys #228

merged 2 commits into from
Sep 6, 2021

Conversation

expede
Copy link
Contributor

@expede expede commented Sep 2, 2021

Add common symmetric keys. We have a use case for storing & (securely) transmitting symmetric keys to encrypted data on IPFS.

Why No Operation Modes?

I didn't split out -CBC, CTR, -GCM, and so on because those are modes of operation, not the actual keys. AFAICT this also extends to XChaCha, which only accepts 256-bit keys but the contents of that 256-bit key is the same. We can add the operation modes to the format, but to me that feels like it belongs on the encrypted data, not on the key.

Why Separate Keys Types?

If we take the reasoning above to the extreme These are all random bits. In theory we could just have "256-bit symmetric key". Flagging the intention of the key feels right here ("oh this is a ChaCha key!") more than the operation mode did. But again, please let me know if you disagree!

Why 0xa?

AES starts with "A", and is an AEAD cipher 😛 I'm not attached to the number range — let me know where to move it and it shall be done!

@expede
Copy link
Contributor Author

expede commented Sep 2, 2021

@b5 Any thoughts?

@b5
Copy link
Contributor

b5 commented Sep 2, 2021

What if we dropped key length prefixes entirely & just relied on the length of the key bytes within the CID? eg: if you're looking at a 0xa0 key that's 24 bytes long, you know you've got a 192 bit AES key. Ditto for ChaCha.

If that works we can drop to just two multicodec entries, one for each symmetric key type

@expede
Copy link
Contributor Author

expede commented Sep 2, 2021

Yeah, totally, just parse the length. That could also be said of SHA, which has multiple length-based versions on this list. I wonder if there's a reason for that? Any idea who the right person to ask about the original reasoning would be?

@vmx
Copy link
Member

vmx commented Sep 3, 2021

The length in the multihash is meant to identify the length of the bytes to come, not the length of the original digest. The idea is that you can also store truncated hashes. So if the resulting hash is different per variant (like in SHA2), it should really be different multicodecs. If the truncated hash is the same as the longer one (like e.g. BLAKE3) there would only be one variant.

I though this would be documented somewhere, but I can't find anything, I've opened multiformats/multihash#133 in hope someone will find the time to document it properly.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to what @vmx said, I'm happy for you to err on the side of more codes to signal lengths unless you think it just doesn't make sense. There are a lot of numbers to be had if we need them.

I'm a little concerned about talk of using CIDs for these, since a CID is meant to be something like: a hash digest for a content addressed chunk of data, combined with information about the hash function used (multihash) and information about the nature of the data that's been hashed (codec).

You could probably mush these into an identity multihash with these as the codec, but it'd be stretching the purpose of CIDs (although such stretching does occur regularly and isn't ruled out).

Let us know if you're happy with these as they are and we can merge.

@b5
Copy link
Contributor

b5 commented Sep 3, 2021

Delighted to learn the reason for length-variant multicodecs. Looks great!

@expede
Copy link
Contributor Author

expede commented Sep 3, 2021

@rvagg

Let us know if you're happy with these as they are and we can merge.

I'm happy; let's merge it! 🚢

@expede
Copy link
Contributor Author

expede commented Sep 3, 2021

Thanks 😄

@rvagg rvagg merged commit 1d46d7f into multiformats:master Sep 6, 2021
@expede expede deleted the symmetric-keys branch September 6, 2021 01:35
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.

4 participants