From 38492bde837338be7f5bcc3b5e6581271c0b4ba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= Date: Mon, 27 Jun 2022 12:01:31 +0200 Subject: [PATCH] check signature when reading subscription MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit and handle signed keys differently w.r.t. age checks, since they will be refreshed less frequently. Signed-off-by: Fabian Grünbichler --- proxmox-subscription/src/files.rs | 13 ++- proxmox-subscription/src/subscription_info.rs | 100 ++++++++++++++---- 2 files changed, 88 insertions(+), 25 deletions(-) diff --git a/proxmox-subscription/src/files.rs b/proxmox-subscription/src/files.rs index 6adc4ad7..25fba547 100644 --- a/proxmox-subscription/src/files.rs +++ b/proxmox-subscription/src/files.rs @@ -81,11 +81,20 @@ fn parse_subscription_file(raw: &str) -> Result, Error> /// - encoded data /// /// Legacy variants of this format as used by older versions of PVE/PMG are supported. -pub fn read_subscription>(path: P) -> Result, Error> { +pub fn read_subscription>( + path: P, + signature_key: &openssl::pkey::PKey, +) -> Result, Error> { match proxmox_sys::fs::file_read_optional_string(path)? { Some(raw) => { let mut info = parse_subscription_file(&raw)?; - info.as_mut().map(|info| info.check_age(false)); + if let Some(info) = info.as_mut() { + // these will set `status` to INVALID if checks fail! + info.check_signature(signature_key); + info.check_server_id(); + info.check_age(false); + }; + Ok(info) } None => Ok(None), diff --git a/proxmox-subscription/src/subscription_info.rs b/proxmox-subscription/src/subscription_info.rs index 707554ba..32a2f11c 100644 --- a/proxmox-subscription/src/subscription_info.rs +++ b/proxmox-subscription/src/subscription_info.rs @@ -1,8 +1,6 @@ use anyhow::{bail, format_err, Error}; -use openssl::{ - hash::{hash, DigestBytes, MessageDigest}, - pkey::Public, -}; +use openssl::hash::{hash, DigestBytes, MessageDigest}; +use proxmox_time::TmEditor; use serde::{Deserialize, Serialize}; #[cfg(feature = "api-types")] @@ -14,6 +12,7 @@ pub(crate) const SHARED_KEY_DATA: &str = "kjfdlskfhiuewhfk947368"; /// How long the local key is valid for in between remote checks pub(crate) const SUBSCRIPTION_MAX_LOCAL_KEY_AGE: i64 = 15 * 24 * 3600; +pub(crate) const SUBSCRIPTION_MAX_LOCAL_SIGNED_KEY_AGE: i64 = 365 * 24 * 3600; pub(crate) const SUBSCRIPTION_MAX_KEY_CHECK_FAILURE_AGE: i64 = 5 * 24 * 3600; // Aliases are needed for PVE compat! @@ -116,28 +115,12 @@ impl SubscriptionInfo { /// Whether a signature exists - *this does not check the signature's validity!* /// - /// Use [SubscriptionInfo::verify()] to verify the signature. + /// Use [SubscriptionInfo::check_signature()] to verify the + /// signature. pub fn is_signed(&self) -> bool { self.signature.is_some() } - /// Verify signature, if `self` is signed. Returns `false` if `self` is unsigned, `true` if signature is valid for `key`. - pub fn verify(&self, key: &openssl::pkey::PKey) -> Result { - if self.signature.is_none() { - return Ok(false); - } - let (signed, signature) = self.signed_data()?; - let signature = match signature { - None => bail!("Failed to extract signature value."), - Some(sig) => sig, - }; - - match key.verify(&signed, &signature) { - Ok(()) => Ok(true), - Err(err) => Err(format_err!("Signature verification failed - {err}")), - } - } - /// Checks whether a [SubscriptionInfo]'s `checktime` matches the age criteria: /// /// - Instances generated (more than 1.5h) in the future are invalid @@ -152,7 +135,9 @@ impl SubscriptionInfo { let now = proxmox_time::epoch_i64(); let age = now - self.checktime.unwrap_or(0); - let cutoff = if recheck { + let cutoff = if self.is_signed() { + SUBSCRIPTION_MAX_LOCAL_SIGNED_KEY_AGE + } else if recheck { SUBSCRIPTION_MAX_KEY_CHECK_FAILURE_AGE } else { SUBSCRIPTION_MAX_LOCAL_KEY_AGE + SUBSCRIPTION_MAX_KEY_CHECK_FAILURE_AGE @@ -170,6 +155,24 @@ impl SubscriptionInfo { self.signature = None; } } + + if self.is_signed() && self.status == SubscriptionStatus::ACTIVE { + if let Some(next_due) = self.nextduedate.as_ref() { + match parse_next_due(next_due.as_str()) { + Ok(next_due) if now > next_due => { + self.status = SubscriptionStatus::INVALID; + self.message = Some("subscription information too old".to_string()); + self.signature = None; + } + Ok(_) => {} + Err(err) => { + self.status = SubscriptionStatus::INVALID; + self.message = Some(format!("Failed parsing 'nextduedate' - {err}")); + self.signature = None; + } + } + } + } } /// Check that server ID contained in [SubscriptionInfo] matches that of current system. @@ -196,6 +199,30 @@ impl SubscriptionInfo { (Some(_), Ok(_)) => {} } } + + /// Check a [SubscriptionInfo]'s signature, if one is available. + /// + /// `status` is set to [SubscriptionStatus::INVALID] and `message` to a human-readable error + /// message in case a signature is available but not valid for the given `key`. + pub fn check_signature(&mut self, key: &openssl::pkey::PKey) { + let verify = |info: &SubscriptionInfo| -> Result<(), Error> { + let (signed, signature) = info.signed_data()?; + let signature = match signature { + None => bail!("Failed to extract signature value."), + Some(sig) => sig, + }; + + key.verify(&signed, &signature) + .map_err(|err| format_err!("Signature verification failed - {err}")) + }; + + if self.is_signed() { + if let Err(err) = verify(&self) { + self.status = SubscriptionStatus::INVALID; + self.message = Some(format!("Signature validation failed - {err}")); + } + } + } } /// Shortcut for md5 sums. @@ -213,3 +240,30 @@ pub fn get_hardware_address() -> Result { Ok(hex::encode(&digest).to_uppercase()) } + +fn parse_next_due(value: &str) -> Result { + let mut components = value.split("-"); + let year = components + .next() + .ok_or_else(|| format_err!("missing year component."))? + .parse::()?; + let month = components + .next() + .ok_or_else(|| format_err!("missing month component."))? + .parse::()?; + let day = components + .next() + .ok_or_else(|| format_err!("missing day component."))? + .parse::()?; + + if components.next().is_some() { + bail!("cannot parse 'nextduedate' value '{value}'"); + } + + let mut tm = TmEditor::new(true); + tm.set_year(year)?; + tm.set_mon(month)?; + tm.set_mday(day)?; + + tm.into_epoch() +}