Add SIV

[sjaeckel]

Checklist

• documentation is added or updated
• tests are added or updated

This is a PoC/RFC for adding the enc+auth mode RFC5297 SIV - Synthetic Initialization Vector.

Feel free to tear it apart, improvements on the API welcome.

I had to decide how to process the AD's

1. incremental
2. by passing a vararg
3. by passing an array of pointers

1 wasn't really an option AFAIU the RFC
whether 2 or 3 I was like ¯\_(ツ)_/¯ so I went for 3

I didn't really look if it would make sense to have the context exposed so we could split the processing up in init()->add_ad()->{en,de}crypt()->done()

[karel-m]

Does this enable implementation of AES-GCM-SIV?

https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-06

[sjaeckel]

I haven't looked at that draft yet but this PR implements the "predecessor"

[sjaeckel]

I've read a bit through the ML and I think we should wait until the RFC is finished to prevent something like #256.

[sjaeckel]

Okay I played a bit with the implementation and I'm going to add an incremental add_AD() function.

Also there should be a siv_memory() function which has to support multiple AD's in one function call. I think I'll go the varargs way for that as it's already used as a pattern in the library whereas the array of pointers isn't. Any better ideas?

[karel-m]

Just FYI - there is a bunch of AES-GCM-SIV test vectors in wycheproof test suite (look for aes_gcm_siv_test.json).

[sjaeckel]

ust FYI - there is a bunch of AES-GCM-SIV test vectors in wycheproof test suite

thanks, but they don't help me now as this is only AES-SIV :)

I found those:
https://github.com/randombit/botan/blob/master/src/tests/data/aead/siv.vec
https://github.com/cryptomator/siv-mode/blob/master/src/test/resources/testcases.txt (attention, the file has 17mb ... you probably shouldn't click on the link ;) )

and I planned to hand-pick some of the cryptomator/siv-mode

[karel-m]

Now exists as RFC8452 (April 2019):

• AES-GCM-SIV: Nonce Misuse-Resistant Authenticated Encryption
• https://tools.ietf.org/html/rfc8452

[levitte]

Also there should be a siv_memory() function which has to support multiple AD's in one function call. I think I'll go the varargs way for that as it's already used as a pattern in the library whereas the array of pointers isn't. Any better ideas?

That would essentially be siv_memory_multi() no?

This PR looks interesting, but needs some love... or is it abandoned?

[sjaeckel]

This PR looks interesting, but needs some love... or is it abandoned?

Absolutely not abandoned, it just needs some love.

[sjaeckel]

My idea was to refactor what exists into a similar API to what we usually provide as XYZ_memory_multi(), maybe in the style of crypt_fsa().

I would not split it up into an iterative/incremental API.

What do you think? Do you have a better idea?

[levitte]

I'd like to see a variant that takes an array of pointers as well, for convenience for programs that build the set of ADs dynamically.

[sjaeckel]

I'd like to see a variant that takes an array of pointers as well, for convenience for programs that build the set of ADs dynamically.

The current API would satisfy that, right?

It feels a bit unnatural/clumsy to use IMO, but I also don't see a better way to provide an API that can accomplish this.

Do we even need another version of the API?

[levitte]

Yes. Sorry, what I meant, I'd rather not lose that API if a siv_memory_multi() comes along.

[sjaeckel]

I decided on a mix for siv_memory(): vararg for AD, the rest only gets a single pointer.

[sjaeckel]

Oh yeah, that looks like a fun one to debug ... seems like something makes the build of the new siv code with clang fail when -DARGTYPE > 1 🤷

It fails for ARGTYPE={2,3,4}, also with clang-18.

[sjaeckel]

While reading through the RFC before writing the documentation, I'm not so sure anymore if we should really hide the detail of "how a nonce is treated in SIV".

I'm thinking of adding the nonce as a separate argument to the API, what do you think? Currently it's kind of hidden as it must be passed as the "last ad element".

Additional to that I'm not so sure if the "only one-shot API" way is the good way (resp. if it's even usable outside of the testcases)! If it's used in some kind of network protocol, some state will most likely have to be persistent/incrementing and can't always restart from scratch ... or am I missing something?

[levitte]

Actually, if you read RFC 5297 carefully, you can see that it doesn't stipulate that the nonce must be the last AD input, but rather that it's "for the purpose of interoperability"... which is, of course, a good choice.

It's also perfectly possible not to have a nonce at all.

So in essence, this comes down to what choices we give the developer using libtomcrypt.
Of course, it's not wrong to have an explicit nonce argument, and if we want to allow non-interoperable encryption, allow the caller to pass NULL there.

Me, I'm leaning toward leaving the choice of how to handle the nonce, if any, to the caller. It does mean that the caller must understand how a nonce can be handled, and how it should be handled for interop... but that's a question of documentation, no?

Regarding "only one-shot API", I can't see any technical reason against having an incremental interface as well. But, must it be added in this PR, or could that be further development?

[sjaeckel]

Me, I'm leaning toward leaving the choice of how to handle the nonce, if any, to the caller. It does mean that the caller must understand how a nonce can be handled, and how it should be handled for interop... but that's a question of documentation, no?

I've played a bit with the API and it feels just fine if the nonce has no separate API parameter.

Regarding "only one-shot API", I can't see any technical reason against having an incremental interface as well. But, must it be added in this PR, or could that be further development?

It could be further development, but I'd prefer to think of how we'd name it then before merging this PR. I guess if we decided to have an iterative API, it should have two API functions siv_encrypt() and siv_decrypt() to align with the rest of the LTC APIs.

Therefore I've renamed these APIs to siv_{en,de}crypt_memory().

[levitte]

This is workable enough.