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

added serde feature for ser/de Strobe state #3

Merged
merged 3 commits into from
Dec 29, 2021

Conversation

dhuseby
Copy link
Contributor

@dhuseby dhuseby commented Dec 14, 2021

Signed-off-by: Dave Huseby dave@cryptid.tech

This does not break no_std since the optional serde feature uses serde in no_std mode.

Signed-off-by: Dave Huseby <dave@cryptid.tech>
@rozbb
Copy link
Owner

rozbb commented Dec 15, 2021

Woah, this looks great, thank you!

One thing before merging: the new serde tests here use known answer vectors. Where did they come from? If they just came from this implementation itself, then I think they have limited value. For comparison, the known answers in basic_tests.rs come from the Python2 reference implementation.

I think all I'd need in terms of serde tests is doing some cryptographic operations and ensuring the doing a serialize/deserialize round-trip in the middle yields the same end state as without doing the round trip. A round trip should be inserted at various places, including between streaming calls, just to verify robustness. The known-answer-style tests can be deleted. Does that make sense?

@dhuseby
Copy link
Contributor Author

dhuseby commented Dec 15, 2021

I think I know what you mean. Yes, it does make sense to delete the known-answer-style tests. I can restructure the round-trip tests so that there is one strobe state the does a bunch of operations without a round-trip in the middle and another strobe state that does the same bunch of operations with a round-trip or two between some operations and then we compare the two states in the end. They should be the same if the serde code does it's job correctly.

@dhuseby
Copy link
Contributor Author

dhuseby commented Dec 15, 2021

It's important to note that this dumps the whole raw keccak state as bytes. This data must be protected because it reveals the private part of the keccak state. I envision that this data will be treated with the same care and reverence as a secret encryption key. It is outside of the scope of this crate but I should probably add some documentation notes warning users of the pitfalls. Knowing the full keccak state--including the private part--allows for the strobe states to be run in reverse to recover any keys or other data mixed into the strobe state.

Signed-off-by: Dave Huseby <dave@cryptid.tech>
Signed-off-by: Dave Huseby <dave@cryptid.tech>
@dhuseby
Copy link
Contributor Author

dhuseby commented Dec 15, 2021

OK, I think I'm done. Please review.

@rozbb
Copy link
Owner

rozbb commented Dec 20, 2021

Sorry for the delay, I will get to this this week. I had to think about the cost and benefit of implementing a way to serialize secrets. Using this feature by accident could lead to catastrophic compromise. The benefit is pretty concrete though: being able to hibernate or transfer Strobe sessions between computers. Out of curiosity, why do you need this feature?

Regardless, I think it makes sense to merge this and feature gate it so nobody uses it by default. Will do this week. Thanks again for the PR.

@dhuseby
Copy link
Contributor Author

dhuseby commented Dec 27, 2021

@rozbb I agree on the feature gate because of the potential catastrophic compromise. The use case for this is twofold:

  1. I'm implementing Disco and if the strobe state can be saved to disk (encrypted) then we finally have a fully decentralized session layer solution where the disco/strobe state is the session. Think about it. Let's say you and I swap our known public keys and then we run through the KK handshake process to set up a Disco session between us. Once the session is set up, you and I can send encrypted messages back and forth using any transport (e.g. sockets, QR codes, carrier pigeons) and with any amount of time between messages. Being able to save the session to disk allows us to keep the session state around for an indefinite amount of time between messages. This is how we're going to send secure messages between planets.
  2. I also need to store the strobe state to disk because I'm using it for creating append-only log files similar to the secure scuttlebutt protocol. It's a handy way to track key history and associated meta data locally. I need to be able to store the strobe state to disk so that later when I want to add a new entry to the append-only log I can restore the strobe state from disk, add the new data to the log and then store the updated strobe state off to disk. Tracking key state this way also comes in handy when you consider using Disco over long stretches of time with key rotations. These "provenance logs" of my key state are a secure way to publish my key history and allow for the Disco state to be updated as key rotations happen by both participants.

@rozbb
Copy link
Owner

rozbb commented Dec 28, 2021 via email

@rozbb rozbb merged commit 4700c11 into rozbb:master Dec 29, 2021
@rozbb
Copy link
Owner

rozbb commented Dec 29, 2021

Just cut a new release. I realized that the extra serde tests weren't necessary if I just interleaved round trips with the other comprehensive tests. Thanks again for the PR!

@dhuseby
Copy link
Contributor Author

dhuseby commented Dec 31, 2021

For use case (2) it is intended to be a public, yet secure, log of old public keys and associated data. If we're going to have digital signatures on everything we have to keep old keys around. I plan on calling ratchet() after I update a new state to the log just to prevent any rollback potential.

...and I'm not Cloudflare ; )

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