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

Proposal for high-level "secret-stream" API #103

Closed
brycx opened this issue Oct 7, 2019 · 2 comments · Fixed by #108
Closed

Proposal for high-level "secret-stream" API #103

brycx opened this issue Oct 7, 2019 · 2 comments · Fixed by #108
Labels
new feature New feature or request question Further information is requested

Comments

@brycx
Copy link
Member

brycx commented Oct 7, 2019

This is a proposal for the high-level interface, providing stream-based encryption based on the hazardous implementation in #99. I find the interface provided by sodiumoxide quite nice and my proposal borrows a fair amount from there for that reason.

The streaming encryption will be a part of the orion::aead module, defining two types. One for handling stream encryption (StreamSealer) and one to handle stream decryption (StreamOpener). Nonce generation will be done automatically. Providing separate types for encryption/decryption is to make misuse harder and is also a concern @snsmac has brought up. Here is a rough implementation of what this interface may look like:

pub struct StreamSealer {
	internal_sealer: SecretStreamXChaCha20Poly1305,
	nonce: Nonce
}

impl StreamSealer {

	pub fn get_nonce(&self) -> Nonce {
		self.nonce
	}

	pub fn new(secret_key: &SecretKey) -> Result<Self, UnknownCryptoError> {
		let nonce = Nonce::generate();
		let sk = &chacha20::SecretKey::from_slice(secret_key.unprotected_as_bytes())?;
		
		Ok(Self {
			internal_sealer: SecretStreamXChaCha20Poly1305::new(&sk, &nonce),
			nonce
		})
	}

	pub fn seal_chunk(&mut self, data: &[u8], ad: Option<&[u8]>, tag: ChunkTag) -> Result<Vec<u8>, UnknownCryptoError> {
		let sealed_chunk_len = data.len().checked_add(SECRETSTREAM_XCHACHA20POLY1305_ABYTES);
		if sealed_chunk_len.is_none() {
			return Err(UnknownCryptoError);
		}
		
		let mut sealed_chunk = vec![0u8; sealed_chunk_len.unwrap()];
		self.internal_sealer.encrypt_message(data, ad, &mut sealed_chunk, tag)?;

		Ok(sealed_chunk)	
	}
}

pub struct StreamOpener {
	internal_sealer: SecretStreamXChaCha20Poly1305,
}

impl StreamOpener {

	pub fn new(secret_key: &SecretKey, nonce: &Nonce) -> Result<Self, UnknownCryptoError> {
		let sk = &chacha20::SecretKey::from_slice(secret_key.unprotected_as_bytes())?;
		
		Ok(Self {
			internal_sealer: SecretStreamXChaCha20Poly1305::new(&sk, &nonce),
		})
	}


	pub fn open_chunk(&mut self, data: &[u8], ad: Option<&[u8]>) -> Result<(Vec<u8>, ChunkTag), UnknownCryptoError> {
		if data.len() < SECRETSTREAM_XCHACHA20POLY1305_ABYTES {
			return Err(UnknownCryptoError);
		}
		
		let mut opened_chunk = vec![0u8; data.len() - SECRETSTREAM_XCHACHA20POLY1305_ABYTES];
		let tag = self.internal_sealer.decrypt_message(data, ad, &mut opened_chunk)?;

		Ok((opened_chunk, tag))
	}
}

I specifically try to move away from calling the encryption/decryption operations push or pull, as I find these to be bad names as they don't clearly communicate the purpose or functionality behind IMO. Including "seal" and "open" was to be consistent with naming of AEAD-related functions throughout the rest of orion.

Once #88 is finalised, there will be a high-level type called Tag as well as all other Tags used in hazardous. That is the reasoning behind importing the stream Tag as ChunkTag, though I don't know if there's something better to call it.

Another remaining question is whether the nonce used should be called a Header as libsodium does it, or if we should stick with Nonce. I personally find Nonce to be a more fitting name, because I find it to convey it's use more clearly.

Any feedback/input is greatly appreciated!

@brycx brycx added new feature New feature or request question Further information is requested labels Oct 7, 2019
@jorickert
Copy link
Contributor

Looks generally fine to me. Personally i also prefer "Nonce" over "Header" .
Instead of pub fn header(&self) i would name this function get_nonce.

@brycx
Copy link
Member Author

brycx commented Oct 15, 2019

Instead of pub fn header(&self) i would name this function get_nonce.

Good catch, I've updated the example. After some more consideration, I thought naming the ChunkTag as InfoTag instead would be more appropriate, to convey its functionality better. What do you think? Let's rename Tag -> StreamTag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants