fix(nix-compat/narinfo/signature): validate name field

We should restrict this to alphanumeric mostly, and we definitely don't
want newlines.

Not entirely sure about the exact additionally allowed characters
outside of alphanumeric, but this can always be extended further.

Change-Id: I1357e79e553f2df2fa97792889f63f0f35d50ed5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10147
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This commit is contained in:
Florian Klink 2023-11-27 16:49:48 +02:00 committed by clbot
parent b7de931cc6
commit 0415bc6fd2

View file

@ -5,7 +5,6 @@ use ed25519_dalek::SIGNATURE_LENGTH;
#[derive(Debug)] #[derive(Debug)]
pub struct Signature<'a> { pub struct Signature<'a> {
/// TODO(edef): be stricter with signature names here, they especially shouldn't have newlines!
name: &'a str, name: &'a str,
bytes: [u8; SIGNATURE_LENGTH], bytes: [u8; SIGNATURE_LENGTH],
} }
@ -20,6 +19,14 @@ impl<'a> Signature<'a> {
.split_once(':') .split_once(':')
.ok_or(SignatureError::MissingSeparator)?; .ok_or(SignatureError::MissingSeparator)?;
if name.is_empty()
|| !name
.chars()
.all(|c| char::is_alphanumeric(c) || c == '-' || c == '.')
{
return Err(SignatureError::InvalidName(name.to_string()));
}
if bytes64.len() != BASE64.encode_len(SIGNATURE_LENGTH) { if bytes64.len() != BASE64.encode_len(SIGNATURE_LENGTH) {
return Err(SignatureError::InvalidSignatureLen(bytes64.len())); return Err(SignatureError::InvalidSignatureLen(bytes64.len()));
} }
@ -54,6 +61,8 @@ impl<'a> Signature<'a> {
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
pub enum SignatureError { pub enum SignatureError {
#[error("Invalid name: {0}")]
InvalidName(String),
#[error("Missing separator")] #[error("Missing separator")]
MissingSeparator, MissingSeparator,
#[error("Invalid signature len: (expected {} b64-encoded, got {}", BASE64.encode_len(SIGNATURE_LENGTH), .0)] #[error("Invalid signature len: (expected {} b64-encoded, got {}", BASE64.encode_len(SIGNATURE_LENGTH), .0)]
@ -114,7 +123,11 @@ mod test {
assert_eq!(expect_valid, sig.verify(fp.as_bytes(), verifying_key)); assert_eq!(expect_valid, sig.verify(fp.as_bytes(), verifying_key));
} }
#[test_case("cache.nixos.org-1:o1DTsjCz0PofLJ216P2RBuSulI8BAb6zHxWE4N+tzlcELk5Uk/GO2SCxWTRN5wJutLZZ+cHTMdWqOHF8"; "wrong_length")] #[test_case("cache.nixos.org-1:o1DTsjCz0PofLJ216P2RBuSulI8BAb6zHxWE4N+tzlcELk5Uk/GO2SCxWTRN5wJutLZZ+cHTMdWqOHF8"; "wrong length")]
#[test_case("test\n:u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "wrong name newline")]
#[test_case("test :u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "wrong name space")]
#[test_case(":u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "empty name")]
#[test_case("u01BybwQhyI5H1bW1EIWXssMDhDDIvXOG5uh8Qzgdyjz6U1qg6DHhMAvXZOUStIj6X5t4/ufFgR8i3fjf0bMAw=="; "b64 only")]
fn parse_fail(input: &'static str) { fn parse_fail(input: &'static str) {
Signature::parse(input).expect_err("must fail"); Signature::parse(input).expect_err("must fail");
} }