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

Safe erasure of secret data #29

Open
kamyuentse opened this issue Mar 11, 2018 · 2 comments
Open

Safe erasure of secret data #29

kamyuentse opened this issue Mar 11, 2018 · 2 comments
Labels

Comments

@kamyuentse
Copy link
Collaborator

NOTE: If the protocol required changes, put this proposal aside, and fix the protocol first.

The Encryption Algorithm is fixed during the design of CSwitch, in other words, this is a part of the CSwitch's protocol, which is the starting point of this proposal.

The design of this proposal has the following goals:

  • Decouple the protocol part to the details of the implementation.
  • Make a strong trait constraint to the crypto primitive types for security consideration.

Currently, we have the following crypto primitive types:

Name Size impl traits
DhPrivateKey \ \
DhPublicKey 256 bits Clone, Group1
PublicKey 256 bits Clone, Group1, Group2
Signature 512 bits Group1
Salt 256 bits Clone, Group1
RandValue1 128 bits Clone, Group1
HashResult2 256 bits Clone, Group1, Group2
Nonce 96 bits Group1

Note:

  • Group1: Including AsRef<[u8]>, Deref<Target=[u8]> and DerefMut<Target=[u8]> for reading and writing the underlying bytes conveniently.
  • Group2: Including PartialEq, Eq and Hash, which means those types may be the key of a HashMap, HashSet, etc.

1: Should we rename this type to RandNonce, because I notice that we always use the rand_nonce in where we hold this value.

2: The name of this type seems to obey the Rust's naming conventions, anyone has an idea about the name here?

Consideration about the Clone

For deriving the symmetric keys, each party needs to save a copy of those types during the handshake: DhPublicKey, PublicKey, Salt. For calculating the channel identifier, they need the DhPublicKey, RandValue. They need impl Clone, but I am not very sure whether we should do that. This information should be consumed and erase as soon as possible.

Drop the secret data safely

For the type mentioned above, I think we should manually implement Drop for them, which erase the memory block where holds these value to ZERO. This will be done by the following code:

impl Drop for Nonce {
    fn drop(&mut self) {
        unsafe {
            for b in &mut self.0 {
                ::std::ptr::write_volatile(b, 0);
            }
        }
    }
}

Traits required by testing

Some traits are required when performing tests, i.e. Eq, PartialEq .etc. I think this can be done by #[cfg_attr(feature = "test", derive(Eq, PartialEq)))], this trick alse can be use for other types.


Comments and fill in the other details are welcome.

@realcr
Copy link
Member

realcr commented Mar 13, 2018

@kamyuentse : For dropping the secret data safely: Can you find out how this is done in Ring? If you can't find out, can you open an issue on Brian's ring repository, asking what is the right way to eliminate secret values from memory?

Should we rename this type to RandNonce:

Isn't this the nonce that is used for the sliding window? If so, in that case it is not random, but an incrementing sequence.

I think that Salt and RandValue are very similar. Maybe we can rename them to Rand256 and Rand128. What do you think?

@kamyuentse
Copy link
Collaborator Author

kamyuentse commented Mar 13, 2018

  1. I sent an e-mail to BrainSmith several hours ago, want to know his consideration about the story of zeroization in the ring, but no reply.

  2. Rand128 and Rand256 sounds good.

@realcr realcr added the P-Low label Jul 18, 2018
@realcr realcr changed the title Proposal: Move the crypto primitive types to proto and stablized the proto. Safe erasure of secret data Mar 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants