From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id EE2BA1FF141 for ; Mon, 13 Apr 2026 10:44:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 40ADF1C39B; Mon, 13 Apr 2026 10:45:41 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 13 Apr 2026 10:45:34 +0200 Message-Id: Subject: Re: [PATCH proxmox 1/6] systemd: add support for machine-id generation To: =?utf-8?q?Fabian_Gr=C3=BCnbichler?= , X-Mailer: aerc 0.20.0 References: <20260410100326.3199377-1-f.gruenbichler@proxmox.com> <20260410100326.3199377-2-f.gruenbichler@proxmox.com> In-Reply-To: <20260410100326.3199377-2-f.gruenbichler@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776069860318 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.123 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [lib.rs,sys.rs] Message-ID-Hash: NW4OE6RTDSUS2WJLYHTLKOZM44Y6RCGI X-Message-ID-Hash: NW4OE6RTDSUS2WJLYHTLKOZM44Y6RCGI X-MailFrom: s.sterz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri Apr 10, 2026 at 12:02 PM CEST, Fabian Gr=C3=BCnbichler wrote: > the plain machine-id should not be leaked to external systems, but libsys= temd > provides helpers for deriving application-id based identifiers that are u= seful > for identifying a machine externally. > > Signed-off-by: Fabian Gr=C3=BCnbichler > --- > > Notes: > to be used in the next patch by proxmox-subscription > > proxmox-systemd/src/lib.rs | 2 + > proxmox-systemd/src/sd_id128.rs | 70 +++++++++++++++++++++++++++++++++ > proxmox-systemd/src/sys.rs | 6 +++ > 3 files changed, 78 insertions(+) > create mode 100644 proxmox-systemd/src/sd_id128.rs > > diff --git a/proxmox-systemd/src/lib.rs b/proxmox-systemd/src/lib.rs > index 456d88c3..f79c204c 100644 > --- a/proxmox-systemd/src/lib.rs > +++ b/proxmox-systemd/src/lib.rs > @@ -7,3 +7,5 @@ pub use escape::{escape_unit, unescape_unit, unescape_uni= t_path, UnescapeError}; > > pub mod journal; > pub mod notify; > + > +pub mod sd_id128; > diff --git a/proxmox-systemd/src/sd_id128.rs b/proxmox-systemd/src/sd_id1= 28.rs > new file mode 100644 > index 00000000..a98a6663 > --- /dev/null > +++ b/proxmox-systemd/src/sd_id128.rs > @@ -0,0 +1,70 @@ > +use std::fmt; > + > +use crate::sys::{self, sd_id128_t}; > + > +#[derive(Debug, PartialEq, Eq)] > +pub enum SystemdId128Error { > + InvalidAppId, > + GenerationError, > +} > + > +impl std::error::Error for SystemdId128Error { > + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { > + None > + } > +} > + > +impl fmt::Display for SystemdId128Error { > + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { > + match self { > + SystemdId128Error::InvalidAppId =3D> f.write_str("Provided a= pplication ID is invalid."), > + SystemdId128Error::GenerationError =3D> { > + f.write_str("Failed to generate machine-id based on appl= ication ID.") > + } > + } > + } > +} > + > +pub fn get_app_specific_id(app_id: [u8; 16]) -> Result<[u8; 16], Systemd= Id128Error> { > + let mut res =3D sd_id128_t { bytes: [0; 16] }; > + > + if app_id.iter().all(|b| *b =3D=3D 0) { > + return Err(SystemdId128Error::InvalidAppId); > + } > + unsafe { > + sys::sd_id128_get_machine_app_specific(sd_id128_t { bytes: app_i= d }, &mut res); one thought: checking the return code here could give more insights into why id generation failed, imo it may make to check that. that may also be slightly nicer than goin through the return array like you do below. > + } > + if res.bytes.iter().all(|b| *b =3D=3D 0) { > + return Err(SystemdId128Error::GenerationError); > + } > + Ok(res.bytes) this may be a little pedantic, but imo it might be nice not to drop the type information here already and return a `sd_id128_t`. it would provide some more information on the semantic meaning here. > +} > + > +#[test] > +fn test_invalid_app_id() { > + let invalid =3D [0; 16]; > + let res =3D get_app_specific_id(invalid); > + assert!(res.is_err()); > + assert_eq!(res, Err(SystemdId128Error::InvalidAppId)); > +} > + > +#[test] > +fn test_valid_app_id() { > + // no machine-id, no app-specific ID either.. > + if !std::path::Path::new("/etc/machine-id").exists() { > + return; > + } > + > + // UUID generated with `systemd-id128 new` and converted from hex > + let valid =3D 950247666410175165299169499632875718_u128.to_le_bytes(= ); > + > + let res =3D get_app_specific_id(valid); > + assert!(res.is_ok()); > + > + let res2 =3D get_app_specific_id(valid); > + assert!(res2.is_ok()); > + > + // cannot verify the expected result, since that depends on the mach= ine the test runs on > + // we can verify that two generations using the same machine and app= -id give identical results > + assert_eq!(res, res2); > +} > diff --git a/proxmox-systemd/src/sys.rs b/proxmox-systemd/src/sys.rs > index eabd44d1..ea2b6061 100644 > --- a/proxmox-systemd/src/sys.rs > +++ b/proxmox-systemd/src/sys.rs > @@ -4,6 +4,11 @@ use std::os::fd::RawFd; > > pub const LISTEN_FDS_START: RawFd =3D 3; > > +#[repr(C)] > +pub struct sd_id128_t { > + pub bytes: [u8; 16], > +} > + > #[link(name =3D "systemd")] > unsafe extern "C" { > pub fn sd_journal_stream_fd( > @@ -24,6 +29,7 @@ unsafe extern "C" { > unset_environment: c_int, > names: *mut *mut *mut c_char, > ) -> c_int; > + pub fn sd_id128_get_machine_app_specific(app_id: sd_id128_t, ret: *m= ut sd_id128_t) -> c_int; > } > > pub fn check_call(ret: c_int) -> Result {