From d638a95af5e3505436c262f6ca09ebfa11bfe77d Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Thu, 17 Nov 2022 13:45:34 +0100 Subject: openpgp: Allow setting a policy for specific versions of a packet. - For some packets we'd like to have different policies depending on the version. This in particular applies to Signatures: by default we want to reject v3 signatures, but accept v4 signatures. - By default, reject v3 signatures as of 2007. - Fixes: #945 --- openpgp/NEWS | 6 + openpgp/src/policy.rs | 327 ++++++++++++++++++++++++++++++++++----- openpgp/src/policy/cutofflist.rs | 313 ++++++++++++++++++++++++++++++++++++- 3 files changed, 600 insertions(+), 46 deletions(-) diff --git a/openpgp/NEWS b/openpgp/NEWS index 626e0e02..08f43f56 100644 --- a/openpgp/NEWS +++ b/openpgp/NEWS @@ -24,6 +24,12 @@ - StandardPolicy::reject_all_symmetric_algos - StandardPolicy::reject_all_aead_algos - StandardPolicy::reject_all_packet_tags + - StandardPolicy::accept_packet_tag_version + - StandardPolicy::reject_packet_tag_version + - StandardPolicy::reject_packet_tag_version_at + - StandardPolicy::packet_tag_version_cutoff +** Deprecated functionality + - StandardPolicy::packet_tag_cutoff * Changes in 1.10.0 ** New functionality - Cert::insert_packets2 diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs index 665536ad..03d36b02 100644 --- a/openpgp/src/policy.rs +++ b/openpgp/src/policy.rs @@ -60,6 +60,7 @@ use cutofflist::{ CutoffList, REJECT, ACCEPT, + VersionedCutoffList, }; /// A policy for cryptographic operations. @@ -746,30 +747,38 @@ a_cutoff_list!(AEADAlgorithmCutoffList, AEADAlgorithm, 3, ACCEPT, // 2. OCB. ]); -a_cutoff_list!(PacketTagCutoffList, Tag, 21, - [ - REJECT, // 0. Reserved. - ACCEPT, // 1. PKESK. - ACCEPT, // 2. Signature. - ACCEPT, // 3. SKESK. - ACCEPT, // 4. OnePassSig. - ACCEPT, // 5. SecretKey. - ACCEPT, // 6. PublicKey. - ACCEPT, // 7. SecretSubkey. - ACCEPT, // 8. CompressedData. - Some(Timestamp::Y2004M2), // 9. SED. - ACCEPT, // 10. Marker. - ACCEPT, // 11. Literal. - ACCEPT, // 12. Trust. - ACCEPT, // 13. UserID. - ACCEPT, // 14. PublicSubkey. - REJECT, // 15. Not assigned. - REJECT, // 16. Not assigned. - ACCEPT, // 17. UserAttribute. - ACCEPT, // 18. SEIP. - ACCEPT, // 19. MDC. - ACCEPT, // 20. AED. - ]); +a_versioned_cutoff_list!(PacketTagCutoffList, Tag, 21, + [ + REJECT, // 0. Reserved. + ACCEPT, // 1. PKESK. + ACCEPT, // 2. Signature. + ACCEPT, // 3. SKESK. + ACCEPT, // 4. OnePassSig. + ACCEPT, // 5. SecretKey. + ACCEPT, // 6. PublicKey. + ACCEPT, // 7. SecretSubkey. + ACCEPT, // 8. CompressedData. + Some(Timestamp::Y2004M2), // 9. SED. + ACCEPT, // 10. Marker. + ACCEPT, // 11. Literal. + ACCEPT, // 12. Trust. + ACCEPT, // 13. UserID. + ACCEPT, // 14. PublicSubkey. + REJECT, // 15. Not assigned. + REJECT, // 16. Not assigned. + ACCEPT, // 17. UserAttribute. + ACCEPT, // 18. SEIP. + ACCEPT, // 19. MDC. + ACCEPT, // 20. AED. + ], + // The versioned list overrides the unversioned list. So we only + // need to tweak the above. + // + // Note: this list must be sorted and the tag and version must be unique! + 1, + [ + (Tag::Signature, 3, Some(Timestamp::Y2007M2)), + ]); // We need to convert a `SystemTime` to a `Timestamp` in // `StandardPolicy::reject_hash_at`. Unfortunately, a `SystemTime` @@ -1302,14 +1311,34 @@ impl<'a> StandardPolicy<'a> { self.aead_algos.cutoff(a).map(|t| t.into()) } - /// Always accept packets with the given tag. + /// Always accept the specified version of the packet. + /// + /// If a packet does not have a version field, then its version is + /// `0`. + pub fn accept_packet_tag_version(&mut self, tag: Tag, version: u8) { + self.packet_tags.set_versioned(tag, version, ACCEPT); + } + + /// Always accept packets with the given tag independent of their + /// version. + /// + /// If you previously set a cutoff for a specific version of a + /// packet, this overrides that. pub fn accept_packet_tag(&mut self, tag: Tag) { - self.packet_tags.set(tag, ACCEPT); + self.packet_tags.set_unversioned(tag, ACCEPT); + } + + /// Always reject the specified version of the packet. + /// + /// If a packet does not have a version field, then its version is + /// `0`. + pub fn reject_packet_tag_version(&mut self, tag: Tag, version: u8) { + self.packet_tags.set_versioned(tag, version, REJECT); } /// Always reject packets with the given tag. pub fn reject_packet_tag(&mut self, tag: Tag) { - self.packet_tags.set(tag, REJECT); + self.packet_tags.set_unversioned(tag, REJECT); } /// Considers all packets to be insecure. @@ -1320,7 +1349,8 @@ impl<'a> StandardPolicy<'a> { self.packet_tags.reject_all(); } - /// Start rejecting packets with the given tag at `t`. + /// Start rejecting the specified version of packets with the + /// given tag at `t`. /// /// A cutoff of `None` means that there is no cutoff and the /// packet has no known vulnerabilities. @@ -1343,17 +1373,69 @@ impl<'a> StandardPolicy<'a> { /// /// [Debian 3.0]: https://www.debian.org/News/2002/20020719 /// [GnuPG 1.0.3]: https://lists.gnupg.org/pipermail/gnupg-announce/2000q3/000075.html + pub fn reject_packet_tag_version_at(&mut self, tag: Tag, version: u8, + cutoff: C) + where C: Into>, + { + self.packet_tags.set_versioned( + tag, version, + cutoff.into().and_then(system_time_cutoff_to_timestamp)); + } + + /// Start rejecting packets with the given tag at `t`. + /// + /// See the documentation for + /// [`StandardPolicy::reject_packet_tag_version_at`]. pub fn reject_packet_tag_at(&mut self, tag: Tag, cutoff: C) where C: Into>, { - self.packet_tags.set( + self.packet_tags.set_unversioned( tag, cutoff.into().and_then(system_time_cutoff_to_timestamp)); } - /// Returns the cutoff times for the specified hash algorithm. + /// Returns the cutoff for the specified version of the specified + /// packet tag. + /// + /// This first considers the versioned cutoff list. If there is + /// no entry in the versioned list, it fallsback to the + /// unversioned cutoff list. If there is also no entry there, + /// then it falls back to the default. + pub fn packet_tag_version_cutoff(&self, tag: Tag, version: u8) + -> Option + { + self.packet_tags.cutoff(tag, version).map(|t| t.into()) + } + + /// Returns the cutoff time for the specified packet tag. + /// + /// This function returns the maximum cutoff for all versions of + /// the packet. That is, if one version has a cutoff of `t1`, and + /// another version has a cutoff of `t2`, this returns `max(t1, + /// t2)`. These semantics answer the question: "Up to which point + /// can we use this packet?" + #[deprecated(note = "Since 1.11. Use `packet_tag_version_cutoff`.")] pub fn packet_tag_cutoff(&self, tag: Tag) -> Option { - self.packet_tags.cutoff(tag).map(|t| t.into()) + // Versioned policy. + self.packet_tags.versioned_cutoffs + .iter() + .filter_map(|(t, _v, cutoff)| { + if t == &tag { + Some(cutoff) + } else { + None + } + }) + // Unversioned policy or default, if nont. + .chain( + std::iter::once( + self.packet_tags.unversioned_cutoffs.get( + u8::from(tag) as usize) + .unwrap_or(&cutofflist::DEFAULT_POLICY))) + // Prefer None. + .max_by(|a, b| a.is_none().cmp(&b.is_none()).then(a.cmp(b))) + .expect("have one") + .map(Into::into) } } @@ -1533,7 +1615,11 @@ impl<'a> Policy for StandardPolicy<'a> { fn packet(&self, packet: &Packet) -> Result<()> { let time = self.time.unwrap_or_else(Timestamp::now); - self.packet_tags.check(packet.tag(), time, None) + self.packet_tags + .check( + packet.tag(), + packet.version().unwrap_or(0), + time, None) .context("Policy rejected packet type") } @@ -2891,11 +2977,174 @@ mod test { &[ AEADAlgorithm::OCB ], &[ AEADAlgorithm::EAX ]); - reject_all_check!(reject_all_packet_tags, - accept_packet_tag, - packet_tag_cutoff, - &[ Tag::SEIP, - Tag::Unknown(252) ], - &[ Tag::Signature, - Tag::Unknown(230) ]); + #[test] + fn reject_all_packets() -> Result<()> { + let mut p = StandardPolicy::new(); + + let set_variants = [ + (Tag::SEIP, 4), + (Tag::Unknown(252), 17), + ]; + let check_variants = [ + (Tag::Signature, 4), + (Tag::Unknown(230), 9), + ]; + + // Accept a few packets explicitly. + for (t, v) in set_variants.iter().cloned() { + p.accept_packet_tag_version(t, v); + assert_eq!( + p.packet_tag_version_cutoff(t, v), + ACCEPT.map(Into::into)); + } + + // Reject all hashes. + p.reject_all_packet_tags(); + + for (t, v) in set_variants.iter().chain(check_variants.iter()).cloned() { + assert_eq!( + p.packet_tag_version_cutoff(t, v), + REJECT.map(Into::into)); + } + + Ok(()) + } + + #[test] + fn packet_versions() -> Result<()> { + // Accept the version of a packet. Optionally make sure a + // different version is not accepted. + fn accept_and_check(p: &mut StandardPolicy, + tag: Tag, + accept_versions: &[u8], + good_versions: &[u8], + bad_versions: &[u8]) { + for v in accept_versions { + p.accept_packet_tag_version(tag, *v); + assert_eq!( + p.packet_tag_version_cutoff(tag, *v), + ACCEPT.map(Into::into)); + } + + for v in good_versions.iter() { + assert_eq!( + p.packet_tag_version_cutoff(tag, *v), + ACCEPT.map(Into::into)); + } + for v in bad_versions.iter() { + assert_eq!( + p.packet_tag_version_cutoff(tag, *v), + REJECT.map(Into::into)); + } + } + + use rand::seq::SliceRandom; + let mut rng = rand::thread_rng(); + + let mut all_versions = (0..=u8::MAX).collect::>(); + all_versions.shuffle(&mut rng); + let all_versions = &all_versions[..]; + let mut not_v5 = all_versions.iter() + .filter(|&&v| v != 5) + .cloned() + .collect::>(); + not_v5.shuffle(&mut rng); + let not_v5 = ¬_v5[..]; + + let p = &mut StandardPolicy::new(); + p.reject_all_packet_tags(); + + // First only use the versioned interfaces. + accept_and_check(p, Tag::Signature, &[3], &[], &[4, 5]); + accept_and_check(p, Tag::Signature, &[4], &[3], &[5]); + + // Only use an unversioned policy. + accept_and_check(p, Tag::SEIP, + &[], // set to accept + &[], // good + all_versions, // bad + ); + p.accept_packet_tag(Tag::SEIP); + accept_and_check(p, Tag::SEIP, + &[], // set to accept + all_versions, // good + &[], // bad + ); + + // Set an unversioned policy and then a versioned policy. + accept_and_check(p, Tag::PKESK, + &[], // set to accept + &[], // good + all_versions, // bad + ); + p.accept_packet_tag(Tag::PKESK); + accept_and_check(p, Tag::PKESK, + &[], // set to accept + &(0..u8::MAX).collect::>()[..], // good + &[], // bad + ); + p.reject_packet_tag_version(Tag::PKESK, 5); + accept_and_check(p, Tag::PKESK, + &[], // set to accept + not_v5, // good + &[5], // bad + ); + + // Set a versioned policy and then an unversioned policy. + // Make sure that the versioned policy is cleared by the + // unversioned policy. + accept_and_check(p, Tag::SKESK, + &[], // set to accept + &[], // good + all_versions, // bad + ); + p.accept_packet_tag_version(Tag::SKESK, 5); + accept_and_check(p, Tag::SKESK, + &[], // set to accept + &[5], // good + not_v5, // bad + ); + p.reject_packet_tag(Tag::SKESK); + // All versions should be bad now... + accept_and_check(p, Tag::SKESK, + &[], // set to accept + &[], // good + all_versions, // bad + ); + + Ok(()) + } + + #[test] + #[allow(deprecated)] + fn packet_tag_cutoff() { + // The semantics of packet_tag_cutoff are: max of all + // versioned cutoffs and the unversioned cutoff. + + let p = &mut StandardPolicy::new(); + p.reject_all_packet_tags(); + + assert_eq!(p.packet_tag_cutoff(Tag::Signature), + REJECT.map(Into::into)); + + p.reject_packet_tag_version_at(Tag::Signature, 5, + Timestamp::Y2007M2); + assert_eq!(p.packet_tag_cutoff(Tag::Signature), + Some(Timestamp::Y2007M2.into())); + + p.reject_packet_tag_version_at(Tag::Signature, 3, + Timestamp::Y2005M2); + assert_eq!(p.packet_tag_cutoff(Tag::Signature), + Some(Timestamp::Y2007M2.into())); + + p.reject_packet_tag_version_at(Tag::Signature, 6, + ACCEPT.map(Into::into)); + assert_eq!(p.packet_tag_cutoff(Tag::Signature), + ACCEPT.map(Into::into)); + + p.reject_packet_tag_version_at(Tag::Signature, 6, + Timestamp::Y2005M2); + assert_eq!(p.packet_tag_cutoff(Tag::Signature), + Some(Timestamp::Y2007M2.into())); + } } diff --git a/openpgp/src/policy/cutofflist.rs b/openpgp/src/policy/cutofflist.rs index d745ae7d..6bbc8eb7 100644 --- a/openpgp/src/policy/cutofflist.rs +++ b/openpgp/src/policy/cutofflist.rs @@ -1,6 +1,5 @@ use std::fmt; - -use std::ops::{Index, IndexMut}; +use std::ops::{Deref, Index, IndexMut}; use crate::{ Error, @@ -50,15 +49,37 @@ impl<'a, T> VecOrSlice<'a, T> { fn resize(&mut self, size: usize, value: T) where T: Clone { - let mut v : Vec = match self { + let v = self.as_mut(); + v.resize(size, value); + } + + pub(super) fn as_mut(&mut self) -> &mut Vec + where T: Clone + { + let v: Vec = match self { VecOrSlice::Vec(ref mut v) => std::mem::take(v), VecOrSlice::Slice(s) => s.to_vec(), - VecOrSlice::Empty() => Vec::with_capacity(size), + VecOrSlice::Empty() => Vec::new(), }; - v.resize(size, value); - *self = VecOrSlice::Vec(v); + if let VecOrSlice::Vec(ref mut v) = self { + v + } else { + unreachable!() + } + } +} + +impl<'a, T> Deref for VecOrSlice<'a, T> { + type Target = [T]; + + fn deref(&self) -> &Self::Target { + match self { + VecOrSlice::Vec(ref v) => &v[..], + VecOrSlice::Slice(s) => s, + VecOrSlice::Empty() => &[], + } } } @@ -140,7 +161,7 @@ impl CutoffList if i >= self.cutoffs.len() { // We reject by default. - self.cutoffs.resize(i + 1, DEFAULT_POLICY) + self.cutoffs.resize(i + 1, DEFAULT_POLICY); } self.cutoffs[i] = cutoff; } @@ -267,3 +288,281 @@ macro_rules! a_cutoff_list { } } } + +/// A data structure may have multiple versions. For instance, there +/// are multiple versions of packets. Each version of a given packet +/// may have different security properties. +#[derive(Debug, Clone)] +pub(super) struct VersionedCutoffList where A: 'static { + // Indexed by `A as u8`. + // + // A value of `None` means that no vulnerabilities are known. + // + // Note: we use `u64` and not `SystemTime`, because there is no + // way to construct a `SystemTime` in a `const fn`. + pub(super) unversioned_cutoffs: VecOrSlice<'static, Option>, + + // The content is: (algo, version, policy). + pub(super) versioned_cutoffs: + VecOrSlice<'static, (A, u8, Option)>, + + pub(super) _a: std::marker::PhantomData, +} + +impl Default for VersionedCutoffList { + fn default() -> Self { + Self::reject_all() + } +} + +impl VersionedCutoffList { + // Rejects all algorithms. + pub(super) const fn reject_all() -> Self { + Self { + unversioned_cutoffs: VecOrSlice::empty(), + versioned_cutoffs: VecOrSlice::empty(), + _a: std::marker::PhantomData, + } + } +} + +impl VersionedCutoffList + where u8: From, + A: fmt::Display, + A: std::clone::Clone, + A: Eq, + A: Ord, +{ + // versioned_cutoffs must be sorted and deduplicated. Make sure + // it is so. + pub(super) fn assert_sorted(&self) { + if cfg!(debug_assertions) || cfg!(test) { + for window in self.versioned_cutoffs.windows(2) { + let a = &window[0]; + let b = &window[1]; + + // Sorted, no duplicates. + assert!((&a.0, a.1) < (&b.0, b.1)); + } + } + } + + // Sets a cutoff time for version `version` of algorithm `algo`. + pub(super) fn set_versioned(&mut self, + algo: A, version: u8, + cutoff: Option) + { + self.assert_sorted(); + let cutofflist = self.versioned_cutoffs.as_mut(); + match cutofflist.binary_search_by(|(a, v, _)| { + algo.cmp(a).then(version.cmp(v)).reverse() + }) { + Ok(i) => { + // Replace. + cutofflist[i] = (algo, version, cutoff); + } + Err(i) => { + // Insert. + cutofflist.insert(i, (algo, version, cutoff)); + } + }; + self.assert_sorted(); + } + + // Sets a cutoff time for algorithm `algo`. + pub(super) fn set_unversioned(&mut self, algo: A, + cutoff: Option) + { + let i: u8 = algo.into(); + let i: usize = i.into(); + + if i >= self.unversioned_cutoffs.len() { + // We reject by default. + self.unversioned_cutoffs.resize(i + 1, DEFAULT_POLICY); + } + self.unversioned_cutoffs[i] = cutoff; + } + + // Returns the cutoff time for version `version` of algorithm `algo`. + #[inline] + pub(super) fn cutoff(&self, algo: A, version: u8) -> Option { + self.assert_sorted(); + match self.versioned_cutoffs.binary_search_by(|(a, v, _)| { + algo.cmp(a).then(version.cmp(v)).reverse() + }) { + Ok(i) => { + self.versioned_cutoffs[i].2 + } + Err(_loc) => { + // Fallback to the unversioned cutoff list. + *self.unversioned_cutoffs.get(u8::from(algo) as usize) + .unwrap_or(&DEFAULT_POLICY) + } + } + } + + // Checks whether version `version` of the algorithm `algo` is safe + // to use at time `time`. + // + // `tolerance` is added to the cutoff time. + #[inline] + pub(super) fn check(&self, a: A, version: u8, time: Timestamp, + tolerance: Option) + -> Result<()> + { + if let Some(cutoff) = self.cutoff(a.clone(), version) { + let cutoff = cutoff + .checked_add(tolerance.unwrap_or_else(|| Duration::seconds(0))) + .unwrap_or(Timestamp::MAX); + if time >= cutoff { + Err(Error::PolicyViolation( + a.to_string(), Some(cutoff.into())).into()) + } else { + Ok(()) + } + } else { + // None => always secure. + Ok(()) + } + } +} + +macro_rules! a_versioned_cutoff_list { + ($name:ident, $algo:ty, + // A slice indexed by the algorithm. + $unversioned_values_count: expr, $unversioned_values: expr, + // A slice of the form: [ (algo, version, cutoff), ... ] + // + // Note: the values must be sorted and (algo, version) must be + // unique! + $versioned_values_count:expr, $versioned_values:expr) => { + // It would be nicer to just have a `CutoffList` and store the + // default as a `VecOrSlice::Slice`. Unfortunately, we can't + // create a slice in a `const fn`, so that doesn't work. + // + // To work around that issue, we store the array in the + // wrapper type, and remember if we are using it or a custom + // version. + #[derive(Debug, Clone)] + enum $name { + Default(), + Custom(VersionedCutoffList<$algo>), + } + + impl std::ops::Deref for $name { + type Target = VersionedCutoffList<$algo>; + + fn deref(&self) -> &Self::Target { + match self { + $name::Default() => &Self::DEFAULT, + $name::Custom(l) => l, + } + } + } + + #[allow(unused)] + impl $name { + const VERSIONED_DEFAULTS: + [ ($algo, u8, Option); $versioned_values_count ] + = $versioned_values; + const UNVERSIONED_DEFAULTS: + [ Option; $unversioned_values_count ] + = $unversioned_values; + + const DEFAULT: VersionedCutoffList<$algo> = VersionedCutoffList { + versioned_cutoffs: + crate::policy::cutofflist::VecOrSlice::Slice( + &Self::VERSIONED_DEFAULTS), + unversioned_cutoffs: + crate::policy::cutofflist::VecOrSlice::Slice( + &Self::UNVERSIONED_DEFAULTS), + _a: std::marker::PhantomData, + }; + + // Turn the `Foo::Default` into a `Foo::Custom`, if + // necessary, to allow modification. + fn force(&mut self) -> &mut VersionedCutoffList<$algo> { + use crate::policy::cutofflist::VecOrSlice; + + if let $name::Default() = self { + *self = Self::Custom($name::DEFAULT); + } + + match self { + $name::Custom(ref mut l) => l, + _ => unreachable!(), + } + } + + // Set the cutoff for the specified version of the + // specified algorithm. + fn set_versioned(&mut self, algo: $algo, version: u8, + cutoff: Option) + { + self.force().set_versioned(algo, version, cutoff) + } + + // Sets the cutoff for the specified algorithm independent + // of its version. + fn set_unversioned(&mut self, algo: $algo, + cutoff: Option) + { + // Clear any versioned cutoffs. + let l = self.force(); + l.versioned_cutoffs.as_mut().retain(|(a, _v, _c)| { + &algo != a + }); + + l.set_unversioned(algo, cutoff) + } + + // Resets the cutoff list to its defaults. + fn defaults(&mut self) { + *self = Self::Default(); + } + + // Causes the cutoff list to reject everything. + fn reject_all(&mut self) { + *self = Self::Custom(VersionedCutoffList::reject_all()); + } + + // Returns the cutoff for the specified version of the + // specified algorithm. + // + // This first considers the versioned cutoff list. If + // there is no entry in the versioned list, it fallsback + // to the unversioned cutoff list. If there is also no + // entry there, then it falls back to the default. + fn cutoff(&self, algo: $algo, version: u8) -> Option { + let cutofflist = if let $name::Custom(ref l) = self { + l + } else { + &Self::DEFAULT + }; + + cutofflist.cutoff(algo, version) + } + + fn check(&self, algo: $algo, version: u8, + time: Timestamp, d: Option) + -> Result<()> + { + let cutofflist = if let $name::Custom(ref l) = self { + l + } else { + &Self::DEFAULT + }; + + cutofflist.check(algo, version, time, d) + } + } + + // Make sure VERSIONED_DEFAULTS is sorted and the keys are + // unique. + #[test] + #[allow(non_snake_case)] + fn $name() { + $name::DEFAULT.assert_sorted(); + } + } +} -- cgit v1.2.3