From 477dc4c4a6c3d89ad67a53e91420e4f19004fe20 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 18 Nov 2025 21:46:30 +0100 Subject: [PATCH 1/7] initial implementation of security callbacks --- rust/agama-software/src/callbacks.rs | 1 + .../src/callbacks/commit_download.rs | 1 + rust/agama-software/src/callbacks/security.rs | 273 ++++++++++++++ rust/agama-software/src/model.rs | 1 + rust/agama-software/src/service.rs | 4 +- rust/agama-software/src/zypp_server.rs | 18 +- rust/zypp-agama/src/callbacks.rs | 1 + rust/zypp-agama/src/callbacks/security.rs | 336 ++++++++++++++++++ rust/zypp-agama/src/lib.rs | 23 +- .../zypp-agama-sys/c-layer/callbacks.cxx | 123 +++++++ .../c-layer/include/callbacks.h | 114 ++++++ .../zypp-agama-sys/c-layer/include/lib.h | 4 +- .../c-layer/include/repository.h | 6 +- .../c-layer/internal/callbacks.hxx | 5 + .../zypp-agama/zypp-agama-sys/c-layer/lib.cxx | 12 +- .../zypp-agama/zypp-agama-sys/src/bindings.rs | 141 +++++++- 16 files changed, 1039 insertions(+), 24 deletions(-) create mode 100644 rust/agama-software/src/callbacks/security.rs create mode 100644 rust/zypp-agama/src/callbacks/security.rs diff --git a/rust/agama-software/src/callbacks.rs b/rust/agama-software/src/callbacks.rs index 0d7e40dfb1..f693aa98e8 100644 --- a/rust/agama-software/src/callbacks.rs +++ b/rust/agama-software/src/callbacks.rs @@ -1 +1,2 @@ pub mod commit_download; +pub mod security; diff --git a/rust/agama-software/src/callbacks/commit_download.rs b/rust/agama-software/src/callbacks/commit_download.rs index d2140231b2..65f546982e 100644 --- a/rust/agama-software/src/callbacks/commit_download.rs +++ b/rust/agama-software/src/callbacks/commit_download.rs @@ -34,6 +34,7 @@ impl Callback for CommitDownload { description: &str, ) -> zypp_agama::callbacks::ProblemResponse { // TODO: make it generic for any problemResponse questions + // TODO: we need support for abort and make it default action let labels = [gettext("Retry"), gettext("Ignore")]; let actions = [ ("Retry", labels[0].as_str()), diff --git a/rust/agama-software/src/callbacks/security.rs b/rust/agama-software/src/callbacks/security.rs new file mode 100644 index 0000000000..b0493672ae --- /dev/null +++ b/rust/agama-software/src/callbacks/security.rs @@ -0,0 +1,273 @@ +use agama_utils::{actor::Handler, api::question::QuestionSpec, question}; +use gettextrs::gettext; +use tokio::runtime::Handle; +use zypp_agama::callbacks::security; + +#[derive(Clone)] +pub struct Security { + questions: Handler, +} + +impl Security { + pub fn new( + questions: Handler, + ) -> Self { + Self { + questions, + } + } +} + +impl security::Callback for Security { + fn unsigned_file(&self, file: String, repository_alias: String) -> bool { + // TODO: support for extra_repositories with allow_unsigned config + // TODO: localization for text when parameters in gextext will be solved + let text = if repository_alias.is_empty() { + format!("The file {file} is not digitally signed. The origin \ + and integrity of the file cannot be verified. Use it anyway?") + } else { + format!("The file {file} from {repository_alias} is not digitally signed. The origin \ + and integrity of the file cannot be verified. Use it anyway?") + }; + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let data = [("filename", file.as_str())]; + let question = QuestionSpec::new(&text, "software.unsigned_file") + .with_actions(&actions) + .with_data(&data) + .with_default_action("No"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } + + fn accept_key( + &self, + key_id: String, + key_name: String, + key_fingerprint: String, + _repository_alias: String, + ) -> security::GpgKeyTrust { + // TODO: support for extra_repositories with specified gpg key checksum + // TODO: localization with params + let text = format!( + "The key {key_id} ({key_name}) with fingerprint {key_fingerprint} is unknown. \ + Do you want to trust this key?" + ); + let labels = [gettext("Trust"), gettext("Skip")]; + let actions = [ + ("Trust", labels[0].as_str()), + ("Skip", labels[1].as_str()), + ]; + let data = [("id", key_id.as_str()), ("name", key_name.as_str()), ("fingerprint", key_fingerprint.as_str())]; + let question = QuestionSpec::new(&text, "software.import_gpg") + .with_actions(&actions) + .with_data(&data) + .with_default_action("Skip"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return security::GpgKeyTrust::Reject; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return security::GpgKeyTrust::Reject; + }; + + answer_str + .action + .as_str() + .parse::() + .unwrap_or(security::GpgKeyTrust::Reject) + } + + fn unknown_key(&self, file: String, key_id: String, repository_alias: String) -> bool { + // TODO: localization for text when parameters in gextext will be solved + let text = if repository_alias.is_empty() { + format!("The file {file} is digitally signed with \ + the following unknown GnuPG key: {key_id}. Use it anyway?") + } else { + format!("The file {file} from {repository_alias} is digitally signed with \ + the following unknown GnuPG key: {key_id}. Use it anyway?") + }; + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let data = [("filename", file.as_str()), ("id", key_id.as_str())]; + let question = QuestionSpec::new(&text, "software.unknown_gpg") + .with_actions(&actions) + .with_data(&data) + .with_default_action("No"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } + + fn verification_failed( + &self, + file: String, + key_id: String, + key_name: String, + key_fingerprint: String, + repository_alias: String, + ) -> bool { + // TODO: localization for text when parameters in gextext will be solved + let text = if repository_alias.is_empty() { + format!("The file {file} is digitally signed with the \ + following GnuPG key, but the integrity check failed: {key_id} ({key_name}). \ + Use it anyway?") + } else { + // TODO: Originally it uses repository url and not alias. Does it matter? + format!("The file {file} from {repository_alias} is digitally signed with the \ + following GnuPG key, but the integrity check failed: {key_id} ({key_name}). \ + Use it anyway?") + }; + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let data = [("filename", file.as_str())]; + let question = QuestionSpec::new(&text, "software.verification_failed") + .with_actions(&actions) + .with_data(&data) + .with_default_action("No"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } + + fn checksum_missing(&self, file: String) -> bool { + // TODO: localization for text when parameters in gextext will be solved + let text = format!("No checksum for the file {file} was found in the repository. This means that \ + although the file is part of the signed repository, the list of checksums \ + does not mention this file. Use it anyway?"); + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let question = QuestionSpec::new(&text, "software.digest.no_digest") + .with_actions(&actions) + .with_default_action("Yes"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } + + fn checksum_unknown(&self, file: String, checksum: String) -> bool { + let text = format!("The checksum of the file {file} is \"{checksum}\" but the expected checksum is \ + unknown. This means that the origin and integrity of the file cannot be verified. \ + Use it anyway?"); + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let question = QuestionSpec::new(&text, "software.digest.unknown_digest") + .with_actions(&actions) + .with_default_action("Yes"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } + + fn checksum_wrong(&self, file: String, expected: String, actual: String) -> bool { + let text = format!("The expected checksum of file %{file} is \"%{actual}\" but it was expected to be \ + \"%{expected}\". The file has changed by accident or by an attacker since the \ + creater signed it. Use it anyway?"); + let labels = [gettext("Yes"), gettext("No")]; + let actions = [ + ("Yes", labels[0].as_str()), + ("No", labels[1].as_str()), + ]; + let question = QuestionSpec::new(&text, "software.digest.unknown_digest") + .with_actions(&actions) + .with_default_action("Yes"); + let result = Handle::current().block_on(async move { + self.questions + .call(question::message::Ask::new(question)) + .await + }); + let Ok(answer) = result else { + tracing::warn!("Failed to ask question {:?}", result); + return false; + }; + let Some(answer_str) = answer.answer else { + tracing::warn!("No answer provided"); + return false; + }; + + answer_str.action == "Yes" + } +} + diff --git a/rust/agama-software/src/model.rs b/rust/agama-software/src/model.rs index cb70f11d52..f7d401a618 100644 --- a/rust/agama-software/src/model.rs +++ b/rust/agama-software/src/model.rs @@ -135,6 +135,7 @@ impl ModelAdapter for Model { self.zypp_sender.send(SoftwareAction::Write { state: software, progress, + question: self.question.clone(), tx, })?; Ok(rx.await??) diff --git a/rust/agama-software/src/service.rs b/rust/agama-software/src/service.rs index 33593c5708..95304aa88e 100644 --- a/rust/agama-software/src/service.rs +++ b/rust/agama-software/src/service.rs @@ -28,9 +28,7 @@ use crate::{ use agama_utils::{ actor::{self, Actor, Handler, MessageHandler}, api::{ - event::{self, Event}, - software::{Config, Proposal, Repository, SoftwareProposal, SystemInfo}, - Issue, IssueSeverity, Scope, + Issue, IssueSeverity, Scope, event::{self, Event}, software::{Config, Proposal, Repository, SoftwareProposal, SystemInfo} }, issue, products::ProductSpec, diff --git a/rust/agama-software/src/zypp_server.rs b/rust/agama-software/src/zypp_server.rs index 0f56dc06ed..3fd2c7cf2b 100644 --- a/rust/agama-software/src/zypp_server.rs +++ b/rust/agama-software/src/zypp_server.rs @@ -35,7 +35,7 @@ use tokio::sync::{ use zypp_agama::ZyppError; use crate::{ - callbacks::commit_download, + callbacks::{commit_download, security}, model::state::{self, SoftwareState}, }; @@ -104,6 +104,7 @@ pub enum SoftwareAction { Write { state: SoftwareState, progress: Handler, + question: Handler, tx: oneshot::Sender>>, }, } @@ -173,16 +174,19 @@ impl ZyppServer { SoftwareAction::Write { state, progress, + question, tx, } => { - self.write(state, progress, tx, zypp).await?; + let mut security_callback = security::Security::new(question); + self.write(state, progress, &mut security_callback, tx, zypp).await?; } SoftwareAction::GetPatternsMetadata(names, tx) => { self.get_patterns(names, tx, zypp).await?; } SoftwareAction::Install(tx, progress, question) => { - let callback = commit_download::CommitDownload::new(progress, question); - tx.send(self.install(zypp, &callback)) + let download_callback = commit_download::CommitDownload::new(progress, question.clone()); + let mut security_callback = security::Security::new(question); + tx.send(self.install(zypp, &download_callback, &mut security_callback)) .map_err(|_| ZyppDispatchError::ResponseChannelClosed)?; } SoftwareAction::Finish(tx) => { @@ -200,11 +204,12 @@ impl ZyppServer { &self, zypp: &zypp_agama::Zypp, download_callback: &commit_download::CommitDownload, + security_callback: &mut security::Security, ) -> ZyppServerResult { let target = "/mnt"; zypp.switch_target(target)?; // TODO: write real install callbacks beside download ones - let result = zypp.commit(download_callback)?; + let result = zypp.commit(download_callback, security_callback)?; tracing::info!("libzypp commit ends with {}", result); Ok(result) } @@ -235,6 +240,7 @@ impl ZyppServer { &self, state: SoftwareState, progress: Handler, + security: &mut security::Security, tx: oneshot::Sender>>, zypp: &zypp_agama::Zypp, ) -> Result<(), ZyppDispatchError> { @@ -310,7 +316,7 @@ impl ZyppServer { let result = zypp.load_source(|percent, alias| { tracing::info!("Refreshing repositories: {} ({}%)", alias, percent); true - }); + }, security); if let Err(error) = result { let message = format!("Could not read the repositories"); diff --git a/rust/zypp-agama/src/callbacks.rs b/rust/zypp-agama/src/callbacks.rs index a773425dc3..fb982ecba7 100644 --- a/rust/zypp-agama/src/callbacks.rs +++ b/rust/zypp-agama/src/callbacks.rs @@ -2,6 +2,7 @@ use std::{fmt, str::FromStr}; pub mod download_progress; pub mod pkg_download; +pub mod security; // empty progress callback pub fn empty_progress(_value: i64, _text: String) -> bool { diff --git a/rust/zypp-agama/src/callbacks/security.rs b/rust/zypp-agama/src/callbacks/security.rs new file mode 100644 index 0000000000..0786147073 --- /dev/null +++ b/rust/zypp-agama/src/callbacks/security.rs @@ -0,0 +1,336 @@ +use std::{os::raw::{c_char, c_void}, str::FromStr}; + +use crate::helpers::{as_c_void, string_from_ptr}; + +/// Represents the decision on how to handle an unknown GPG key. +#[derive(Debug, PartialEq, Eq)] +pub enum GpgKeyTrust { + /// Reject the key. + Reject, + /// Trust the key temporarily for the current session. + Temporary, + /// Import and trust the key permanently. + Import, +} + +impl From for zypp_agama_sys::GPGKeyTrust { + fn from(val: GpgKeyTrust) -> Self { + match val { + GpgKeyTrust::Reject => zypp_agama_sys::GPGKeyTrust_GPGKT_REJECT, + GpgKeyTrust::Temporary => zypp_agama_sys::GPGKeyTrust_GPGKT_TEMPORARY, + GpgKeyTrust::Import => zypp_agama_sys::GPGKeyTrust_GPGKT_IMPORT, + } + } +} + +impl FromStr for GpgKeyTrust { + // TODO: make dedicated Error for it for better reusability + type Err = String; + fn from_str(s: &str) -> Result { + match s { + // NOTE: match keys with defined question for GpgKeyTrust + "Skip" => Ok(GpgKeyTrust::Reject), + "Temporary" => Ok(GpgKeyTrust::Temporary), + "Trust" => Ok(GpgKeyTrust::Import), + _ => Err(format!("Unknown action {:?}", s)), + } + } +} + +/// A trait for handling security-related callbacks from `libzypp`. +/// +/// Implementors of this trait can react to events like unknown GPG keys, +/// unsigned files, and checksum failures. +pub trait Callback { + /// Called when an unknown GPG key is encountered. + /// + /// # Returns + /// + /// A [GpgKeyTrust] value indicating whether to reject, temporarily trust, + /// or permanently import the key. + fn accept_key( + &self, + _key_id: String, + _key_name: String, + _key_fingerprint: String, + _repository_alias: String, + ) -> GpgKeyTrust { + GpgKeyTrust::Reject + } + + /// Called when an unsigned file is encountered. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn unsigned_file(&self, _file: String, _repository_alias: String) -> bool { + false + } + + /// Called when a file is signed with an unknown GPG key. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn unknown_key(&self, _file: String, _key_id: String, _repository_alias: String) -> bool { + false + } + + /// Called when GPG verification of a signed file fails. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn verification_failed( + &self, + _file: String, + _key_id: String, + _key_name: String, + _key_fingerprint: String, + _repository_alias: String, + ) -> bool { + false + } + + /// Called when a checksum is missing for a file. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn checksum_missing(&self, _file: String) -> bool { + false + } + + /// Called when a file's checksum is incorrect. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn checksum_wrong(&self, _file: String, _expected: String, _actual: String) -> bool { + false + } + + /// Called when the checksum type for a file is unknown. + /// + /// # Returns + /// + /// `true` to accept the file and continue, `false` to reject it and abort. + fn checksum_unknown(&self, _file: String, _checksum: String) -> bool { + false + } + + /// block to use callbacks inside. Recommended to not redefine. + fn with(&mut self, block: &mut F) -> R +where + F: FnMut(zypp_agama_sys::SecurityCallbacks) -> R, +{ + let mut accept_key_call = |key_id, key_name, key_fingerprint, repository_alias| { + self.accept_key(key_id, key_name, key_fingerprint, repository_alias) + }; + let cb_accept_key = get_accept_key(&accept_key_call); + + let mut unsigned_file_call = + |file, repository_alias| self.unsigned_file(file, repository_alias); + let cb_unsigned_file = get_unsigned_file(&unsigned_file_call); + + let mut unknown_key_call = + |file, key_id, repository_alias| self.unknown_key(file, key_id, repository_alias); + let cb_unknown_key = get_unknown_key(&unknown_key_call); + + let mut verification_failed_call = + |file, key_id, key_name, key_fingerprint, repository_alias| { + self.verification_failed(file, key_id, key_name, key_fingerprint, repository_alias) + }; + let cb_verification_failed = get_verification_failed(&verification_failed_call); + + let mut checksum_missing_call = |file| self.checksum_missing(file); + let cb_checksum_missing = get_checksum_missing(&checksum_missing_call); + + let mut checksum_wrong_call = + |file, expected, actual| self.checksum_wrong(file, expected, actual); + let cb_checksum_wrong = get_checksum_wrong(&checksum_wrong_call); + + let mut checksum_unknown_call = + |file, checksum| self.checksum_unknown(file, checksum); + let cb_checksum_unknown = get_checksum_unknown(&checksum_unknown_call); + + let callbacks = zypp_agama_sys::SecurityCallbacks { + accept_key: cb_accept_key, + accept_key_data: as_c_void(&mut accept_key_call), + unsigned_file: cb_unsigned_file, + unsigned_file_data: as_c_void(&mut unsigned_file_call), + unknown_key: cb_unknown_key, + unknown_key_data: as_c_void(&mut unknown_key_call), + verification_failed: cb_verification_failed, + verification_failed_data: as_c_void(&mut verification_failed_call), + checksum_missing: cb_checksum_missing, + checksum_missing_data: as_c_void(&mut checksum_missing_call), + checksum_wrong: cb_checksum_wrong, + checksum_wrong_data: as_c_void(&mut checksum_wrong_call), + checksum_unknown: cb_checksum_unknown, + checksum_unknown_data: as_c_void(&mut checksum_unknown_call), + }; + block(callbacks) +} +} + +/// Default implementation that does nothing and rejects all security risks. +pub struct EmptyCallback; +impl Callback for EmptyCallback {} + +unsafe extern "C" fn accept_key( + key_id: *const c_char, + key_name: *const c_char, + key_fingerprint: *const c_char, + repository_alias: *const c_char, + user_data: *mut c_void, +) -> zypp_agama_sys::GPGKeyTrust +where + F: FnMut(String, String, String, String) -> GpgKeyTrust, +{ + let user_data = &mut *(user_data as *mut F); + let res = user_data( + string_from_ptr(key_id), + string_from_ptr(key_name), + string_from_ptr(key_fingerprint), + string_from_ptr(repository_alias), + ); + res.into() +} + +fn get_accept_key(_closure: &F) -> zypp_agama_sys::GPGAcceptKeyCallback +where + F: FnMut(String, String, String, String) -> GpgKeyTrust, +{ + Some(accept_key::) +} + +unsafe extern "C" fn unsigned_file( + file: *const c_char, + repository_alias: *const c_char, + user_data: *mut c_void, +) -> bool +where + F: FnMut(String, String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data(string_from_ptr(file), string_from_ptr(repository_alias)) +} + +fn get_unsigned_file(_closure: &F) -> zypp_agama_sys::GPGUnsignedFile +where + F: FnMut(String, String) -> bool, +{ + Some(unsigned_file::) +} + +unsafe extern "C" fn unknown_key( + file: *const c_char, + key_id: *const c_char, + repository_alias: *const c_char, + user_data: *mut c_void, +) -> bool +where + F: FnMut(String, String, String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data( + string_from_ptr(file), + string_from_ptr(key_id), + string_from_ptr(repository_alias), + ) +} + +fn get_unknown_key(_closure: &F) -> zypp_agama_sys::GPGUnknownKey +where + F: FnMut(String, String, String) -> bool, +{ + Some(unknown_key::) +} + +unsafe extern "C" fn verification_failed( + file: *const c_char, + key_id: *const c_char, + key_name: *const c_char, + key_fingerprint: *const c_char, + repository_alias: *const c_char, + user_data: *mut c_void, +) -> bool +where + F: FnMut(String, String, String, String, String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data( + string_from_ptr(file), + string_from_ptr(key_id), + string_from_ptr(key_name), + string_from_ptr(key_fingerprint), + string_from_ptr(repository_alias), + ) +} + +fn get_verification_failed(_closure: &F) -> zypp_agama_sys::GPGVerificationFailed +where + F: FnMut(String, String, String, String, String) -> bool, +{ + Some(verification_failed::) +} + +unsafe extern "C" fn checksum_missing(file: *const c_char, user_data: *mut c_void) -> bool +where + F: FnMut(String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data(string_from_ptr(file)) +} + +fn get_checksum_missing(_closure: &F) -> zypp_agama_sys::ChecksumMissing +where + F: FnMut(String) -> bool, +{ + Some(checksum_missing::) +} + +unsafe extern "C" fn checksum_wrong( + file: *const c_char, + expected: *const c_char, + actual: *const c_char, + user_data: *mut c_void, +) -> bool +where + F: FnMut(String, String, String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data( + string_from_ptr(file), + string_from_ptr(expected), + string_from_ptr(actual), + ) +} + +fn get_checksum_wrong(_closure: &F) -> zypp_agama_sys::ChecksumWrong +where + F: FnMut(String, String, String) -> bool, +{ + Some(checksum_wrong::) +} + +unsafe extern "C" fn checksum_unknown( + file: *const c_char, + checksum: *const c_char, + user_data: *mut c_void, +) -> bool +where + F: FnMut(String, String) -> bool, +{ + let user_data = &mut *(user_data as *mut F); + user_data(string_from_ptr(file), string_from_ptr(checksum)) +} + +fn get_checksum_unknown(_closure: &F) -> zypp_agama_sys::ChecksumUnknown +where + F: FnMut(String, String) -> bool, +{ + Some(checksum_unknown::) +} + diff --git a/rust/zypp-agama/src/lib.rs b/rust/zypp-agama/src/lib.rs index f6991e5263..9c0265be17 100644 --- a/rust/zypp-agama/src/lib.rs +++ b/rust/zypp-agama/src/lib.rs @@ -152,12 +152,16 @@ impl Zypp { } } - pub fn commit(&self, report: &impl callbacks::pkg_download::Callback) -> ZyppResult { + pub fn commit(&self, report: &impl callbacks::pkg_download::Callback, security: &mut impl callbacks::security::Callback) -> ZyppResult { let mut status: Status = Status::default(); let status_ptr = &mut status as *mut _; unsafe { let mut commit_fn = - |mut callbacks| zypp_agama_sys::commit(self.ptr, status_ptr, &mut callbacks); + |mut callbacks| { + security.with(&mut |mut sec_callback| { + zypp_agama_sys::commit(self.ptr, status_ptr, &mut callbacks, &mut sec_callback) + }) + }; let res = callbacks::pkg_download::with_callback(report, &mut commit_fn); helpers::status_to_result(status, res) } @@ -356,18 +360,22 @@ impl Zypp { &self, alias: &str, progress: &impl callbacks::download_progress::Callback, + security: &mut impl callbacks::security::Callback, ) -> ZyppResult<()> { unsafe { let mut status: Status = Status::default(); let status_ptr = &mut status as *mut _; let c_alias = CString::new(alias).unwrap(); let mut refresh_fn = |mut callbacks| { - zypp_agama_sys::refresh_repository( - self.ptr, + security.with(&mut |mut sec_callback| { + zypp_agama_sys::refresh_repository( + self.ptr, c_alias.as_ptr(), status_ptr, &mut callbacks, - ) + &mut sec_callback, + ) + }) }; callbacks::download_progress::with_callback(progress, &mut refresh_fn); @@ -492,7 +500,7 @@ impl Zypp { } // high level method to load source - pub fn load_source(&self, progress: F) -> ZyppResult<()> + pub fn load_source(&self, progress: F, security: &mut impl callbacks::security::Callback) -> ZyppResult<()> where F: Fn(i64, String) -> bool, { @@ -511,7 +519,8 @@ impl Zypp { if !cont { return abort_err; } - self.refresh_repository(&i.alias, &callbacks::download_progress::EmptyCallback)?; + + self.refresh_repository(&i.alias, &callbacks::download_progress::EmptyCallback, security)?; percent += percent_step; cont = progress( percent.floor() as i64, diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/callbacks.cxx b/rust/zypp-agama/zypp-agama-sys/c-layer/callbacks.cxx index 63ce17d20d..777350a170 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/callbacks.cxx +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/callbacks.cxx @@ -2,6 +2,7 @@ #include #include #include +#include "zypp/Digest.h" #include "callbacks.h" @@ -316,6 +317,113 @@ struct CommitPreloadReport static CommitPreloadReport commit_preload_report; +struct KeyRingReport + : public zypp::callback::ReceiveReport { + struct SecurityCallbacks *callbacks; + + KeyRingReport() { callbacks = NULL; } + + void set_callbacks(SecurityCallbacks *callbacks_) { + callbacks = callbacks_; + } + + zypp::KeyRingReport::KeyTrust + askUserToAcceptKey(const zypp::PublicKey &key, + const zypp::KeyContext &context) { + if (callbacks == NULL || callbacks->accept_key == NULL) { + return zypp::KeyRingReport::askUserToAcceptKey(key, context); + } + enum GPGKeyTrust response = callbacks->accept_key( + key.id().c_str(), key.name().c_str(), key.fingerprint().c_str(), + context.repoInfo().alias().c_str(), callbacks->accept_key_data); + + return convert_trust(response); + } + + bool askUserToAcceptUnsignedFile(const std::string &file, + const zypp::KeyContext &context) { + if (callbacks == NULL || callbacks->unsigned_file == NULL) { + return zypp::KeyRingReport::askUserToAcceptUnsignedFile(file, context); + } + return callbacks->unsigned_file(file.c_str(), + context.repoInfo().alias().c_str(), + callbacks->unsigned_file_data); + } + + bool askUserToAcceptUnknownKey(const std::string &file, const std::string &id, + const zypp::KeyContext &context) { + if (callbacks == NULL || callbacks->unknown_key == NULL) { + return zypp::KeyRingReport::askUserToAcceptUnknownKey(file, id, context); + } + return callbacks->unknown_key(file.c_str(), id.c_str(), + context.repoInfo().alias().c_str(), + callbacks->unknown_key_data); + } + + bool askUserToAcceptVerificationFailed(const std::string &file, + const zypp::PublicKey &key, + const zypp::KeyContext &context) { + if (callbacks == NULL || callbacks->verification_failed == NULL) { + return zypp::KeyRingReport::askUserToAcceptVerificationFailed(file, key, + context); + } + return callbacks->verification_failed( + file.c_str(), key.id().c_str(), key.name().c_str(), + key.fingerprint().c_str(), context.repoInfo().alias().c_str(), + callbacks->verification_failed_data); + } + +private: + inline zypp::KeyRingReport::KeyTrust convert_trust(GPGKeyTrust response) { + switch (response) { + case GPGKT_REJECT: + return zypp::KeyRingReport::KEY_DONT_TRUST; + case GPGKT_TEMPORARY: + return zypp::KeyRingReport::KEY_TRUST_TEMPORARILY; + case GPGKT_IMPORT: + return zypp::KeyRingReport::KEY_TRUST_AND_IMPORT; + } + // fallback that should not happen + return zypp::KeyRingReport::KEY_DONT_TRUST; + } +}; + +static KeyRingReport key_ring_report; + +struct DigestReceive : public zypp::callback::ReceiveReport { + struct SecurityCallbacks *callbacks; + + DigestReceive() { callbacks = NULL; } + + void set_callbacks(SecurityCallbacks *callbacks_) { + callbacks = callbacks_; + } + + bool askUserToAcceptNoDigest( const zypp::Pathname &file ){ + if (callbacks == NULL || callbacks->checksum_missing == NULL) { + return zypp::DigestReport::askUserToAcceptNoDigest(file); + } + return callbacks->checksum_missing(file.c_str(), callbacks->checksum_missing_data); + } + + bool askUserToAccepUnknownDigest( const zypp::Pathname &file, const std::string &name ) { + if (callbacks == NULL || callbacks->checksum_unknown == NULL) { + return zypp::DigestReport::askUserToAccepUnknownDigest(file, name); + } + return callbacks->checksum_unknown(file.c_str(), name.c_str(), callbacks->checksum_unknown_data); + } + + bool askUserToAcceptWrongDigest( const zypp::Pathname &file, const std::string &requested, const std::string &found ) { + if (callbacks == NULL || callbacks->checksum_wrong == NULL) { + return zypp::DigestReport::askUserToAcceptWrongDigest(file, requested, found); + } + return callbacks->checksum_wrong(file.c_str(), requested.c_str(), found.c_str(), callbacks->checksum_wrong_data); + } +}; + +static DigestReceive digest_receive; + + extern "C" { void set_zypp_progress_callback(ZyppProgressCallback progress, void *user_data) { @@ -353,6 +461,21 @@ void unset_zypp_resolvable_download_callbacks() { commit_preload_report.disconnect(); } +void set_zypp_security_callbacks(struct SecurityCallbacks *callbacks) { + key_ring_report.set_callbacks(callbacks); + key_ring_report.connect(); + digest_receive.set_callbacks(callbacks); + digest_receive.connect(); +} + +void unset_zypp_security_callbacks() { + // NULL pointer to struct to be sure it is not called + key_ring_report.set_callbacks(NULL); + key_ring_report.disconnect(); + digest_receive.set_callbacks(NULL); + digest_receive.disconnect(); +} + #ifdef __cplusplus bool dynamic_progress_callback(ZyppProgressCallback progress, void *user_data, const zypp::ProgressData &task) { diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h b/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h index 84afa17e6f..fb86391b34 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h @@ -134,6 +134,120 @@ struct DownloadResolvableCallbacks { ZyppDownloadResolvableFileFinishCallback file_finish; void *file_finish_data; }; + +/** + * @brief What to do with an unknown GPG key. + * @see KeyRingReport::KeyTrust in libzypp + */ +enum GPGKeyTrust { + /** Reject the key. */ + GPGKT_REJECT, + /** Trust key temporary. Will be asked again when something is signed with it. + Even within same session. */ + GPGKT_TEMPORARY, + /** Import key and trust it. */ + GPGKT_IMPORT +}; + +/** + * @brief Callback to decide whether to accept an unknown GPG key. + * @param key_id The ID of the GPG key. + * @param key_name The name of the GPG key. + * @param key_fingerprint The fingerprint of the GPG key. + * @param repository_alias The alias of the repository providing the key. Can be + * an empty string if not available. + * @param user_data User-defined data. + * @return A GPGKeyTrust value indicating the action to take. + */ +typedef enum GPGKeyTrust (*GPGAcceptKeyCallback)(const char *key_id, + const char *key_name, + const char *key_fingerprint, + const char *repository_alias, + void *user_data); +/** + * @brief Callback for handling unsigned files. + * @param file The path to the unsigned file. + * @param repository_alias The alias of the repository. Can be an empty string + * if not available. + * @param user_data User-defined data. + * @return true to continue, false to abort. + */ +typedef bool (*GPGUnsignedFile)(const char *file, const char *repository_alias, + void *user_data); +/** + * @brief Callback for handling a file signed by an unknown key. + * @param file The path to the file. + * @param key_id The ID of the unknown GPG key. + * @param repository_alias The alias of the repository. Can be an empty string + * if not available. + * @param user_data User-defined data. + * @return true to continue, false to abort. + */ +typedef bool (*GPGUnknownKey)(const char *file, const char *key_id, + const char *repository_alias, void *user_data); +/** + * @brief Callback for when GPG verification of a signed file fails. + * @param file The path to the file. + * @param key_id The ID of the GPG key. + * @param key_name The name of the GPG key. + * @param key_fingerprint The fingerprint of the GPG key. + * @param repository_alias The alias of the repository. Can be an empty string + * if not available. + * @param user_data User-defined data. + * @return true to continue, false to abort. + */ +typedef bool (*GPGVerificationFailed)(const char *file, const char *key_id, + const char *key_name, + const char *key_fingerprint, + const char *repository_alias, + void *user_data); +typedef bool (*ChecksumMissing)(const char *file, void *user_data); +typedef bool (*ChecksumWrong)(const char *file, const char *expected, + const char *actual, void *user_data); +typedef bool (*ChecksumUnknown)(const char *file, const char *checksum, + void *user_data); + +/** + * @brief Callbacks for handling security related issues. + * + * This struct provides callbacks for handling various security-related events + * that can occur during libzypp operations, such as GPG key management and + * checksum verification. + * + * Each callback has a corresponding `_data` pointer that can be used to pass + * user-defined data to the callback function. + */ +struct SecurityCallbacks { + /** @brief Callback to decide whether to accept an unknown GPG key. */ + GPGAcceptKeyCallback accept_key; + /** @brief User data for the `accept_key` callback. */ + void *accept_key_data; + /** @brief Callback for handling unsigned files. */ + GPGUnsignedFile unsigned_file; + /** @brief User data for the `unsigned_file` callback. */ + void *unsigned_file_data; + /** @brief Callback for handling files signed with an unknown key. */ + GPGUnknownKey unknown_key; + /** @brief User data for the `unknown_key` callback. */ + void *unknown_key_data; + /** @brief Callback for handling GPG verification failures. */ + GPGVerificationFailed verification_failed; + /** @brief User data for the `verification_failed` callback. */ + void *verification_failed_data; + /** @brief Callback for when a checksum is missing. */ + ChecksumMissing checksum_missing; + /** @brief User data for the `checksum_missing` callback. */ + void *checksum_missing_data; + /** @brief Callback for when a checksum is wrong. */ + ChecksumWrong checksum_wrong; + /** @brief User data for the `checksum_wrong` callback. */ + void *checksum_wrong_data; + /** @brief Callback for when the checksum type is unknown. */ + ChecksumUnknown checksum_unknown; + /** @brief User data for the `checksum_unknown` callback. */ + void *checksum_unknown_data; +}; + #ifdef __cplusplus } #endif diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/include/lib.h b/rust/zypp-agama/zypp-agama-sys/c-layer/include/lib.h index 6325ff769f..3d2d5d00ab 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/include/lib.h +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/include/lib.h @@ -65,9 +65,11 @@ void switch_target(struct Zypp *zypp, const char *root, /// @param zypp /// @param status /// @param download_callbacks +/// @param security_callbacks /// @return true if there is no error bool commit(struct Zypp *zypp, struct Status *status, - struct DownloadResolvableCallbacks *download_callbacks) noexcept; + struct DownloadResolvableCallbacks *download_callbacks, + struct SecurityCallbacks *security_callbacks) noexcept; /// Represents a single mount point and its space usage. /// The string pointers are not owned by this struct. diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/include/repository.h b/rust/zypp-agama/zypp-agama-sys/c-layer/include/repository.h index 1c8eb55689..a00dce78be 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/include/repository.h +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/include/repository.h @@ -69,11 +69,13 @@ void remove_repository(struct Zypp *zypp, const char *alias, /// @param zypp see \ref init_target /// @param alias alias of repository to refresh /// @param[out] status (will overwrite existing contents) -/// @param callbacks pointer to struct with callbacks or NULL if no progress is +/// @param progress pointer to struct with callbacks or NULL if no progress is /// needed +/// @param security pointer to struct with security callbacks void refresh_repository(struct Zypp *zypp, const char *alias, struct Status *status, - struct DownloadProgressCallbacks *callbacks) noexcept; + struct DownloadProgressCallbacks *progress, + struct SecurityCallbacks *security) noexcept; void build_repository_cache(struct Zypp *zypp, const char *alias, struct Status *status, diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/internal/callbacks.hxx b/rust/zypp-agama/zypp-agama-sys/c-layer/internal/callbacks.hxx index 488e1d7335..5f7b21b9ef 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/internal/callbacks.hxx +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/internal/callbacks.hxx @@ -20,4 +20,9 @@ void set_zypp_resolvable_download_callbacks( struct DownloadResolvableCallbacks *callbacks); void unset_zypp_resolvable_download_callbacks(); +// pair of set/unset callbacks used for security reports. +// Uses mixture of KeyRingReport and DigestReport +void set_zypp_security_callbacks(struct SecurityCallbacks *callbacks); +void unset_zypp_security_callbacks(); + #endif diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/lib.cxx b/rust/zypp-agama/zypp-agama-sys/c-layer/lib.cxx index ffa9ae91f6..c14511c02c 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/lib.cxx +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/lib.cxx @@ -156,17 +156,21 @@ void switch_target(struct Zypp *zypp, const char *root, } bool commit(struct Zypp *zypp, struct Status *status, - struct DownloadResolvableCallbacks *download_callbacks) noexcept { + struct DownloadResolvableCallbacks *download_callbacks, + struct SecurityCallbacks *security_callbacks) noexcept { try { set_zypp_resolvable_download_callbacks(download_callbacks); + set_zypp_security_callbacks(security_callbacks); zypp::ZYppCommitPolicy policy; zypp::ZYppCommitResult result = zypp->zypp_pointer->commit(policy); STATUS_OK(status); unset_zypp_resolvable_download_callbacks(); + unset_zypp_security_callbacks(); return result.noError(); } catch (zypp::Exception &excpt) { STATUS_EXCEPT(status, excpt); unset_zypp_resolvable_download_callbacks(); + unset_zypp_security_callbacks(); return false; } } @@ -393,7 +397,8 @@ bool run_solver(struct Zypp *zypp, struct Status *status) noexcept { void refresh_repository(struct Zypp *zypp, const char *alias, struct Status *status, - struct DownloadProgressCallbacks *callbacks) noexcept { + struct DownloadProgressCallbacks *callbacks, + struct SecurityCallbacks *security_callbacks) noexcept { if (zypp->repo_manager == NULL) { STATUS_ERROR(status, "Internal Error: Repo manager is not initialized."); return; @@ -407,13 +412,16 @@ void refresh_repository(struct Zypp *zypp, const char *alias, } set_zypp_download_callbacks(callbacks); + set_zypp_security_callbacks(security_callbacks); zypp->repo_manager->refreshMetadata( zypp_repo, zypp::RepoManager::RawMetadataRefreshPolicy::RefreshIfNeeded); STATUS_OK(status); unset_zypp_download_callbacks(); + unset_zypp_security_callbacks(); } catch (zypp::Exception &excpt) { STATUS_EXCEPT(status, excpt); + unset_zypp_security_callbacks(); unset_zypp_download_callbacks(); // TODO: we can add C++ final action helper // if it is more common } diff --git a/rust/zypp-agama/zypp-agama-sys/src/bindings.rs b/rust/zypp-agama/zypp-agama-sys/src/bindings.rs index b6b58c7dbf..ed9f6b8eef 100644 --- a/rust/zypp-agama/zypp-agama-sys/src/bindings.rs +++ b/rust/zypp-agama/zypp-agama-sys/src/bindings.rs @@ -190,6 +190,139 @@ const _: () = { ["Offset of field: DownloadResolvableCallbacks::file_finish_data"] [::std::mem::offset_of!(DownloadResolvableCallbacks, file_finish_data) - 56usize]; }; +#[doc = " Reject the key."] +pub const GPGKeyTrust_GPGKT_REJECT: GPGKeyTrust = 0; +#[doc = " Trust key temporary. Will be asked again when something is signed with it.\nEven within same session."] +pub const GPGKeyTrust_GPGKT_TEMPORARY: GPGKeyTrust = 1; +#[doc = " Import key and trust it."] +pub const GPGKeyTrust_GPGKT_IMPORT: GPGKeyTrust = 2; +#[doc = " @brief What to do with an unknown GPG key.\n @see KeyRingReport::KeyTrust in libzypp"] +pub type GPGKeyTrust = ::std::os::raw::c_uint; +#[doc = " @brief Callback to decide whether to accept an unknown GPG key.\n @param key_id The ID of the GPG key.\n @param key_name The name of the GPG key.\n @param key_fingerprint The fingerprint of the GPG key.\n @param repository_alias The alias of the repository providing the key. Can be\n an empty string if not available.\n @param user_data User-defined data.\n @return A GPGKeyTrust value indicating the action to take."] +pub type GPGAcceptKeyCallback = ::std::option::Option< + unsafe extern "C" fn( + key_id: *const ::std::os::raw::c_char, + key_name: *const ::std::os::raw::c_char, + key_fingerprint: *const ::std::os::raw::c_char, + repository_alias: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> GPGKeyTrust, +>; +#[doc = " @brief Callback for handling unsigned files.\n @param file The path to the unsigned file.\n @param repository_alias The alias of the repository. Can be an empty string\n if not available.\n @param user_data User-defined data.\n @return true to continue, false to abort."] +pub type GPGUnsignedFile = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + repository_alias: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +#[doc = " @brief Callback for handling a file signed by an unknown key.\n @param file The path to the file.\n @param key_id The ID of the unknown GPG key.\n @param repository_alias The alias of the repository. Can be an empty string\n if not available.\n @param user_data User-defined data.\n @return true to continue, false to abort."] +pub type GPGUnknownKey = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + key_id: *const ::std::os::raw::c_char, + repository_alias: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +#[doc = " @brief Callback for when GPG verification of a signed file fails.\n @param file The path to the file.\n @param key_id The ID of the GPG key.\n @param key_name The name of the GPG key.\n @param key_fingerprint The fingerprint of the GPG key.\n @param repository_alias The alias of the repository. Can be an empty string\n if not available.\n @param user_data User-defined data.\n @return true to continue, false to abort."] +pub type GPGVerificationFailed = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + key_id: *const ::std::os::raw::c_char, + key_name: *const ::std::os::raw::c_char, + key_fingerprint: *const ::std::os::raw::c_char, + repository_alias: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +pub type ChecksumMissing = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +pub type ChecksumWrong = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + expected: *const ::std::os::raw::c_char, + actual: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +pub type ChecksumUnknown = ::std::option::Option< + unsafe extern "C" fn( + file: *const ::std::os::raw::c_char, + checksum: *const ::std::os::raw::c_char, + user_data: *mut ::std::os::raw::c_void, + ) -> bool, +>; +#[doc = " @brief Callbacks for handling security related issues.\n\n This struct provides callbacks for handling various security-related events\n that can occur during libzypp operations, such as GPG key management and\n checksum verification.\n\n Each callback has a corresponding `_data` pointer that can be used to pass\n user-defined data to the callback function."] +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct SecurityCallbacks { + #[doc = " @brief Callback to decide whether to accept an unknown GPG key."] + pub accept_key: GPGAcceptKeyCallback, + #[doc = " @brief User data for the `accept_key` callback."] + pub accept_key_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for handling unsigned files."] + pub unsigned_file: GPGUnsignedFile, + #[doc = " @brief User data for the `unsigned_file` callback."] + pub unsigned_file_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for handling files signed with an unknown key."] + pub unknown_key: GPGUnknownKey, + #[doc = " @brief User data for the `unknown_key` callback."] + pub unknown_key_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for handling GPG verification failures."] + pub verification_failed: GPGVerificationFailed, + #[doc = " @brief User data for the `verification_failed` callback."] + pub verification_failed_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for when a checksum is missing."] + pub checksum_missing: ChecksumMissing, + #[doc = " @brief User data for the `checksum_missing` callback."] + pub checksum_missing_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for when a checksum is wrong."] + pub checksum_wrong: ChecksumWrong, + #[doc = " @brief User data for the `checksum_wrong` callback."] + pub checksum_wrong_data: *mut ::std::os::raw::c_void, + #[doc = " @brief Callback for when the checksum type is unknown."] + pub checksum_unknown: ChecksumUnknown, + #[doc = " @brief User data for the `checksum_unknown` callback."] + pub checksum_unknown_data: *mut ::std::os::raw::c_void, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of SecurityCallbacks"][::std::mem::size_of::() - 112usize]; + ["Alignment of SecurityCallbacks"][::std::mem::align_of::() - 8usize]; + ["Offset of field: SecurityCallbacks::accept_key"] + [::std::mem::offset_of!(SecurityCallbacks, accept_key) - 0usize]; + ["Offset of field: SecurityCallbacks::accept_key_data"] + [::std::mem::offset_of!(SecurityCallbacks, accept_key_data) - 8usize]; + ["Offset of field: SecurityCallbacks::unsigned_file"] + [::std::mem::offset_of!(SecurityCallbacks, unsigned_file) - 16usize]; + ["Offset of field: SecurityCallbacks::unsigned_file_data"] + [::std::mem::offset_of!(SecurityCallbacks, unsigned_file_data) - 24usize]; + ["Offset of field: SecurityCallbacks::unknown_key"] + [::std::mem::offset_of!(SecurityCallbacks, unknown_key) - 32usize]; + ["Offset of field: SecurityCallbacks::unknown_key_data"] + [::std::mem::offset_of!(SecurityCallbacks, unknown_key_data) - 40usize]; + ["Offset of field: SecurityCallbacks::verification_failed"] + [::std::mem::offset_of!(SecurityCallbacks, verification_failed) - 48usize]; + ["Offset of field: SecurityCallbacks::verification_failed_data"] + [::std::mem::offset_of!(SecurityCallbacks, verification_failed_data) - 56usize]; + ["Offset of field: SecurityCallbacks::checksum_missing"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_missing) - 64usize]; + ["Offset of field: SecurityCallbacks::checksum_missing_data"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_missing_data) - 72usize]; + ["Offset of field: SecurityCallbacks::checksum_wrong"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_wrong) - 80usize]; + ["Offset of field: SecurityCallbacks::checksum_wrong_data"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_wrong_data) - 88usize]; + ["Offset of field: SecurityCallbacks::checksum_unknown"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_unknown) - 96usize]; + ["Offset of field: SecurityCallbacks::checksum_unknown_data"] + [::std::mem::offset_of!(SecurityCallbacks, checksum_unknown_data) - 104usize]; +}; #[doc = " status struct to pass and obtain from calls that can fail.\n After usage free with \\ref free_status function.\n\n Most functions act as *constructors* for this, taking a pointer\n to it as an output parameter, disregarding the struct current contents\n and filling it in. Thus, if you reuse a `Status` without \\ref free_status\n in between, `error` will leak."] #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -379,11 +512,12 @@ unsafe extern "C" { ) -> *mut Zypp; #[doc = " Switch Zypp target (where to install packages to).\n @param root\n @param[out] status"] pub fn switch_target(zypp: *mut Zypp, root: *const ::std::os::raw::c_char, status: *mut Status); - #[doc = " Commit zypp settings and install\n TODO: install callbacks\n @param zypp\n @param status\n @param download_callbacks\n @return true if there is no error"] + #[doc = " Commit zypp settings and install\n TODO: install callbacks\n @param zypp\n @param status\n @param download_callbacks\n @param security_callbacks\n @return true if there is no error"] pub fn commit( zypp: *mut Zypp, status: *mut Status, download_callbacks: *mut DownloadResolvableCallbacks, + security_callbacks: *mut SecurityCallbacks, ) -> bool; #[doc = " Calculates the space usage for a given list of mount points.\n This function populates the `used_size` field for each element in the\n provided `mount_points` array.\n\n @param zypp The Zypp context.\n @param[out] status Output status object.\n @param[in,out] mount_points An array of mount points to be evaluated.\n @param mount_points_size The number of elements in the `mount_points` array."] pub fn get_space_usage( @@ -471,12 +605,13 @@ unsafe extern "C" { callback: ZyppProgressCallback, user_data: *mut ::std::os::raw::c_void, ); - #[doc = "\n @param zypp see \\ref init_target\n @param alias alias of repository to refresh\n @param[out] status (will overwrite existing contents)\n @param callbacks pointer to struct with callbacks or NULL if no progress is\n needed"] + #[doc = "\n @param zypp see \\ref init_target\n @param alias alias of repository to refresh\n @param[out] status (will overwrite existing contents)\n @param progress pointer to struct with callbacks or NULL if no progress is\n needed\n @param security pointer to struct with security callbacks"] pub fn refresh_repository( zypp: *mut Zypp, alias: *const ::std::os::raw::c_char, status: *mut Status, - callbacks: *mut DownloadProgressCallbacks, + progress: *mut DownloadProgressCallbacks, + security: *mut SecurityCallbacks, ); pub fn build_repository_cache( zypp: *mut Zypp, From 6bee4ad61caf80568a155e1273f23a53689543f6 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 21 Nov 2025 11:36:54 +0100 Subject: [PATCH 2/7] add helper to properly ask question and use it --- .../src/callbacks/commit_download.rs | 21 +- rust/agama-software/src/callbacks/security.rs | 239 +++++++----------- rust/agama-software/src/zypp_server.rs | 17 +- rust/agama-utils/src/question.rs | 63 +++++ rust/zypp-agama/src/callbacks/security.rs | 105 ++++---- rust/zypp-agama/src/lib.rs | 37 ++- 6 files changed, 255 insertions(+), 227 deletions(-) diff --git a/rust/agama-software/src/callbacks/commit_download.rs b/rust/agama-software/src/callbacks/commit_download.rs index 65f546982e..b0c8d383d6 100644 --- a/rust/agama-software/src/callbacks/commit_download.rs +++ b/rust/agama-software/src/callbacks/commit_download.rs @@ -1,4 +1,9 @@ -use agama_utils::{actor::Handler, api::question::QuestionSpec, progress, question}; +use agama_utils::{ + actor::Handler, + api::question::QuestionSpec, + progress, + question::{self, ask_question}, +}; use gettextrs::gettext; use tokio::runtime::Handle; use zypp_agama::callbacks::pkg_download::{Callback, DownloadError}; @@ -45,22 +50,14 @@ impl Callback for CommitDownload { let question = QuestionSpec::new(description, "software.package_error.provide_error") .with_actions(&actions) .with_data(&data); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return zypp_agama::callbacks::ProblemResponse::ABORT; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return zypp_agama::callbacks::ProblemResponse::ABORT; - }; - - answer_str + answer .action .as_str() .parse::() diff --git a/rust/agama-software/src/callbacks/security.rs b/rust/agama-software/src/callbacks/security.rs index b0493672ae..f76d6d4a1d 100644 --- a/rust/agama-software/src/callbacks/security.rs +++ b/rust/agama-software/src/callbacks/security.rs @@ -1,4 +1,8 @@ -use agama_utils::{actor::Handler, api::question::QuestionSpec, question}; +use agama_utils::{ + actor::Handler, + api::question::QuestionSpec, + question::{self, ask_question}, +}; use gettextrs::gettext; use tokio::runtime::Handle; use zypp_agama::callbacks::security; @@ -9,12 +13,8 @@ pub struct Security { } impl Security { - pub fn new( - questions: Handler, - ) -> Self { - Self { - questions, - } + pub fn new(questions: Handler) -> Self { + Self { questions } } } @@ -23,251 +23,202 @@ impl security::Callback for Security { // TODO: support for extra_repositories with allow_unsigned config // TODO: localization for text when parameters in gextext will be solved let text = if repository_alias.is_empty() { - format!("The file {file} is not digitally signed. The origin \ - and integrity of the file cannot be verified. Use it anyway?") + format!( + "The file {file} is not digitally signed. The origin \ + and integrity of the file cannot be verified. Use it anyway?" + ) } else { - format!("The file {file} from {repository_alias} is not digitally signed. The origin \ - and integrity of the file cannot be verified. Use it anyway?") + format!( + "The file {file} from {repository_alias} is not digitally signed. The origin \ + and integrity of the file cannot be verified. Use it anyway?" + ) }; let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.unsigned_file") .with_actions(&actions) .with_data(&data) .with_default_action("No"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } fn accept_key( - &self, - key_id: String, - key_name: String, - key_fingerprint: String, - _repository_alias: String, - ) -> security::GpgKeyTrust { + &self, + key_id: String, + key_name: String, + key_fingerprint: String, + _repository_alias: String, + ) -> security::GpgKeyTrust { // TODO: support for extra_repositories with specified gpg key checksum // TODO: localization with params let text = format!( "The key {key_id} ({key_name}) with fingerprint {key_fingerprint} is unknown. \ Do you want to trust this key?" - ); + ); let labels = [gettext("Trust"), gettext("Skip")]; - let actions = [ - ("Trust", labels[0].as_str()), - ("Skip", labels[1].as_str()), + let actions = [("Trust", labels[0].as_str()), ("Skip", labels[1].as_str())]; + let data = [ + ("id", key_id.as_str()), + ("name", key_name.as_str()), + ("fingerprint", key_fingerprint.as_str()), ]; - let data = [("id", key_id.as_str()), ("name", key_name.as_str()), ("fingerprint", key_fingerprint.as_str())]; let question = QuestionSpec::new(&text, "software.import_gpg") - .with_actions(&actions) - .with_data(&data) - .with_default_action("Skip"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + .with_actions(&actions) + .with_data(&data) + .with_default_action("Skip"); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return security::GpgKeyTrust::Reject; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return security::GpgKeyTrust::Reject; - }; - answer_str + answer .action .as_str() .parse::() .unwrap_or(security::GpgKeyTrust::Reject) - } + } fn unknown_key(&self, file: String, key_id: String, repository_alias: String) -> bool { // TODO: localization for text when parameters in gextext will be solved let text = if repository_alias.is_empty() { - format!("The file {file} is digitally signed with \ - the following unknown GnuPG key: {key_id}. Use it anyway?") + format!( + "The file {file} is digitally signed with \ + the following unknown GnuPG key: {key_id}. Use it anyway?" + ) } else { - format!("The file {file} from {repository_alias} is digitally signed with \ - the following unknown GnuPG key: {key_id}. Use it anyway?") + format!( + "The file {file} from {repository_alias} is digitally signed with \ + the following unknown GnuPG key: {key_id}. Use it anyway?" + ) }; let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str()), ("id", key_id.as_str())]; let question = QuestionSpec::new(&text, "software.unknown_gpg") .with_actions(&actions) .with_data(&data) .with_default_action("No"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } fn verification_failed( - &self, - file: String, - key_id: String, - key_name: String, - key_fingerprint: String, - repository_alias: String, - ) -> bool { - // TODO: localization for text when parameters in gextext will be solved + &self, + file: String, + key_id: String, + key_name: String, + key_fingerprint: String, + repository_alias: String, + ) -> bool { + // TODO: localization for text when parameters in gextext will be solved let text = if repository_alias.is_empty() { - format!("The file {file} is digitally signed with the \ + format!( + "The file {file} is digitally signed with the \ following GnuPG key, but the integrity check failed: {key_id} ({key_name}). \ - Use it anyway?") + Use it anyway?" + ) } else { // TODO: Originally it uses repository url and not alias. Does it matter? - format!("The file {file} from {repository_alias} is digitally signed with the \ + format!( + "The file {file} from {repository_alias} is digitally signed with the \ following GnuPG key, but the integrity check failed: {key_id} ({key_name}). \ - Use it anyway?") + Use it anyway?" + ) }; let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.verification_failed") .with_actions(&actions) .with_data(&data) .with_default_action("No"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } fn checksum_missing(&self, file: String) -> bool { - // TODO: localization for text when parameters in gextext will be solved - let text = format!("No checksum for the file {file} was found in the repository. This means that \ + // TODO: localization for text when parameters in gextext will be solved + let text = format!( + "No checksum for the file {file} was found in the repository. This means that \ although the file is part of the signed repository, the list of checksums \ - does not mention this file. Use it anyway?"); + does not mention this file. Use it anyway?" + ); let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let question = QuestionSpec::new(&text, "software.digest.no_digest") .with_actions(&actions) .with_default_action("Yes"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } fn checksum_unknown(&self, file: String, checksum: String) -> bool { - let text = format!("The checksum of the file {file} is \"{checksum}\" but the expected checksum is \ + let text = format!( + "The checksum of the file {file} is \"{checksum}\" but the expected checksum is \ unknown. This means that the origin and integrity of the file cannot be verified. \ - Use it anyway?"); + Use it anyway?" + ); let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let question = QuestionSpec::new(&text, "software.digest.unknown_digest") .with_actions(&actions) .with_default_action("Yes"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } fn checksum_wrong(&self, file: String, expected: String, actual: String) -> bool { - let text = format!("The expected checksum of file %{file} is \"%{actual}\" but it was expected to be \ + let text = format!( + "The expected checksum of file %{file} is \"%{actual}\" but it was expected to be \ \"%{expected}\". The file has changed by accident or by an attacker since the \ - creater signed it. Use it anyway?"); + creater signed it. Use it anyway?" + ); let labels = [gettext("Yes"), gettext("No")]; - let actions = [ - ("Yes", labels[0].as_str()), - ("No", labels[1].as_str()), - ]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let question = QuestionSpec::new(&text, "software.digest.unknown_digest") .with_actions(&actions) .with_default_action("Yes"); - let result = Handle::current().block_on(async move { - self.questions - .call(question::message::Ask::new(question)) - .await - }); + let result = Handle::current() + .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return false; }; - let Some(answer_str) = answer.answer else { - tracing::warn!("No answer provided"); - return false; - }; - answer_str.action == "Yes" + answer.action == "Yes" } } - diff --git a/rust/agama-software/src/zypp_server.rs b/rust/agama-software/src/zypp_server.rs index d2575625eb..75e5010a3a 100644 --- a/rust/agama-software/src/zypp_server.rs +++ b/rust/agama-software/src/zypp_server.rs @@ -178,13 +178,15 @@ impl ZyppServer { tx, } => { let mut security_callback = security::Security::new(question); - self.write(state, progress, &mut security_callback, tx, zypp).await?; + self.write(state, progress, &mut security_callback, tx, zypp) + .await?; } SoftwareAction::GetPatternsMetadata(names, tx) => { self.get_patterns(names, tx, zypp).await?; } SoftwareAction::Install(tx, progress, question) => { - let download_callback = commit_download::CommitDownload::new(progress, question.clone()); + let download_callback = + commit_download::CommitDownload::new(progress, question.clone()); let mut security_callback = security::Security::new(question); tx.send(self.install(zypp, &download_callback, &mut security_callback)) .map_err(|_| ZyppDispatchError::ResponseChannelClosed)?; @@ -311,10 +313,13 @@ impl ZyppServer { progress.cast(progress::message::Next::new(Scope::Software))?; if to_add.is_empty() || to_remove.is_empty() { - let result = zypp.load_source(|percent, alias| { - tracing::info!("Refreshing repositories: {} ({}%)", alias, percent); - true - }, security); + let result = zypp.load_source( + |percent, alias| { + tracing::info!("Refreshing repositories: {} ({}%)", alias, percent); + true + }, + security, + ); if let Err(error) = result { let message = format!("Could not read the repositories"); diff --git a/rust/agama-utils/src/question.rs b/rust/agama-utils/src/question.rs index b4776a469f..dfd539056a 100644 --- a/rust/agama-utils/src/question.rs +++ b/rust/agama-utils/src/question.rs @@ -52,9 +52,72 @@ //! # }); //! ``` pub mod service; +use std::time::Duration; + pub use service::Service; pub mod message; pub mod start; pub use start::start; +use tokio::time::sleep; + +use crate::{ + actor::Handler, + api::question::{Answer, QuestionSpec}, + question, +}; + +#[derive(thiserror::Error, Debug)] +pub enum AskError { + #[error("Question not found")] + QuestionNotFound, + #[error(transparent)] + Service(#[from] question::service::Error), + #[error(transparent)] + Actor(#[from] crate::actor::Error), +} + +/// Asks a question and waits until it is answered. +/// +/// This is a helper function for internal Rust services that have a direct handler to the +/// `question::Service` and need to ask a question and wait for the answer in a +/// synchronous-like manner within an `async` context. It simplifies the process by +/// abstracting away the need to listen for events or poll the API, which would +/// typically be done by an external client (like a web UI). +/// +/// The function sends the question to the `question::Service` and then polls for an +/// answer. Once the question is answered (either by a user, a pre-configured rule, +/// or a default policy), the function returns the answer and deletes the question +/// from the service to clean up. +/// +/// # Arguments +/// * `handler` - A handler to the `question::Service`. +/// * `question` - The `QuestionSpec` defining the question to be asked. +/// +/// # Errors +/// This function can return the following errors: +/// * `AskError::QuestionNotFound`: If the question is deleted by another process before it can be answered. +/// * `AskError::Service`: If there is an error within the `question::Service` actor. +/// * `AskError::Actor`: If there is a communication error with the actor system (e.g., the actor task has terminated). +pub async fn ask_question( + handler: &Handler, + question: QuestionSpec, +) -> Result { + let result = handler.call(message::Ask::new(question)).await?; + let mut answer = result.answer; + while answer.is_none() { + // FIXME: use more efficient way than active polling + sleep(Duration::from_secs(1)).await; + let questions = handler.call(message::Get {}).await?; + let new_question = questions.iter().find(|q| q.id == result.id); + let Some(new_question) = new_question else { + // someone remove the question. Should not happen + return Err(AskError::QuestionNotFound); + }; + answer = new_question.answer.clone(); + } + handler.cast(message::Delete { id: result.id })?; + // here unwrap is ok as we know it cannot be none due to previous logic in while loop + Ok(answer.unwrap()) +} diff --git a/rust/zypp-agama/src/callbacks/security.rs b/rust/zypp-agama/src/callbacks/security.rs index 0786147073..66e70619aa 100644 --- a/rust/zypp-agama/src/callbacks/security.rs +++ b/rust/zypp-agama/src/callbacks/security.rs @@ -1,4 +1,7 @@ -use std::{os::raw::{c_char, c_void}, str::FromStr}; +use std::{ + os::raw::{c_char, c_void}, + str::FromStr, +}; use crate::helpers::{as_c_void, string_from_ptr}; @@ -121,57 +124,56 @@ pub trait Callback { /// block to use callbacks inside. Recommended to not redefine. fn with(&mut self, block: &mut F) -> R -where - F: FnMut(zypp_agama_sys::SecurityCallbacks) -> R, -{ - let mut accept_key_call = |key_id, key_name, key_fingerprint, repository_alias| { - self.accept_key(key_id, key_name, key_fingerprint, repository_alias) - }; - let cb_accept_key = get_accept_key(&accept_key_call); - - let mut unsigned_file_call = - |file, repository_alias| self.unsigned_file(file, repository_alias); - let cb_unsigned_file = get_unsigned_file(&unsigned_file_call); - - let mut unknown_key_call = - |file, key_id, repository_alias| self.unknown_key(file, key_id, repository_alias); - let cb_unknown_key = get_unknown_key(&unknown_key_call); - - let mut verification_failed_call = - |file, key_id, key_name, key_fingerprint, repository_alias| { - self.verification_failed(file, key_id, key_name, key_fingerprint, repository_alias) + where + F: FnMut(zypp_agama_sys::SecurityCallbacks) -> R, + { + let mut accept_key_call = |key_id, key_name, key_fingerprint, repository_alias| { + self.accept_key(key_id, key_name, key_fingerprint, repository_alias) }; - let cb_verification_failed = get_verification_failed(&verification_failed_call); - - let mut checksum_missing_call = |file| self.checksum_missing(file); - let cb_checksum_missing = get_checksum_missing(&checksum_missing_call); - - let mut checksum_wrong_call = - |file, expected, actual| self.checksum_wrong(file, expected, actual); - let cb_checksum_wrong = get_checksum_wrong(&checksum_wrong_call); - - let mut checksum_unknown_call = - |file, checksum| self.checksum_unknown(file, checksum); - let cb_checksum_unknown = get_checksum_unknown(&checksum_unknown_call); - - let callbacks = zypp_agama_sys::SecurityCallbacks { - accept_key: cb_accept_key, - accept_key_data: as_c_void(&mut accept_key_call), - unsigned_file: cb_unsigned_file, - unsigned_file_data: as_c_void(&mut unsigned_file_call), - unknown_key: cb_unknown_key, - unknown_key_data: as_c_void(&mut unknown_key_call), - verification_failed: cb_verification_failed, - verification_failed_data: as_c_void(&mut verification_failed_call), - checksum_missing: cb_checksum_missing, - checksum_missing_data: as_c_void(&mut checksum_missing_call), - checksum_wrong: cb_checksum_wrong, - checksum_wrong_data: as_c_void(&mut checksum_wrong_call), - checksum_unknown: cb_checksum_unknown, - checksum_unknown_data: as_c_void(&mut checksum_unknown_call), - }; - block(callbacks) -} + let cb_accept_key = get_accept_key(&accept_key_call); + + let mut unsigned_file_call = + |file, repository_alias| self.unsigned_file(file, repository_alias); + let cb_unsigned_file = get_unsigned_file(&unsigned_file_call); + + let mut unknown_key_call = + |file, key_id, repository_alias| self.unknown_key(file, key_id, repository_alias); + let cb_unknown_key = get_unknown_key(&unknown_key_call); + + let mut verification_failed_call = + |file, key_id, key_name, key_fingerprint, repository_alias| { + self.verification_failed(file, key_id, key_name, key_fingerprint, repository_alias) + }; + let cb_verification_failed = get_verification_failed(&verification_failed_call); + + let mut checksum_missing_call = |file| self.checksum_missing(file); + let cb_checksum_missing = get_checksum_missing(&checksum_missing_call); + + let mut checksum_wrong_call = + |file, expected, actual| self.checksum_wrong(file, expected, actual); + let cb_checksum_wrong = get_checksum_wrong(&checksum_wrong_call); + + let mut checksum_unknown_call = |file, checksum| self.checksum_unknown(file, checksum); + let cb_checksum_unknown = get_checksum_unknown(&checksum_unknown_call); + + let callbacks = zypp_agama_sys::SecurityCallbacks { + accept_key: cb_accept_key, + accept_key_data: as_c_void(&mut accept_key_call), + unsigned_file: cb_unsigned_file, + unsigned_file_data: as_c_void(&mut unsigned_file_call), + unknown_key: cb_unknown_key, + unknown_key_data: as_c_void(&mut unknown_key_call), + verification_failed: cb_verification_failed, + verification_failed_data: as_c_void(&mut verification_failed_call), + checksum_missing: cb_checksum_missing, + checksum_missing_data: as_c_void(&mut checksum_missing_call), + checksum_wrong: cb_checksum_wrong, + checksum_wrong_data: as_c_void(&mut checksum_wrong_call), + checksum_unknown: cb_checksum_unknown, + checksum_unknown_data: as_c_void(&mut checksum_unknown_call), + }; + block(callbacks) + } } /// Default implementation that does nothing and rejects all security risks. @@ -333,4 +335,3 @@ where { Some(checksum_unknown::) } - diff --git a/rust/zypp-agama/src/lib.rs b/rust/zypp-agama/src/lib.rs index b9b42d8ccf..fb5da43b21 100644 --- a/rust/zypp-agama/src/lib.rs +++ b/rust/zypp-agama/src/lib.rs @@ -152,16 +152,19 @@ impl Zypp { } } - pub fn commit(&self, report: &impl callbacks::pkg_download::Callback, security: &mut impl callbacks::security::Callback) -> ZyppResult { + pub fn commit( + &self, + report: &impl callbacks::pkg_download::Callback, + security: &mut impl callbacks::security::Callback, + ) -> ZyppResult { let mut status: Status = Status::default(); let status_ptr = &mut status as *mut _; unsafe { - let mut commit_fn = - |mut callbacks| { - security.with(&mut |mut sec_callback| { - zypp_agama_sys::commit(self.ptr, status_ptr, &mut callbacks, &mut sec_callback) - }) - }; + let mut commit_fn = |mut callbacks| { + security.with(&mut |mut sec_callback| { + zypp_agama_sys::commit(self.ptr, status_ptr, &mut callbacks, &mut sec_callback) + }) + }; let res = callbacks::pkg_download::with_callback(report, &mut commit_fn); helpers::status_to_result(status, res) } @@ -376,10 +379,10 @@ impl Zypp { security.with(&mut |mut sec_callback| { zypp_agama_sys::refresh_repository( self.ptr, - c_alias.as_ptr(), - status_ptr, - &mut callbacks, - &mut sec_callback, + c_alias.as_ptr(), + status_ptr, + &mut callbacks, + &mut sec_callback, ) }) }; @@ -506,7 +509,11 @@ impl Zypp { } // high level method to load source - pub fn load_source(&self, progress: F, security: &mut impl callbacks::security::Callback) -> ZyppResult<()> + pub fn load_source( + &self, + progress: F, + security: &mut impl callbacks::security::Callback, + ) -> ZyppResult<()> where F: Fn(i64, String) -> bool, { @@ -526,7 +533,11 @@ impl Zypp { return abort_err; } - self.refresh_repository(&i.alias, &callbacks::download_progress::EmptyCallback, security)?; + self.refresh_repository( + &i.alias, + &callbacks::download_progress::EmptyCallback, + security, + )?; percent += percent_step; cont = progress( percent.floor() as i64, From d9976ddea0dcf24f156f03bfda9b6aef96f2fee0 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 21 Nov 2025 15:46:32 +0100 Subject: [PATCH 3/7] add yes_no_action helper as QoL improvement --- rust/agama-software/src/callbacks/security.rs | 44 ++++++------------- rust/agama-software/src/zypp_server.rs | 4 +- rust/agama-utils/src/api/question.rs | 23 ++++++++++ 3 files changed, 38 insertions(+), 33 deletions(-) diff --git a/rust/agama-software/src/callbacks/security.rs b/rust/agama-software/src/callbacks/security.rs index f76d6d4a1d..cd3291644c 100644 --- a/rust/agama-software/src/callbacks/security.rs +++ b/rust/agama-software/src/callbacks/security.rs @@ -33,13 +33,10 @@ impl security::Callback for Security { and integrity of the file cannot be verified. Use it anyway?" ) }; - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.unsigned_file") - .with_actions(&actions) - .with_data(&data) - .with_default_action("No"); + .with_yes_no_actions() + .with_data(&data); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -101,13 +98,10 @@ impl security::Callback for Security { the following unknown GnuPG key: {key_id}. Use it anyway?" ) }; - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str()), ("id", key_id.as_str())]; let question = QuestionSpec::new(&text, "software.unknown_gpg") - .with_actions(&actions) - .with_data(&data) - .with_default_action("No"); + .with_yes_no_actions() + .with_data(&data); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -123,7 +117,7 @@ impl security::Callback for Security { file: String, key_id: String, key_name: String, - key_fingerprint: String, + _key_fingerprint: String, repository_alias: String, ) -> bool { // TODO: localization for text when parameters in gextext will be solved @@ -141,13 +135,10 @@ impl security::Callback for Security { Use it anyway?" ) }; - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.verification_failed") - .with_actions(&actions) - .with_data(&data) - .with_default_action("No"); + .with_yes_no_actions() + .with_data(&data); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -165,11 +156,7 @@ impl security::Callback for Security { although the file is part of the signed repository, the list of checksums \ does not mention this file. Use it anyway?" ); - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; - let question = QuestionSpec::new(&text, "software.digest.no_digest") - .with_actions(&actions) - .with_default_action("Yes"); + let question = QuestionSpec::new(&text, "software.digest.no_digest").with_yes_no_actions(); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -186,11 +173,8 @@ impl security::Callback for Security { unknown. This means that the origin and integrity of the file cannot be verified. \ Use it anyway?" ); - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; - let question = QuestionSpec::new(&text, "software.digest.unknown_digest") - .with_actions(&actions) - .with_default_action("Yes"); + let question = + QuestionSpec::new(&text, "software.digest.unknown_digest").with_yes_no_actions(); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -207,11 +191,9 @@ impl security::Callback for Security { \"%{expected}\". The file has changed by accident or by an attacker since the \ creater signed it. Use it anyway?" ); - let labels = [gettext("Yes"), gettext("No")]; - let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; - let question = QuestionSpec::new(&text, "software.digest.unknown_digest") - .with_actions(&actions) - .with_default_action("Yes"); + + let question = + QuestionSpec::new(&text, "software.digest.unknown_digest").with_yes_no_actions(); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { diff --git a/rust/agama-software/src/zypp_server.rs b/rust/agama-software/src/zypp_server.rs index 75e5010a3a..f3b765b9a8 100644 --- a/rust/agama-software/src/zypp_server.rs +++ b/rust/agama-software/src/zypp_server.rs @@ -377,8 +377,8 @@ impl ZyppServer { .map_err(|_| ZyppDispatchError::ResponseChannelClosed)?; return Ok(()); } - self.registration_finish(); // TODO: move it outside of zypp server as it do not need zypp lock - self.modify_zypp_conf(); // TODO: move it outside of zypp server as it do not need zypp lock + let _ = self.registration_finish(); // TODO: move it outside of zypp server as it do not need zypp lock + let _ = self.modify_zypp_conf(); // TODO: move it outside of zypp server as it do not need zypp lock if let Err(error) = self.modify_full_repo(zypp) { tx.send(Err(error.into())) diff --git a/rust/agama-utils/src/api/question.rs b/rust/agama-utils/src/api/question.rs index cd101159ac..bd70aea016 100644 --- a/rust/agama-utils/src/api/question.rs +++ b/rust/agama-utils/src/api/question.rs @@ -236,6 +236,29 @@ impl QuestionSpec { self } + /// Adds localized "Yes" and "No" actions to the question. + /// + /// This is a convenience method for creating questions that require a simple + /// yes or no answer. The action IDs will be "Yes" and "No", and their labels + /// will be localized. + /// + /// This method sets the default action to "No". If you need a different + /// default, you can override it with a subsequent call to `with_default_action()`. + /// + /// # Example + /// + /// ``` + /// use agama_utils::api::question::QuestionSpec; + /// let q = QuestionSpec::new("Continue?", "q.continue").with_yes_no_actions(); + /// assert_eq!(q.default_action.as_deref(), Some("No")); + /// ``` + pub fn with_yes_no_actions(self) -> Self { + // we have to always call fresh gettext to ensure it is properly localized after change of locale + let labels = [gettextrs::gettext("Yes"), gettextrs::gettext("No")]; + let actions = [("Yes", labels[0].as_str()), ("No", labels[1].as_str())]; + self.with_actions(&actions).with_default_action("No") + } + /// Sets the additional data. /// /// * `data`: available actions in `(id, label)` format. From 2000ad18509ba0a19626aca1cdd293f3345c2f0b Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 21 Nov 2025 22:47:04 +0100 Subject: [PATCH 4/7] change trait to pass string as it is what we construct from libzypp and can pass it to avoid double allocate --- .../src/callbacks/commit_download.rs | 20 +++-- rust/agama-software/src/zypp_server.rs | 6 +- .../src/callbacks/download_progress.rs | 22 ++--- rust/zypp-agama/src/callbacks/pkg_download.rs | 80 ++++++++++--------- rust/zypp-agama/src/lib.rs | 14 ++-- 5 files changed, 80 insertions(+), 62 deletions(-) diff --git a/rust/agama-software/src/callbacks/commit_download.rs b/rust/agama-software/src/callbacks/commit_download.rs index b0c8d383d6..3f501a0f2a 100644 --- a/rust/agama-software/src/callbacks/commit_download.rs +++ b/rust/agama-software/src/callbacks/commit_download.rs @@ -34,9 +34,9 @@ impl Callback for CommitDownload { fn problem( &self, - name: &str, + name: String, error: DownloadError, - description: &str, + description: String, ) -> zypp_agama::callbacks::ProblemResponse { // TODO: make it generic for any problemResponse questions // TODO: we need support for abort and make it default action @@ -46,10 +46,14 @@ impl Callback for CommitDownload { ("Ignore", labels[1].as_str()), ]; let error_str = error.to_string(); - let data = [("package", name), ("error_code", error_str.as_str())]; - let question = QuestionSpec::new(description, "software.package_error.provide_error") - .with_actions(&actions) - .with_data(&data); + let data = [ + ("package", name.as_str()), + ("error_code", error_str.as_str()), + ]; + let question = + QuestionSpec::new(description.as_str(), "software.package_error.provide_error") + .with_actions(&actions) + .with_data(&data); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -66,8 +70,8 @@ impl Callback for CommitDownload { fn gpg_check( &self, - resolvable_name: &str, - _repo_url: &str, + resolvable_name: String, + _repo_url: String, check_result: zypp_agama::callbacks::pkg_download::GPGCheckResult, ) -> Option { if check_result == zypp_agama::callbacks::pkg_download::GPGCheckResult::Ok { diff --git a/rust/agama-software/src/zypp_server.rs b/rust/agama-software/src/zypp_server.rs index f3b765b9a8..7be49286b6 100644 --- a/rust/agama-software/src/zypp_server.rs +++ b/rust/agama-software/src/zypp_server.rs @@ -185,10 +185,10 @@ impl ZyppServer { self.get_patterns(names, tx, zypp).await?; } SoftwareAction::Install(tx, progress, question) => { - let download_callback = + let mut download_callback = commit_download::CommitDownload::new(progress, question.clone()); let mut security_callback = security::Security::new(question); - tx.send(self.install(zypp, &download_callback, &mut security_callback)) + tx.send(self.install(zypp, &mut download_callback, &mut security_callback)) .map_err(|_| ZyppDispatchError::ResponseChannelClosed)?; } SoftwareAction::Finish(tx) => { @@ -205,7 +205,7 @@ impl ZyppServer { fn install( &self, zypp: &zypp_agama::Zypp, - download_callback: &commit_download::CommitDownload, + download_callback: &mut commit_download::CommitDownload, security_callback: &mut security::Security, ) -> ZyppServerResult { let target = "/mnt"; diff --git a/rust/zypp-agama/src/callbacks/download_progress.rs b/rust/zypp-agama/src/callbacks/download_progress.rs index 103eec5128..ab0b4c3fc1 100644 --- a/rust/zypp-agama/src/callbacks/download_progress.rs +++ b/rust/zypp-agama/src/callbacks/download_progress.rs @@ -56,7 +56,7 @@ pub trait Callback { /// /// * `_url`: The URL of the file being downloaded. /// * `_localfile`: The local path where the file will be stored. - fn start(&self, _url: &str, _localfile: &str) {} + fn start(&self, _url: String, _localfile: String) {} /// Called periodically to report download progress. /// @@ -70,7 +70,7 @@ pub trait Callback { /// # Returns /// /// `true` to continue the download, `false` to abort it. - fn progress(&self, _value: i32, _url: &str, _bps_avg: f64, _bps_current: f64) -> bool { + fn progress(&self, _value: i32, _url: String, _bps_avg: f64, _bps_current: f64) -> bool { true } @@ -85,7 +85,12 @@ pub trait Callback { /// # Returns /// /// A [ProblemResponse] indicating how to proceed (e.g., abort, retry, ignore). - fn problem(&self, _url: &str, _error_id: DownloadError, _description: &str) -> ProblemResponse { + fn problem( + &self, + _url: String, + _error_id: DownloadError, + _description: String, + ) -> ProblemResponse { ProblemResponse::ABORT } @@ -96,7 +101,7 @@ pub trait Callback { /// * `_url`: The URL of the downloaded file. /// * `_error_id`: [DownloadError::NoError] on success, or the specific error on failure. /// * `_reason`: A string providing more details about the finish status. - fn finish(&self, _url: &str, _error_id: DownloadError, _reason: &str) {} + fn finish(&self, _url: String, _error_id: DownloadError, _reason: String) {} } // Default progress that do nothing @@ -187,17 +192,16 @@ pub(crate) fn with_callback(callbacks: &impl Callback, block: &mut F) -> R where F: FnMut(zypp_agama_sys::DownloadProgressCallbacks) -> R, { - let mut start_call = |url: String, localfile: String| callbacks.start(&url, &localfile); + let mut start_call = |url: String, localfile: String| callbacks.start(url, localfile); let cb_start = get_start(&start_call); let mut progress_call = |value, url: String, bps_avg, bps_current| { - callbacks.progress(value, &url, bps_avg, bps_current) + callbacks.progress(value, url, bps_avg, bps_current) }; let cb_progress = get_progress(&progress_call); let mut problem_call = - |url: String, error, description: String| callbacks.problem(&url, error, &description); + |url: String, error, description: String| callbacks.problem(url, error, description); let cb_problem = get_problem(&problem_call); - let mut finish_call = - |url: String, error, description: String| callbacks.finish(&url, error, &description); + let mut finish_call = |url: String, error, reason: String| callbacks.finish(url, error, reason); let cb_finish = get_finish(&finish_call); let callbacks = zypp_agama_sys::DownloadProgressCallbacks { diff --git a/rust/zypp-agama/src/callbacks/pkg_download.rs b/rust/zypp-agama/src/callbacks/pkg_download.rs index fd5d241031..3eab33bd63 100644 --- a/rust/zypp-agama/src/callbacks/pkg_download.rs +++ b/rust/zypp-agama/src/callbacks/pkg_download.rs @@ -115,7 +115,12 @@ pub trait Callback { /// callback when problem occurs during download of resolvable /// /// Corresponding libzypp callback name: DownloadResolvableReport::problem - fn problem(&self, _name: &str, _error: DownloadError, _description: &str) -> ProblemResponse { + fn problem( + &self, + _name: String, + _error: DownloadError, + _description: String, + ) -> ProblemResponse { ProblemResponse::ABORT } /// Callback after a GPG check is performed on a package. @@ -131,8 +136,8 @@ pub trait Callback { /// Corresponding libzypp callback name: DownloadResolvableReport::pkgGpgCheck fn gpg_check( &self, - _resolvable_name: &str, - _repo_url: &str, + _resolvable_name: String, + _repo_url: String, _check_result: GPGCheckResult, ) -> Option { None @@ -151,12 +156,44 @@ pub trait Callback { /// Corresponding libzypp callback name: CommitPreloadReport::fileDone fn finish_preload( &self, - _url: &str, - _local_path: &str, + _url: String, + _local_path: String, _error: PreloadError, - _error_details: &str, + _error_details: String, ) { } + + /// block to use callbacks inside. Recommended to not redefine. + fn with(&mut self, block: &mut F) -> R + where + F: FnMut(zypp_agama_sys::DownloadResolvableCallbacks) -> R, + { + let mut start_call = || self.start_preload(); + let cb_start = get_start_preload(&start_call); + let mut problem_call = + |name: String, error, description: String| self.problem(name, error, description); + let cb_problem = get_problem(&problem_call); + let mut gpg_check = |name: String, url: String, check_result: GPGCheckResult| { + self.gpg_check(name, url, check_result) + }; + let cb_gpg_check = get_gpg_check(&gpg_check); + let mut finish_call = |url: String, local_path: String, error, details: String| { + self.finish_preload(url, local_path, error, details) + }; + let cb_finish = get_preload_finish(&finish_call); + + let callbacks = zypp_agama_sys::DownloadResolvableCallbacks { + start_preload: cb_start, + start_preload_data: as_c_void(&mut start_call), + problem: cb_problem, + problem_data: as_c_void(&mut problem_call), + gpg_check: cb_gpg_check, + gpg_check_data: as_c_void(&mut gpg_check), + file_finish: cb_finish, + file_finish_data: as_c_void(&mut finish_call), + }; + block(callbacks) + } } // Default progress that do nothing @@ -255,34 +292,3 @@ where { Some(preload_finish::) } - -pub(crate) fn with_callback(callback: &impl Callback, block: &mut F) -> R -where - F: FnMut(zypp_agama_sys::DownloadResolvableCallbacks) -> R, -{ - let mut start_call = || callback.start_preload(); - let cb_start = get_start_preload(&start_call); - let mut problem_call = - |name: String, error, description: String| callback.problem(&name, error, &description); - let cb_problem = get_problem(&problem_call); - let mut gpg_check = |name: String, url: String, check_result: GPGCheckResult| { - callback.gpg_check(&name, &url, check_result) - }; - let cb_gpg_check = get_gpg_check(&gpg_check); - let mut finish_call = |url: String, local_path: String, error, details: String| { - callback.finish_preload(&url, &local_path, error, &details) - }; - let cb_finish = get_preload_finish(&finish_call); - - let callbacks = zypp_agama_sys::DownloadResolvableCallbacks { - start_preload: cb_start, - start_preload_data: as_c_void(&mut start_call), - problem: cb_problem, - problem_data: as_c_void(&mut problem_call), - gpg_check: cb_gpg_check, - gpg_check_data: as_c_void(&mut gpg_check), - file_finish: cb_finish, - file_finish_data: as_c_void(&mut finish_call), - }; - block(callbacks) -} diff --git a/rust/zypp-agama/src/lib.rs b/rust/zypp-agama/src/lib.rs index fb5da43b21..5a04180e2a 100644 --- a/rust/zypp-agama/src/lib.rs +++ b/rust/zypp-agama/src/lib.rs @@ -154,18 +154,22 @@ impl Zypp { pub fn commit( &self, - report: &impl callbacks::pkg_download::Callback, + report: &mut impl callbacks::pkg_download::Callback, security: &mut impl callbacks::security::Callback, ) -> ZyppResult { let mut status: Status = Status::default(); let status_ptr = &mut status as *mut _; unsafe { - let mut commit_fn = |mut callbacks| { + let res = report.with(&mut |mut report_callback| { security.with(&mut |mut sec_callback| { - zypp_agama_sys::commit(self.ptr, status_ptr, &mut callbacks, &mut sec_callback) + zypp_agama_sys::commit( + self.ptr, + status_ptr, + &mut report_callback, + &mut sec_callback, + ) }) - }; - let res = callbacks::pkg_download::with_callback(report, &mut commit_fn); + }); helpers::status_to_result(status, res) } } From 1d44098d91d3665dfea2c3c1dc1adc7e287d157e Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 24 Nov 2025 14:23:41 +0100 Subject: [PATCH 5/7] changes from review --- rust/agama-software/src/callbacks.rs | 17 ++++++++++++++-- .../src/callbacks/commit_download.rs | 14 ++++++------- rust/agama-software/src/callbacks/security.rs | 20 ++++++++----------- rust/agama-software/src/zypp_server.rs | 14 ++++++------- rust/agama-utils/src/api/question.rs | 8 +++++++- rust/agama-utils/src/question.rs | 14 ++++++------- 6 files changed, 51 insertions(+), 36 deletions(-) diff --git a/rust/agama-software/src/callbacks.rs b/rust/agama-software/src/callbacks.rs index f693aa98e8..53fa21c97a 100644 --- a/rust/agama-software/src/callbacks.rs +++ b/rust/agama-software/src/callbacks.rs @@ -1,2 +1,15 @@ -pub mod commit_download; -pub mod security; +mod commit_download; +use agama_utils::{actor::Handler, api::question::{Answer, QuestionSpec}, question::{self, AskError, ask_question}}; +pub use commit_download::CommitDownload; +mod security; +pub use security::Security; +use tokio::runtime::Handle; + + +fn ask_software_question( + handler: &Handler, + question: QuestionSpec, +) -> Result { + Handle::current() + .block_on(async move { ask_question(handler, question).await }) +} \ No newline at end of file diff --git a/rust/agama-software/src/callbacks/commit_download.rs b/rust/agama-software/src/callbacks/commit_download.rs index 3f501a0f2a..e45052021c 100644 --- a/rust/agama-software/src/callbacks/commit_download.rs +++ b/rust/agama-software/src/callbacks/commit_download.rs @@ -8,6 +8,8 @@ use gettextrs::gettext; use tokio::runtime::Handle; use zypp_agama::callbacks::pkg_download::{Callback, DownloadError}; +use crate::callbacks::ask_software_question; + #[derive(Clone)] pub struct CommitDownload { progress: Handler, @@ -46,16 +48,14 @@ impl Callback for CommitDownload { ("Ignore", labels[1].as_str()), ]; let error_str = error.to_string(); - let data = [ - ("package", name.as_str()), - ("error_code", error_str.as_str()), - ]; let question = QuestionSpec::new(description.as_str(), "software.package_error.provide_error") .with_actions(&actions) - .with_data(&data); - let result = Handle::current() - .block_on(async move { ask_question(&self.questions, question).await }); + .with_data(&[ + ("package", name.as_str()), + ("error_code", error_str.as_str()), + ]); + let result = ask_software_question(&self.questions, question); let Ok(answer) = result else { tracing::warn!("Failed to ask question {:?}", result); return zypp_agama::callbacks::ProblemResponse::ABORT; diff --git a/rust/agama-software/src/callbacks/security.rs b/rust/agama-software/src/callbacks/security.rs index cd3291644c..8cb9d95297 100644 --- a/rust/agama-software/src/callbacks/security.rs +++ b/rust/agama-software/src/callbacks/security.rs @@ -33,10 +33,9 @@ impl security::Callback for Security { and integrity of the file cannot be verified. Use it anyway?" ) }; - let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.unsigned_file") .with_yes_no_actions() - .with_data(&data); + .with_data(&[("filename", file.as_str())]); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -62,14 +61,13 @@ impl security::Callback for Security { ); let labels = [gettext("Trust"), gettext("Skip")]; let actions = [("Trust", labels[0].as_str()), ("Skip", labels[1].as_str())]; - let data = [ - ("id", key_id.as_str()), - ("name", key_name.as_str()), - ("fingerprint", key_fingerprint.as_str()), - ]; let question = QuestionSpec::new(&text, "software.import_gpg") .with_actions(&actions) - .with_data(&data) + .with_data(&[ + ("id", key_id.as_str()), + ("name", key_name.as_str()), + ("fingerprint", key_fingerprint.as_str()), + ]) .with_default_action("Skip"); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); @@ -98,10 +96,9 @@ impl security::Callback for Security { the following unknown GnuPG key: {key_id}. Use it anyway?" ) }; - let data = [("filename", file.as_str()), ("id", key_id.as_str())]; let question = QuestionSpec::new(&text, "software.unknown_gpg") .with_yes_no_actions() - .with_data(&data); + .with_data(&[("filename", file.as_str()), ("id", key_id.as_str())]); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { @@ -135,10 +132,9 @@ impl security::Callback for Security { Use it anyway?" ) }; - let data = [("filename", file.as_str())]; let question = QuestionSpec::new(&text, "software.verification_failed") .with_yes_no_actions() - .with_data(&data); + .with_data(&[("filename", file.as_str())]); let result = Handle::current() .block_on(async move { ask_question(&self.questions, question).await }); let Ok(answer) = result else { diff --git a/rust/agama-software/src/zypp_server.rs b/rust/agama-software/src/zypp_server.rs index 7be49286b6..4da749eb40 100644 --- a/rust/agama-software/src/zypp_server.rs +++ b/rust/agama-software/src/zypp_server.rs @@ -35,7 +35,7 @@ use tokio::sync::{ use zypp_agama::ZyppError; use crate::{ - callbacks::{commit_download, security}, + callbacks, model::state::{self, SoftwareState}, }; @@ -177,7 +177,7 @@ impl ZyppServer { question, tx, } => { - let mut security_callback = security::Security::new(question); + let mut security_callback = callbacks::Security::new(question); self.write(state, progress, &mut security_callback, tx, zypp) .await?; } @@ -186,8 +186,8 @@ impl ZyppServer { } SoftwareAction::Install(tx, progress, question) => { let mut download_callback = - commit_download::CommitDownload::new(progress, question.clone()); - let mut security_callback = security::Security::new(question); + callbacks::CommitDownload::new(progress, question.clone()); + let mut security_callback = callbacks::Security::new(question); tx.send(self.install(zypp, &mut download_callback, &mut security_callback)) .map_err(|_| ZyppDispatchError::ResponseChannelClosed)?; } @@ -205,8 +205,8 @@ impl ZyppServer { fn install( &self, zypp: &zypp_agama::Zypp, - download_callback: &mut commit_download::CommitDownload, - security_callback: &mut security::Security, + download_callback: &mut callbacks::CommitDownload, + security_callback: &mut callbacks::Security, ) -> ZyppServerResult { let target = "/mnt"; zypp.switch_target(target)?; @@ -242,7 +242,7 @@ impl ZyppServer { &self, state: SoftwareState, progress: Handler, - security: &mut security::Security, + security: &mut callbacks::Security, tx: oneshot::Sender>>, zypp: &zypp_agama::Zypp, ) -> Result<(), ZyppDispatchError> { diff --git a/rust/agama-utils/src/api/question.rs b/rust/agama-utils/src/api/question.rs index bd70aea016..3f4ebd7196 100644 --- a/rust/agama-utils/src/api/question.rs +++ b/rust/agama-utils/src/api/question.rs @@ -243,7 +243,7 @@ impl QuestionSpec { /// will be localized. /// /// This method sets the default action to "No". If you need a different - /// default, you can override it with a subsequent call to `with_default_action()`. + /// default, you can override it with a subsequent call to [QuestionSpec::with_default_action]. /// /// # Example /// @@ -251,6 +251,12 @@ impl QuestionSpec { /// use agama_utils::api::question::QuestionSpec; /// let q = QuestionSpec::new("Continue?", "q.continue").with_yes_no_actions(); /// assert_eq!(q.default_action.as_deref(), Some("No")); + /// assert_eq!(q.actions.len(), 2); + /// assert_eq!(q.actions[0].id, "Yes"); + /// assert_eq!(q.actions[1].id, "No"); + /// // localized labels + /// assert_eq!(q.actions[0].label, "Yes"); + /// assert_eq!(q.actions[1].label, "No"); /// ``` pub fn with_yes_no_actions(self) -> Self { // we have to always call fresh gettext to ensure it is properly localized after change of locale diff --git a/rust/agama-utils/src/question.rs b/rust/agama-utils/src/question.rs index dfd539056a..03e0f3a85c 100644 --- a/rust/agama-utils/src/question.rs +++ b/rust/agama-utils/src/question.rs @@ -81,25 +81,25 @@ pub enum AskError { /// Asks a question and waits until it is answered. /// /// This is a helper function for internal Rust services that have a direct handler to the -/// `question::Service` and need to ask a question and wait for the answer in a +/// [question::Service] and need to ask a question and wait for the answer in a /// synchronous-like manner within an `async` context. It simplifies the process by /// abstracting away the need to listen for events or poll the API, which would /// typically be done by an external client (like a web UI). /// -/// The function sends the question to the `question::Service` and then polls for an +/// The function sends the question to the [question::Service] and then polls for an /// answer. Once the question is answered (either by a user, a pre-configured rule, /// or a default policy), the function returns the answer and deletes the question /// from the service to clean up. /// /// # Arguments -/// * `handler` - A handler to the `question::Service`. -/// * `question` - The `QuestionSpec` defining the question to be asked. +/// * `handler` - A handler to the [question::Service]. +/// * `question` - The [QuestionSpec] defining the question to be asked. /// /// # Errors /// This function can return the following errors: -/// * `AskError::QuestionNotFound`: If the question is deleted by another process before it can be answered. -/// * `AskError::Service`: If there is an error within the `question::Service` actor. -/// * `AskError::Actor`: If there is a communication error with the actor system (e.g., the actor task has terminated). +/// * [AskError::QuestionNotFound]: If the question is deleted by another process before it can be answered. +/// * [AskError::Service]: If there is an error within the [question::Service] actor. +/// * [AskError::Actor]: If there is a communication error with the actor system (e.g., the actor task has terminated). pub async fn ask_question( handler: &Handler, question: QuestionSpec, From 6942f30cad57db120ad498a1e7b56305301a7449 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 24 Nov 2025 14:35:40 +0100 Subject: [PATCH 6/7] cargo fmt --- rust/agama-software/src/callbacks.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/rust/agama-software/src/callbacks.rs b/rust/agama-software/src/callbacks.rs index 53fa21c97a..69fca64c43 100644 --- a/rust/agama-software/src/callbacks.rs +++ b/rust/agama-software/src/callbacks.rs @@ -1,15 +1,17 @@ mod commit_download; -use agama_utils::{actor::Handler, api::question::{Answer, QuestionSpec}, question::{self, AskError, ask_question}}; +use agama_utils::{ + actor::Handler, + api::question::{Answer, QuestionSpec}, + question::{self, ask_question, AskError}, +}; pub use commit_download::CommitDownload; mod security; pub use security::Security; use tokio::runtime::Handle; - fn ask_software_question( handler: &Handler, question: QuestionSpec, ) -> Result { - Handle::current() - .block_on(async move { ask_question(handler, question).await }) -} \ No newline at end of file + Handle::current().block_on(async move { ask_question(handler, question).await }) +} From e8e5049a2c6903011d26ce331ffb58eefac59d8c Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 24 Nov 2025 14:52:48 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Martin Vidner --- rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h b/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h index fb86391b34..01f0cd6222 100644 --- a/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h +++ b/rust/zypp-agama/zypp-agama-sys/c-layer/include/callbacks.h @@ -137,7 +137,7 @@ struct DownloadResolvableCallbacks { /** * @brief What to do with an unknown GPG key. - * @see KeyRingReport::KeyTrust in libzypp + * @see zypp::KeyRingReport::KeyTrust in https://github.com/openSUSE/libzypp/blob/master/zypp-logic/zypp/KeyRing.h */ enum GPGKeyTrust { /** Reject the key. */ @@ -158,6 +158,7 @@ enum GPGKeyTrust { * an empty string if not available. * @param user_data User-defined data. * @return A GPGKeyTrust value indicating the action to take. + * @see zypp::KeyRingReport::askUserToAcceptKey in https://github.com/openSUSE/libzypp/blob/master/zypp-logic/zypp/KeyRing.h */ typedef enum GPGKeyTrust (*GPGAcceptKeyCallback)(const char *key_id, const char *key_name, @@ -201,6 +202,9 @@ typedef bool (*GPGVerificationFailed)(const char *file, const char *key_id, const char *key_fingerprint, const char *repository_alias, void *user_data); +/** + * @see zypp::DigestReport in https://github.com/openSUSE/libzypp/blob/master/zypp-logic/zypp/Digest.h + */ typedef bool (*ChecksumMissing)(const char *file, void *user_data); typedef bool (*ChecksumWrong)(const char *file, const char *expected, const char *actual, void *user_data);