gh-libtom/libtomcrypt
Watch
1
0
Fork
You've already forked libtomcrypt
 0
Code Issues 42 Pull requests 17 Releases 9 Wiki Activity

Add SIV #319

Merged
sjaeckel merged 5 commits from add/SIV into develop 2025-01-09 06:36:02 -06:00
Conversation 20 Commits 5 Files changed 15 +1019 -29
sjaeckel commented 2017-10-25 09:01:08 -05:00 (Migrated from github.com)

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()

### Checklist <!-- Remove items that do not apply. For completed items, change [ ] to [x]. --> * [x] documentation is added or updated * [x] tests are added or updated This is a PoC/RFC for adding the enc+auth mode [RFC5297 SIV - Synthetic Initialization Vector](https://tools.ietf.org/html/rfc5297). 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 commented 2017-10-25 10:09:25 -05:00 (Migrated from github.com)

Does this enable implementation of AES-GCM-SIV?

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

Does this enable implementation of AES-GCM-SIV? https://tools.ietf.org/html/draft-irtf-cfrg-gcmsiv-06
sjaeckel commented 2017-10-25 10:32:40 -05:00 (Migrated from github.com)

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

I haven't looked at that draft yet but this PR implements the "predecessor"
sjaeckel commented 2017-10-25 12:41:14 -05:00 (Migrated from github.com)

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

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 commented 2017-11-05 14:52:33 -06:00 (Migrated from github.com)

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?

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 commented 2017-11-05 15:05:01 -06:00 (Migrated from github.com)

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

Just FYI - there is a bunch of `AES-GCM-SIV` test vectors in wycheproof test suite (look for `aes_gcm_siv_test.json`).
sjaeckel commented 2017-11-06 16:40:26 -06:00 (Migrated from github.com)

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

> 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 commented 2021-04-10 07:26:34 -05:00 (Migrated from github.com)

Now exists as RFC8452 (April 2019):

Now exists as RFC8452 (April 2019): * AES-GCM-SIV: Nonce Misuse-Resistant Authenticated Encryption * https://tools.ietf.org/html/rfc8452
levitte commented 2024-08-20 05:22:20 -05:00 (Migrated from github.com)

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?

> 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 commented 2024-12-13 09:11:08 -06:00 (Migrated from github.com)

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

Absolutely not abandoned, it just needs some love.

> This PR looks interesting, but needs some love... or is it abandoned? Absolutely not abandoned, it just needs some love.
sjaeckel commented 2024-12-13 09:22:04 -06:00 (Migrated from github.com)

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?

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 commented 2024-12-15 14:41:27 -06:00 (Migrated from github.com)

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.

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 commented 2024-12-15 15:29:12 -06:00 (Migrated from github.com)

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?

> 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 commented 2024-12-16 09:47:58 -06:00 (Migrated from github.com)

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

Yes. Sorry, what I meant, I'd rather not lose that API if a `siv_memory_multi()` comes along.
sjaeckel commented 2024-12-17 11:26:03 -06:00 (Migrated from github.com)

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

I decided on a mix for `siv_memory()`: vararg for AD, the rest only gets a single pointer.
sjaeckel commented 2024-12-17 12:31:49 -06:00 (Migrated from github.com)

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.

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` :shrug: It fails for `ARGTYPE={2,3,4}`, also with clang-18.
sjaeckel commented 2024-12-18 10:33:12 -06:00 (Migrated from github.com)

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?

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 commented 2024-12-20 12:05:27 -06:00 (Migrated from github.com)

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?

Actually, if you read [RFC 5297](https://datatracker.ietf.org/doc/html/rfc5297) 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 commented 2024-12-28 12:13:04 -06:00 (Migrated from github.com)

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().

> 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()`.
👍 1
levitte (Migrated from github.com) approved these changes 2025-01-09 05:20:06 -06:00
levitte commented 2025-01-09 05:20:26 -06:00 (Migrated from github.com)

This is workable enough.

This is workable enough.
Sign in to join this conversation.
Reviewers
No reviewers
levitte
Labels
Clear labels   
bug

   
duplicate

   
enhancement

   
feature

   
help wanted

   
improve API

   
invalid

   
needs documentation

   
not-planned
This ticket is not planned to be implemented and/or fixed from the core team.

  
rtfm

   
stale

   
support

   
wishlist
No labels
bug
duplicate
enhancement
feature
help wanted
improve API
invalid
needs documentation
not-planned
rtfm
stale
support
wishlist
Milestone
Clear milestone
No items
No milestone
next
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libtom/libtomcrypt!319
No description provided.