From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 169A7955F2 for ; Mon, 26 Feb 2024 21:29:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E81D514A27 for ; Mon, 26 Feb 2024 21:29:19 +0100 (CET) Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 26 Feb 2024 21:29:18 +0100 (CET) Received: by mail-qt1-x82d.google.com with SMTP id d75a77b69052e-42a4516ec5dso29158491cf.3 for ; Mon, 26 Feb 2024 12:29:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1708979351; x=1709584151; darn=lists.proxmox.com; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=/sbZkJ8k1SCCKuEzOwW0p1lURJFPy/rzhJHCqInrUJs=; b=ZyWrgh5ufe2PmqGdH0HfE/oQjxRTbP2D+0CJV5uMVhLt+wFxtxX67NZgv0SWpBJiuV TTHOiT0DSSLhtKWSPj9BXpT1HtQzHxZqmA3nishfbpAajS1J6RWmxLzt+E28C1hMUVFq 6oJyUaR07x9zX0KasXk7OZ/GUOUeEY+Jypj7mumsCUYAaj1dS637PrHUChW6a70/wUc4 EBo6bqJY+qLes7AcFC6SCFoHF5pZDvvDYcXOpcgHews53DKhSoENZveKCP5zLmxSFz/L vKGI3Tls2OhUfO4WEA7Q/2UySbm8olC5WA9fmfZOZDKVbV7oa1xnCVAK6YCbksxEzhnW Y6Hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708979351; x=1709584151; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/sbZkJ8k1SCCKuEzOwW0p1lURJFPy/rzhJHCqInrUJs=; b=uEoO3osix3aI+twzWfHQkAY6rE59N8qlcn4LqoY0+A9hHnJXAn23LH6+F5j5YsApxQ m0hxdFO9eCs6Ur5Z1/MCDANhwiXveHCoSBc7THmKEpI5DG2g4F+Ewqo497YI9rN8Y7F+ YrVsk1RlifYYAFBWodZZ3wKMujAKLA+EMi8MNHvss/s8GNm/x/RsxetTNi9h+1/HQZoW IYoBe3j0eYBm4THfim4UmoyINISkUhsbPu0OZLNdmNwe5Npg88QaKr+X8+Jmsp7GYT4N DQ9h5YKBeKFzKe8wIoX+nbP6k0sRR2W0FYGrdGyw82lq2hT93g4/b3nWSv8qfpxFbuwe KnMg== X-Gm-Message-State: AOJu0YxajsA4cHYNkbFocW6rvKmlAjIBVWAiKv1Lnp9K3/j9JvmwncAr t+svFT+24Pwk7bEhtwWsNcuTLPIGsMdBfEgufjcDZ+1ypehFhL8V7/Qxbqc7Tdc= X-Google-Smtp-Source: AGHT+IFKVTiObxZGc3FQAXuUVCaUjpBKTFG3jXwr9o96Kl/7MfwhOTcw23YKt2XNZvnFxBgOqgiwZQ== X-Received: by 2002:a05:6870:35d4:b0:21e:fb57:bcb9 with SMTP id c20-20020a05687035d400b0021efb57bcb9mr7800571oak.15.1708978966314; Mon, 26 Feb 2024 12:22:46 -0800 (PST) Received: from tema539532 ([2001:19f0:5c01:d22:5400:4ff:fe93:4e4a]) by smtp.gmail.com with ESMTPSA id wg8-20020a05620a568800b00787c6ed9a68sm2462293qkn.91.2024.02.26.12.22.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Feb 2024 12:22:46 -0800 (PST) Sender: Esi Y Date: Mon, 26 Feb 2024 20:22:44 +0000 From: Esi Y To: pbs-devel@lists.proxmox.com Message-ID: References: <20240215152001.269490-1-s.sterz@proxmox.com> <20240215152001.269490-2-s.sterz@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240215152001.269490-2-s.sterz@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.075 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy FREEMAIL_FROM 0.001 Sender email is commonly abused enduser mail provider RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox 01/12] auth-api: move signing into the private key X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 Feb 2024 20:29:50 -0000 This might be a bit nitpicky, but wouldn't this exactly be an opportunity to properly introduce traits, so that in the future on the same kind of swapout, it is more streamlined and less regression prone going forward? Just wondering ... if there was a reason not to, on such a rewrite. On Thu, Feb 15, 2024 at 04:19:50PM +0100, Stefan Sterz wrote: > this commit moves the current ticket signing code into the private key > implementation. the upside is that the caller does not need to deal > with openssl's `Signer` directly. it also simplifies and unifies the > code by using the same helper for verifying a signature and creating it. > > also derive `Clone` on `PrivateKey` and `PublicKey`. as they are > essentially thin wrappers around `openssl::pkey::PKey` and > `openssl::pkey::PKey`, which can be cloned, deriving `Clone` > just makes them easier to use. > > Signed-off-by: Stefan Sterz > --- > proxmox-auth-api/src/auth_key.rs | 26 ++++++++++++++++---------- > proxmox-auth-api/src/ticket.rs | 21 ++------------------- > 2 files changed, 18 insertions(+), 29 deletions(-) > > diff --git a/proxmox-auth-api/src/auth_key.rs b/proxmox-auth-api/src/auth_key.rs > index cec7360..32120a3 100644 > --- a/proxmox-auth-api/src/auth_key.rs > +++ b/proxmox-auth-api/src/auth_key.rs > @@ -9,11 +9,13 @@ use openssl::rsa::Rsa; > use openssl::sign::{Signer, Verifier}; > > /// A private auth key used for API ticket signing and verification. > +#[derive(Clone)] > pub struct PrivateKey { > pub(crate) key: PKey, > } > > /// A private auth key used for API ticket verification. > +#[derive(Clone)] > pub struct PublicKey { > pub(crate) key: PKey, > } > @@ -88,6 +90,13 @@ impl PrivateKey { > pub fn public_key(&self) -> Result { > PublicKey::from_pem(&self.public_key_to_pem()?) > } > + > + pub(self) fn sign(&self, digest: MessageDigest, data: &[u8]) -> Result, Error> { > + Signer::new(digest, &self.key) > + .map_err(|e| format_err!("could not create private key signer - {e}"))? > + .sign_oneshot_to_vec(data) > + .map_err(|e| format_err!("could not sign with private key - {e}")) > + } > } > > impl From> for PrivateKey { > @@ -204,15 +213,12 @@ impl Keyring { > Ok(false) > } > > - pub(crate) fn signer(&self, digest: MessageDigest) -> Result { > - Signer::new( > - digest, > - &self > - .signing_key > - .as_ref() > - .ok_or_else(|| format_err!("no private key available for signing"))? > - .key, > - ) > - .map_err(|err| format_err!("failed to create openssl signer - {err}")) > + pub(crate) fn sign(&self, digest: MessageDigest, data: &[u8]) -> Result, Error> { > + let key = self > + .signing_key > + .as_ref() > + .ok_or_else(|| format_err!("no private key available for signing"))?; > + > + key.sign(digest, data) > } > } > diff --git a/proxmox-auth-api/src/ticket.rs b/proxmox-auth-api/src/ticket.rs > index 1737912..81054f8 100644 > --- a/proxmox-auth-api/src/ticket.rs > +++ b/proxmox-auth-api/src/ticket.rs > @@ -116,27 +116,10 @@ where > /// Sign the ticket. > pub fn sign(&mut self, keyring: &Keyring, aad: Option<&str>) -> Result { > let mut output = self.ticket_data(); > - let mut signer = keyring.signer(MessageDigest::sha256())?; > - > - signer > - .update(output.as_bytes()) > - .map_err(Error::from) > - .and_then(|()| { > - if let Some(aad) = aad { > - signer > - .update(b":") > - .and_then(|()| signer.update(aad.as_bytes())) > - .map_err(Error::from) > - } else { > - Ok::<_, Error>(()) > - } > - }) > + let signature = keyring > + .sign(MessageDigest::sha256(), &self.verification_data(aad)) > .map_err(|err| format_err!("error signing ticket: {}", err))?; > > - let signature = signer > - .sign_to_vec() > - .map_err(|err| format_err!("error finishing ticket signature: {}", err))?; > - > use std::fmt::Write; > write!( > &mut output, > -- > 2.39.2 > > >