Fix nonce incrementing in stateful transport to match the specification

[complexspaces]

This PR fixes #150 by correcting the stateful encrypt/decrypt functions to never use the reserved value of u64::MAX, which additionally guarantees a lack of wraparound issues.

In addition, a unit test was added to ensure this behavior does not regress by mistake.

Fixes #150

[mcginty]

This looks good, and given the wording of that nonce being reserved, I'm wondering if the same restriction should be placed on StatelessCipherState as it would also be using a "special" nonce, regardless of it being exhausted or not.

snow/src/cipherstate.rs

Lines 123 to 153 in b55a713

	pub fn encrypt_ad(
	&self,
	nonce: u64,
	authtext: &[u8],
	plaintext: &[u8],
	out: &mut [u8],
	) -> Result<usize, Error> {
	if !self.has_key {
	return Err(StateProblem::MissingKeyMaterial.into());
	}
	Ok(self.cipher.encrypt(nonce, authtext, plaintext, out))
	}
	pub fn decrypt_ad(
	&self,
	nonce: u64,
	authtext: &[u8],
	ciphertext: &[u8],
	out: &mut [u8],
	) -> Result<usize, Error> {
	if (ciphertext.len() < TAGLEN) || out.len() < (ciphertext.len() - TAGLEN) {
	return Err(Error::Decrypt);
	}
	if !self.has_key {
	return Err(StateProblem::MissingKeyMaterial.into());
	}
	self.cipher.decrypt(nonce, authtext, ciphertext, out)
	}

[complexspaces]

... I'm wondering if the same restriction should be placed on StatelessCipherState as it would also be using a "special" nonce

That's a valid question, and I almost had implemented that too. The specification is unclear if "reserved" means "reserved for Noise" or "reserved for a protocol to use for certain things." If its the first interpretation, we should, but in the second we might prevent an otherwise valid use case.

[mcginty]

Given the wording for 11.4. Out-of-order transport messages, I think we should cautiously treat it as abiding by the same rules as the in-order transports, if you don't mind adding that to this PR.

I asked for clarification and if I hear anything to the contrary we can update.

[mcginty]

Ah yes, MAXNONCE is used in the REKEY(k) function, so we should consider that reserved for all cases where the nonce is used.

[complexspaces]

Oh great catch! I agree we need the check everywhere then. I'll update this PR shortly.

[mcginty]

Thanks to Trevor for pointing it out :).

[complexspaces]

I have pushed the changes that now include validations for the stateless cipher function as well.

[mcginty]

Thanks, this looks great.

The only thought I had is that Exhausted is probably not a great name any more (maybe InvalidNonce with rustdocs to explain the scenario), but that's actually better for me to change separately, as I'd like to get this merged into the 0.9 branch for a correctness patch release.