From 26455c7e1687dfbbbfc9294a14112c22c41f3a64 Mon Sep 17 00:00:00 2001 From: Dmitrii Okunev Date: Wed, 11 Nov 2026 09:12:20 +0000 Subject: [PATCH 01/56] feat(hii): keyword set of STRING questions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `hii keyword set` refused every STRING before its VALUE. It now takes one as `set` takes its text. SetData() writes a VALUE as given: edk2-stable202608 ConfigKeywordHandler.c takes a STRING as wide as its MaxSize CHAR16s (GetWidth :1643-1644) and appends the VALUE (:2248), and ConfigRouting.c HiiConfigToBlock reads its digits last first (GetValueOfNumber :518) and copies that many bytes (:6353). So a STRING's VALUE holds all MaxSize CHAR16s, last byte first, as `hii keyword get` prints them: "AB" of MaxSize 4 is 0000000000420041. That is edk2's order, which keyword get and set already take for the other kinds, and the reverse of UEFI 2.11 §35.2.1.5, which lists a 's bytes in memory order; the README names it. check_set takes the text that the VALUE holds up to its first NULL, as get reads it, and refuses a UTF-16 surrogate there. It writes that text with set's StringInput::write and string_own_write, so set's refusals apply: MinSize, MaxSize, whose last CHAR16 holds the NULL, a line break in a single-line STRING, the Private Use Area. A VALUE that is not the bytes that set writes for the text, NULL-padded (UEFI 2.11 §33.2.5.4.9), such as one with a character after the NULL, fails check_set's existing comparison, and one of another width its GetWidth check. UEFI 2.11 §33.2.5.4.9 represents a STRING's result as its characters and the terminating NULL, but a VALUE of fewer bytes than MaxSize CHAR16s is refused: ConfigRouting.c HiiConfigToBlock copies the question's width (:6353) from a buffer of half the VALUE's digits (GetValueOfNumber :508-509), which the firmware would read past; the README says so. The refusals that the other kinds share apply too: EFI_IFR_FLAG_CALLBACK, a WRITE expression, storage that no variable holds, &READONLY, a read-only question, the bytes of a password; a declared setup password needs --i-am-fine-to-brick-this-system. The internal-inconsistency refusal of an ORDERED_LIST whose description holds another TextInput now serves a STRING too, and names both TextInputs by a TextInputKind. Test Plan: - RED: these tests on the parent's code, 6 failed (1479 passed), each on "uefisettings does not yet implement keyword set of EFI_IFR_STRING": check_set_takes_a_string_value_as_set_takes_its_text, check_set_refuses_a_string_value_that_set_refuses_or_would_not_write, check_set_refuses_a_string_over_the_bytes_of_a_password, check_get_takes_string_date_time_and_ordered_list_but_not_password, set_of_a_string_with_a_declared_password_needs_the_flag and set_refuses_a_string_beside_a_password_of_var_store_id_0. - strings_set_by_keyword_as_by_prompt is a characterization test, which passes on the parent too: `set --keyword` of a STRING already writes its text as `set` by prompt does, NULL-padded, and nothing for a text that set refuses. - keywords.rs: check_set_takes_a_string_value_as_set_takes_its_text takes no character, "AB", MaxSize - 1 characters, a line feed in a multi-line STRING, a Common Control Code and " A ", its spaces kept. check_set_refuses_a_string_value_that_set_refuses_or_would_not_write: refused are 4 characters, fewer than MinSize, a line feed in a single-line STRING, U+E000, a surrogate, a character after the NULL, fewer bytes than MaxSize CHAR16s, a read-only STRING, EFI_IFR_FLAG_CALLBACK, &READONLY, a WRITE expression and name/value storage; the last 4 pin refusals that the parent makes before the kind as well. check_set_refuses_a_string_over_the_bytes_of_a_password: refused where its last CHAR16 overlaps a password, taken beside it. check_get_takes_string_date_time_and_ordered_list_but_not_password: set of a STRING's one-byte VALUE is now refused as not as wide as its question. - keyword_access.rs: set_of_a_string_with_a_declared_password_needs_the_flag replaces set_refuses_a_string_before_the_setup_password: refused without the flag, before any call; U+E000 refused with set's error with and without it, before the setup password's refusal; with it, one SetData() of the VALUE, a GetData() that reads it back, and the setup password lifted. set_refuses_a_string_beside_a_password_of_var_store_id_0: beside a PASSWORD of VarStoreId 0, EFI_IFR_FLAG_CALLBACK or not, set of a STRING with the flag is refused with that PASSWORD's record through variables of either reach, with no variable read and no call, as the parent refuses it; beside a PASSWORD on Store that ends before it, one SetData() writes the VALUE, which the parent refuses. Its PASSWORDs come from flagged_password_on, a new fixture of ifr_fixtures.rs: password_on with question Flags. - cargo test --lib: 1485 passed. - 15 mutants, at the next commit's tree, whose src/ is this commit's; 14 fail named tests: the VALUE's bytes written as given, the text dropped, a surrogate taken as no text, the width check or the comparison skipped for a STRING, GetWidth one CHAR16 wider, &READONLY or EFI_IFR_FLAG_CALLBACK taken for a STRING, EFI_IFR_FLAG_CALLBACK taken on every set, U+2028 taken in a single-line STRING, the text trimmed, or trimmed at its end, the setup password refused before the keywords' checks, and check_question's refusal of a WRITE expression skipped for a STRING, which string_own_write then refuses with its own error. The 15th has string_write pass VariableReach::Runtime, which only a question without a VarStore reads (QuestionDescriptor::store), and check_question refuses VarStoreId 0 first: it changes nothing. Signed-off-by: Dmitrii Okunev --- README.md | 9 +- src/lib/exports.rs | 57 +++++ src/lib/hii/forms.rs | 41 +++- src/lib/hii/forms/ifr_fixtures.rs | 13 +- src/lib/hii/forms/keywords.rs | 325 ++++++++++++++++++++++++--- src/lib/hii/forms/string_question.rs | 2 +- src/lib/hii/forms/write_plan.rs | 2 +- src/lib/hii/keyword_access.rs | 165 ++++++++++++-- 8 files changed, 544 insertions(+), 70 deletions(-) diff --git a/README.md b/README.md index d2d5ed2..f989b10 100644 --- a/README.md +++ b/README.md @@ -904,7 +904,14 @@ ORDERED_LIST each container, whose options up to the first empty one `set` takes every container after it empty. A container is as wide as the Type of the ONE_OF_OPTION right after the ORDERED_LIST, as edk2's GetWidth takes it, where UEFI 2.11 §33.2.5.4.8 gives it one byte; a VALUE is refused where `set` writes containers of another width, that of the first option -that is no default. A REF, whose EFI_HII_REF `set` does not write, and a STRING are refused. +that is no default. A REF, whose EFI_HII_REF `set` does not write, is refused. Of a STRING, a +VALUE holds all MaxSize CHAR16s, as edk2's GetWidth takes them, so `AB` of MaxSize 4 is +`0000000000420041`; UEFI 2.11 §33.2.5.4.9 represents a STRING's result as "hex dump of the +string, including the terminating NULL character", but a VALUE of fewer bytes is refused, as +edk2-stable202608 ConfigRouting.c HiiConfigToBlock copies the question's width (:6353) from a +buffer of half the VALUE's digits (:508-509): `set`'s checks take its text up to the first NULL, +and a VALUE that is not the bytes that `set` writes for that text, NULL-padded (UEFI 2.11 +§33.2.5.4.9), or that holds a UTF-16 surrogate is refused. ```plain FS0:\> uefisettings.efi hii keyword get --namespace x-UEFI-ns KEYWORD= diff --git a/src/lib/exports.rs b/src/lib/exports.rs index f0fdb23..546b6f9 100644 --- a/src/lib/exports.rs +++ b/src/lib/exports.rs @@ -12901,6 +12901,63 @@ mod tests { } } + /// set --keyword selects a STRING by its keyword as set by its prompt does, and writes its + /// text with set's checks: NULL-padded to MaxSize CHAR16s, and nothing for a text that set + /// refuses. + #[test] + fn strings_set_by_keyword_as_by_prompt() { + let string = flagged_string_on(1, 1, (1, 0), (0, 4), (0x00, 0x00), &[]); + let packages = [ + form_package(&[var_store_efi(1, "Setup", NV_BS_RT, 8), form(1, &[string])]), + en_strings_package_of(&["S"]), + strings_package_of(b"x-UEFI-ns", &["KString"]), + END_PACKAGE.to_vec(), + ]; + let hiidb = package_list_with_guid([0x62; 16], &packages.concat()); + let set = |text: &str, variables: &Rc| { + let by_keyword = QuestionSelector::Keyword { + keyword: "KString".parse().expect("parse the keyword"), + namespace: None, + }; + set_responses( + &hiidb, + by_keyword, + &SetValue::Text(text.to_owned()), + None, + &SetOptions::default(), + shared(variables), + &**variables, + None, + ) + .map(drop) + .map_err(|error| format!("{error:#}")) + }; + + let variables = setup_holding(&[0; 8]); + assert_eq!(set("AB", &variables), Ok(()), "set AB"); + assert_eq!( + variables.writes_to(&setup()), + [(0, vec![0x41, 0, 0x42, 0, 0, 0, 0, 0])], + "writes of AB" + ); + + let refused = setup_holding(&[0; 8]); + assert_eq!( + set("ABCD", &refused), + Err( + "cannot set question \"S\": the STRING takes at most 3 characters, its last \ + CHAR16 of MaxSize 4 holding the NULL (UEFI 2.11 §33.2.5.4.9), not 4" + .to_owned() + ), + "set ABCD" + ); + assert_eq!( + refused.writes_to(&setup()), + Vec::<(u16, Vec)>::new(), + "writes of ABCD" + ); + } + /// set of a STRING through `hiidb`'s variables with `value`, as the CLI prints an error, and /// what it wrote to Setup, which held `stored`. fn string_set(hiidb: &[u8], value: &SetValue, stored: &[u8]) -> SetOutcome { diff --git a/src/lib/hii/forms.rs b/src/lib/hii/forms.rs index 3185c2c..9f9e969 100644 --- a/src/lib/hii/forms.rs +++ b/src/lib/hii/forms.rs @@ -3516,19 +3516,44 @@ enum TextInput { } impl TextInput { - /// The name of its variant, which an internal inconsistency names. - fn kind(&self) -> &'static str { + /// Its variant, which an internal inconsistency names. + fn kind(&self) -> TextInputKind { match self { - Self::Range(_) => "Range", - Self::Maximum(_) => "Maximum", - Self::DateTime(_) => "DateTime", - Self::OrderedList(_) => "OrderedList", - Self::CrossReference => "CrossReference", - Self::String(_) => "String", + Self::Range(_) => TextInputKind::Range, + Self::Maximum(_) => TextInputKind::Maximum, + Self::DateTime(_) => TextInputKind::DateTime, + Self::OrderedList(_) => TextInputKind::OrderedList, + Self::CrossReference => TextInputKind::CrossReference, + Self::String(_) => TextInputKind::String, } } } +/// The variant of a TextInput. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum TextInputKind { + Range, + Maximum, + DateTime, + OrderedList, + CrossReference, + String, +} + +impl fmt::Display for TextInputKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let name = match self { + Self::Range => "Range", + Self::Maximum => "Maximum", + Self::DateTime => "DateTime", + Self::OrderedList => "OrderedList", + Self::CrossReference => "CrossReference", + Self::String => "String", + }; + write!(f, "TextInput::{name}") + } +} + #[derive(Debug)] pub struct AnswerOption { /// The option's text, as get shows it diff --git a/src/lib/hii/forms/ifr_fixtures.rs b/src/lib/hii/forms/ifr_fixtures.rs index f4e1864..2892a27 100644 --- a/src/lib/hii/forms/ifr_fixtures.rs +++ b/src/lib/hii/forms/ifr_fixtures.rs @@ -790,9 +790,20 @@ pub(crate) fn password_on( prompt: u16, var_store_id: u16, offset: u16, +) -> Vec { + flagged_password_on(question_id, prompt, var_store_id, offset, 0x00) +} + +/// password_on, with the question Flags `flags`: 0x04 is EFI_IFR_FLAG_CALLBACK. +pub(crate) fn flagged_password_on( + question_id: u16, + prompt: u16, + var_store_id: u16, + offset: u16, + flags: u8, ) -> Vec { let data = [ - question_header_on(question_id, prompt, var_store_id, offset), + question_header_with(question_id, prompt, var_store_id, offset, flags), [&1u16.to_le_bytes()[..], &20u16.to_le_bytes()].concat(), // MinSize, MaxSize ] .concat(); diff --git a/src/lib/hii/forms/keywords.rs b/src/lib/hii/forms/keywords.rs index fafe47c..5fdef94 100644 --- a/src/lib/hii/forms/keywords.rs +++ b/src/lib/hii/forms/keywords.rs @@ -64,9 +64,12 @@ use super::set::own_write; use super::set::ChangeValueError; use super::set::NewValue; use super::set::VarStoreWrite; +use super::string_question::stored_text; +use super::string_question::StringInput; use super::try_walk_scopes; use super::walk_scopes; use super::write_plan::list_own_write; +use super::write_plan::string_own_write; use super::AnswerOption; use super::Choice; use super::DeclaredStore; @@ -88,6 +91,7 @@ use super::ReadOnlyWriter; use super::SelectionError; use super::ShortVarStoreEfi; use super::TextInput; +use super::TextInputKind; use super::TimeStorage; use super::TypeValue; use super::Walk; @@ -246,17 +250,18 @@ pub(crate) enum KeywordRefusal { storage_width: usize, question: String, }, - /// An ORDERED_LIST whose description holds no ListInput, which describe_question always - /// gives it (handle_ordered_list) + /// An ORDERED_LIST or STRING whose description holds another TextInput than its own, which + /// describe_question always gives it (handle_ordered_list, handle_string) #[error( - "keyword {keyword}: internal inconsistency: question \"{question}\" is an \ - EFI_IFR_ORDERED_LIST, but its description holds set's TextInput::{input}, not a \ - ListInput" + "keyword {keyword}: internal inconsistency: question \"{question}\" is an {op}, but its \ + description holds set's {input}, not {expected}" )] - ListInputMismatch { + InputMismatch { keyword: String, question: String, - input: &'static str, + op: &'static str, + input: TextInputKind, + expected: TextInputKind, }, #[error("keyword {keyword}: VALUE={value} {reason} question \"{question}\"")] Value { @@ -829,10 +834,10 @@ impl<'a> KeywordQuestion<'a> { /// `include`, `volatile` and the variables of `reader`: the question's state, whether it is /// read-only, which no `include` lifts, its option of the VALUE or the VALUE's range, the /// fields of a DATE or TIME (DateTimeInput::number_value), the items of an ORDERED_LIST - /// (list_write), the EFI_HII_REF of a REF, which set does not write, the variable's, and - /// whether the bytes written overlap those that `reader` withholds, which hold a password - /// (ChangeValueError::PasswordBytes); and refuses a VALUE that is not the bytes that set - /// writes. Returns what SetData() writes. + /// (list_write), the text of a STRING (string_write), the EFI_HII_REF of a REF, which set + /// does not write, the variable's, and whether the bytes written overlap those that `reader` + /// withholds, which hold a password (ChangeValueError::PasswordBytes); and refuses a VALUE + /// that is not the bytes that set writes. Returns what SetData() writes. pub(crate) fn check_set( &self, resp: &KeywordResp, @@ -848,10 +853,6 @@ impl<'a> KeywordQuestion<'a> { let Some(listed) = ListedQuestion::of(&op) else { return Err(self.unchecked(&op).into()); }; - // set takes a STRING, but keyword set takes none yet: refused before its VALUE - if let ListedQuestion::String(_) = listed { - return Err(self.unchecked(&op).into()); - } let prompt = self.texts.display(header.prompt_string_id); if keyword_width(&op) != Some(resp.value.as_bytes().len()) { return Err(self.value_refusal(resp, prompt, ValueRefusal::Width).into()); @@ -865,15 +866,20 @@ impl<'a> KeywordQuestion<'a> { } ListedQuestion::OrderedList(_) => { let TextInput::OrderedList(input) = &descriptor.input else { - return Err(KeywordRefusal::ListInputMismatch { - keyword: self.keyword.to_string(), - question: descriptor.question.clone(), - input: descriptor.input.kind(), - } - .into()); + return Err(self + .input_mismatch(&op, &descriptor, TextInputKind::OrderedList) + .into()); }; Some(self.list_write(input, &descriptor, resp, prompt, include, reader)?) } + ListedQuestion::String(_) => { + let TextInput::String(input) = &descriptor.input else { + return Err(self + .input_mismatch(&op, &descriptor, TextInputKind::String) + .into()); + }; + Some(self.string_write(input, &descriptor, resp, prompt, reader)?) + } // as set refuses it, after its choice (plan_question) ListedQuestion::Ref(_) => { return Err(ChangeValueError::CrossReference { @@ -881,8 +887,6 @@ impl<'a> KeywordQuestion<'a> { }) .with_context(|| format!("keyword {}", self.keyword)); } - // refused before its VALUE, above - ListedQuestion::String(_) => return Err(self.unchecked(&op).into()), }; let write = write.ok_or_else(|| self.question_refusal(&header, QuestionRefusal::Storageless))?; @@ -1514,6 +1518,29 @@ impl<'a> KeywordQuestion<'a> { .with_context(|| self.value_context(resp, prompt)) } + /// set's write of `resp`, a keyword VALUE of every CHAR16 of the STRING of `descriptor`, of + /// `input`, prompted by `prompt`, through variables of `reader`: the text of the VALUE up to + /// its first NULL, as get reads it (stored_text), written as set writes a text + /// (StringInput::write, string_own_write), so that set's refusals of it apply. Refuses a VALUE + /// that holds a UTF-16 surrogate there, which get does not read either. + fn string_write<'d>( + &self, + input: &StringInput, + descriptor: &'d QuestionDescriptor, + resp: &KeywordResp, + prompt: &str, + reader: &EfivarReader, + ) -> Result> { + let text = + stored_text(resp.value.as_bytes()).with_context(|| self.value_context(resp, prompt))?; + let write = input + .write(&text) + .map_err(|refusal| refusal.of(descriptor)) + .with_context(|| self.value_context(resp, prompt))?; + string_own_write(descriptor, write, reader.reach()) + .with_context(|| self.value_context(resp, prompt)) + } + /// What names `resp`, the keyword's VALUE, for the question prompted by `prompt`, in the errors /// of set's checks of it. fn value_context(&self, resp: &KeywordResp, prompt: &str) -> String { @@ -1575,6 +1602,23 @@ impl<'a> KeywordQuestion<'a> { } } + /// The refusal of the question `op` of `descriptor`, whose description holds another + /// TextInput than its `expected` one. + fn input_mismatch( + &self, + op: &IFROperation, + descriptor: &QuestionDescriptor, + expected: TextInputKind, + ) -> KeywordRefusal { + KeywordRefusal::InputMismatch { + keyword: self.keyword.to_string(), + question: descriptor.question.clone(), + op: op.op_code.spec_name(), + input: descriptor.input.kind(), + expected, + } + } + fn question_refusal(&self, header: &QuestionHeader, reason: QuestionRefusal) -> KeywordRefusal { KeywordRefusal::Question { keyword: self.keyword.to_string(), @@ -1838,6 +1882,7 @@ mod tests { use super::super::ifr_fixtures::conditional; use super::super::ifr_fixtures::eq_id_val_of; use super::super::ifr_fixtures::flagged_option; + use super::super::ifr_fixtures::flagged_string_on; use super::super::ifr_fixtures::form; use super::super::ifr_fixtures::form_set; use super::super::ifr_fixtures::form_set_with_guid; @@ -2821,8 +2866,8 @@ mod tests { } /// GetData() reads a STRING, DATE, TIME or ORDERED_LIST as stored, which check_get takes; - /// check_set refuses a STRING, as keyword set of it is not implemented yet, and a VALUE of a - /// byte for the others, which it takes as wide as GetWidth; both refuse a PASSWORD. + /// check_set refuses a VALUE of a byte for each, which it takes as wide as GetWidth; both + /// refuse a PASSWORD. #[test] fn check_get_takes_string_date_time_and_ordered_list_but_not_password() { let question = |op_code: u8| { @@ -2833,12 +2878,12 @@ mod tests { .concat(); operation(op_code, false, &data) }; - let unchecked = |op: &'static str| KeywordRefusal::Unchecked { - keyword: KEYWORD.to_owned(), - op, - }; for (op_code, op, refusal) in [ - (STRING, "EFI_IFR_STRING", unchecked("EFI_IFR_STRING")), + ( + STRING, + "EFI_IFR_STRING", + value_refusal("03", ValueRefusal::Width), + ), ( DATE, "EFI_IFR_DATE", @@ -2871,10 +2916,6 @@ mod tests { "{op}: set" ); } - assert_eq!( - unchecked("EFI_IFR_STRING").to_string(), - format!("keyword {KEYWORD}: uefisettings does not yet implement keyword set of EFI_IFR_STRING") - ); let password = password_on(1, PROMPT, 1, 5); let lists = [list(PATH, &[(NAMESPACE, KEYWORD)], password.clone())]; assert_eq!( @@ -3558,6 +3599,222 @@ mod tests { ); } + /// A STRING of MaxSize 4, of MinSize `min_size`, question Flags `question_flags` and STRING + /// Flags `flags`, prompted by PROMPT at OFFSET of VarStore 1. + fn string_of(min_size: u8, question_flags: u8, flags: u8) -> Vec { + flagged_string_on( + 1, + PROMPT, + (1, 5), + (min_size, 4), + (question_flags, flags), + &[], + ) + } + + /// SetData() writes a STRING's VALUE, its MaxSize CHAR16s last byte first, as given, so + /// check_set takes the text that the VALUE holds up to its first NULL, as get reads it, as + /// set takes that text, where the VALUE is the bytes that set writes for it: NULL-padded. + #[test] + fn check_set_takes_a_string_value_as_set_takes_its_text() { + const MULTI_LINE: u8 = 0x01; + let string = OFFSET..OFFSET + 8; + for (name, ops, value) in [ + ("no character", string_of(0, 0, 0), "0000000000000000"), + ("AB", string_of(0, 0, 0), "0000000000420041"), + ( + "MaxSize - 1 characters", + string_of(0, 0, 0), + "0000004300420041", + ), + ( + "a line feed, multi-line", + string_of(0, 0, MULTI_LINE), + "00000042000a0041", + ), + ( + "a Common Control Code", + string_of(0, 0, 0), + "000000000000f620", + ), + ("spaces", string_of(0, 0, 0), "0000002000410020"), + ] { + assert_eq!( + keyword_set(ops, value, &[0; 8]), + Ok(vec![string.clone()]), + "{name}" + ); + } + } + + /// check_set refuses a STRING's VALUE whose text set refuses, with set's refusal, one whose + /// text holds a UTF-16 surrogate, which get does not read either, one that is not the bytes + /// that set writes for its text, one of fewer bytes than MaxSize CHAR16s (GetWidth), a + /// read-only STRING, one of EFI_IFR_FLAG_CALLBACK, one whose has &READONLY, one + /// with a WRITE expression, and one of name/value storage, which no variable holds. + #[test] + fn check_set_refuses_a_string_value_that_set_refuses_or_would_not_write() { + use super::super::string_question::StringRefusal; + use std::num::NonZeroU8; + let refused = |value: &str, refusal: StringRefusal| { + value_error( + value, + ChangeValueError::String { + question: QUESTION.to_owned(), + refusal, + }, + ) + }; + let max_size = NonZeroU8::new(4).expect("MaxSize 4"); + let private_use = char::from_u32(0xE000).expect("U+E000"); + let cases = [ + ( + "no NULL", + string_of(0, 0, 0), + "0044004300420041", + refused( + "0044004300420041", + StringRefusal::TooLong { max_size, given: 4 }, + ), + ), + ( + "fewer characters than MinSize", + string_of(2, 0, 0), + "0000000000000041", + refused( + "0000000000000041", + StringRefusal::TooShort { min: 2, given: 1 }, + ), + ), + ( + "a line feed, single-line", + string_of(0, 0, 0), + "00000042000a0041", + refused("00000042000a0041", StringRefusal::LineBreak), + ), + ( + "the Private Use Area", + string_of(0, 0, 0), + "000000000000e000", + refused("000000000000e000", StringRefusal::PrivateUse(private_use)), + ), + ( + "a surrogate", + string_of(0, 0, 0), + "000000000000d800", + value_error( + "000000000000d800", + "its character 0 is CHAR16 0xd800, a UTF-16 surrogate, which UCS-2 has no \ + character for (UEFI 2.11 §33.2.6.2.2)", + ), + ), + ( + "a character after the NULL", + string_of(0, 0, 0), + "0000004200000041", + Err(KeywordRefusal::Unwritten { + keyword: KEYWORD.to_owned(), + value: "0000004200000041".to_owned(), + written: "0000000000000041 at VarOffset 5".to_owned(), + question: QUESTION.to_owned(), + } + .to_string()), + ), + ( + "fewer bytes than MaxSize CHAR16s", + string_of(0, 0, 0), + "00420041", + Err(value_refusal("00420041", ValueRefusal::Width).to_string()), + ), + ( + "read-only", + string_of(0, QuestionFlags::READ_ONLY.bits(), 0), + "0000000000420041", + Err(format!( + "keyword {KEYWORD}: cannot set question {QUESTION:?}: EFI_IFR_FLAG_READ_ONLY \ + makes it read-only (UEFI 2.11 §33.3.8.2.2)" + )), + ), + ( + "EFI_IFR_FLAG_CALLBACK", + string_of(0, QuestionFlags::CALLBACK.bits(), 0), + "0000000000420041", + Err(question_refusal(QuestionRefusal::Callback).to_string()), + ), + ( + "&READONLY", + string_of(0, 0, 0), + "0000000000420041&READONLY", + Err(KeywordRefusal::ReadOnlyResp(KEYWORD.to_owned()).to_string()), + ), + ( + "a WRITE expression", + flagged_string_on( + 1, + PROMPT, + (1, 5), + (0, 4), + (0x00, 0x00), + &[operation(WRITE, true, &[]), uint8(1), END.to_vec()], + ), + "0000000000420041", + Err(question_refusal(QuestionRefusal::Write).to_string()), + ), + ]; + for (name, ops, value, want) in cases { + assert_eq!(keyword_set(ops, value, &[0; 8]), want, "{name}"); + } + let name_value = [ + operation( + VAR_STORE_NAME_VALUE, + false, + &[2_u16.to_le_bytes().to_vec(), vec![0x55; 16]].concat(), + ), + flagged_string_on(1, PROMPT, (2, 7), (0, 4), (0x00, 0x00), &[]), + ] + .concat(); + assert_eq!( + keyword_set(name_value, "0000000000420041", &[0; 8]), + Err(format!( + "keyword {KEYWORD}: question {QUESTION:?} has no VarStore that uefisettings reads: \ + HII name/value varstore requires EFI_HII_CONFIG_ACCESS_PROTOCOL" + )), + "name/value storage" + ); + } + + /// SetData() writes every byte of a STRING's MaxSize CHAR16s, so check_set refuses a STRING + /// whose CHAR16s overlap the bytes of a password, which the reader withholds, at their last + /// byte, and takes one beside them. + #[test] + fn check_set_refuses_a_string_over_the_bytes_of_a_password() { + let lists = [list(PATH, &[(NAMESPACE, KEYWORD)], string_of(0, 0, 0))]; + let question = question_of(&lists); + let set = |password: Range| { + question + .check_set( + &resp("0000000000420041"), + IncludedStates::empty(), + VolatileWrites::Refuse, + &mut withholding(password), + ) + .map(|write| write.bytes) + .map_err(|error| format!("{error:#}")) + }; + + assert_eq!( + set(OFFSET + 7..OFFSET + 9), + Err(format!( + "keyword {KEYWORD}: cannot set question {QUESTION:?}: EFI variable {} holds a \ + password at bytes 12..14, which set never writes", + store_key() + )), + "the last byte" + ); + let string = OFFSET..OFFSET + 8; + assert_eq!(set(OFFSET + 8..OFFSET + 10), Ok(vec![string]), "beside"); + } + /// SetData() writes every byte of a question's storage, so check_set refuses a write over /// those of a password, which the reader withholds, as set does, and takes one beside them. #[test] diff --git a/src/lib/hii/forms/string_question.rs b/src/lib/hii/forms/string_question.rs index eee4c8b..780d93e 100644 --- a/src/lib/hii/forms/string_question.rs +++ b/src/lib/hii/forms/string_question.rs @@ -135,7 +135,7 @@ fn stored_units(bytes: &[u8]) -> impl Iterator + '_ { } /// The text of `bytes`, the CHAR16s of a STRING that stored_units takes. -fn stored_text(bytes: &[u8]) -> Result { +pub(super) fn stored_text(bytes: &[u8]) -> Result { stored_units(bytes) .enumerate() .map(|(index, unit)| char::from_u32(u32::from(unit)).ok_or(NotUcs2 { index, unit })) diff --git a/src/lib/hii/forms/write_plan.rs b/src/lib/hii/forms/write_plan.rs index 0a14682..72d312c 100644 --- a/src/lib/hii/forms/write_plan.rs +++ b/src/lib/hii/forms/write_plan.rs @@ -257,7 +257,7 @@ pub(super) fn list_own_write<'a>( /// The write of `write`, the CHAR16s of the STRING `question`, through variables of `reach`: /// refuses one with a WRITE expression, which takes its characters, or without a VarStore. -fn string_own_write<'a>( +pub(super) fn string_own_write<'a>( question: &'a QuestionDescriptor, StringWrite { offset, bytes }: StringWrite, reach: VariableReach, diff --git a/src/lib/hii/keyword_access.rs b/src/lib/hii/keyword_access.rs index a84adc6..deaf2b7 100644 --- a/src/lib/hii/keyword_access.rs +++ b/src/lib/hii/keyword_access.rs @@ -906,6 +906,7 @@ mod tests { use crate::hii::forms::ifr_fixtures::conditional; use crate::hii::forms::ifr_fixtures::eq_id_val_of; use crate::hii::forms::ifr_fixtures::flagged_option; + use crate::hii::forms::ifr_fixtures::flagged_password_on; use crate::hii::forms::ifr_fixtures::form_set; use crate::hii::forms::ifr_fixtures::form_set_with_guid; use crate::hii::forms::ifr_fixtures::in_unknown_scope; @@ -1712,37 +1713,75 @@ mod tests { assert_eq!(got, Ok(vec![]), "no password: lifted"); } - /// In a HiiDB that declares a PASSWORD question, set refuses a keyword of a STRING, whose - /// keyword set uefisettings does not implement yet, before the setup password, with and - /// without UnsafeOperations::Allow, and calls no SetData(). + /// keyword set of a STRING in a HiiDB that declares a PASSWORD question takes UnsafeOperations + /// as the other kinds do: set's refusal of the VALUE's text, a character of the Private Use + /// Area, comes before the setup password's, which refuses before any call unless + /// UnsafeOperations allows it; then SetData() writes the VALUE as given, GetData() reads it + /// back and UnsafeOperation::SetupPassword returns, lifted. #[test] - fn set_refuses_a_string_before_the_setup_password() { + fn set_of_a_string_with_a_declared_password_needs_the_flag() { let lists = [list( PATH, &[vec![string_on(3, 3, (1, 8), (0, 4), &[])], password()].concat(), )]; - for grant in [UnsafeOperations::Refuse, UnsafeOperations::Allow] { - let handler = MemoryKeywordHandler::with(&[(PATH, KEYWORD, "4100000000000000")]); + let written = resp(PATH, KEYWORD, "0000000000420041"); + let private_use = resp(PATH, KEYWORD, "000000000000e000"); - let got = set( - &lists, - &resp(PATH, KEYWORD, "4100420000000000"), - grant, - &handler, - ) - .map(|values| values.to_string()) - .map_err(|error| format!("{error:#}")); + let refused = MemoryKeywordHandler::default(); + let password = set(&lists, &written, UnsafeOperations::Refuse, &refused) + .expect_err("a declared setup password"); + let string = set(&lists, &private_use, UnsafeOperations::Allow, &refused) + .expect_err("the Private Use Area"); + let string_refused = set(&lists, &private_use, UnsafeOperations::Refuse, &refused) + .expect_err("the Private Use Area, refused"); + assert_eq!( + format!("{string_refused:#}"), + format!("{string:#}"), + "the STRING's refusal before the setup password's" + ); + let allowed = MemoryKeywordHandler::default(); + let (values, lifted) = set_keywords( + &lists, + &written, + IncludedStates::empty(), + UnsafeOperations::Allow, + VolatileWrites::Refuse, + variables(), + &allowed, + ) + .expect("allowed"); - assert_eq!( - got, - Err(format!( - "keyword {KEYWORD}: uefisettings does not yet implement keyword set of \ - EFI_IFR_STRING" - )), - "{grant:?}" - ); - assert_eq!(handler.calls(), [], "{grant:?}: calls"); - } + assert_eq!( + format!("{password:#}"), + "this HiiDB declares a setup password question, and the UEFI Shell runs no setup \ + browser, so no password is asked or checked; pass --i-am-fine-to-brick-this-system \ + to write without it", + "refused" + ); + assert_eq!( + format!("{string:#}"), + format!( + "keyword {KEYWORD}: VALUE=000000000000e000 of question \"Keyword question\": \ + cannot set question \"Keyword question\": '\\u{{e000}}' (U+E000) lies in the \ + Private Use Area, U+E000..U+F8FF, which UEFI 2.11 §33.2.6.2.1 prohibits" + ), + "the STRING's refusal, whatever the grant" + ); + assert_eq!(refused.calls(), vec![], "no call when refused"); + assert_eq!(values.to_string(), written, "allowed: the value read back"); + assert_eq!( + lifted, + vec![UnsafeOperation::SetupPassword], + "allowed: lifted" + ); + assert_eq!( + allowed.calls(), + vec![ + KeywordCall::SetData(written.clone()), + get_call(PATH, KEYWORD) + ], + "allowed: one SetData(), then a GetData()" + ); } /// set refuses a keyword whose question EFI_IFR_FLAG_READ_ONLY makes read-only, under a @@ -4315,6 +4354,84 @@ mod tests { ); } + /// Set by keyword of a STRING beside a PASSWORD of VarStoreId 0, EFI_IFR_FLAG_CALLBACK or not, + /// fails before any variable read or call through variables of either reach, with + /// UnsafeOperations::Allow too, as set does; beside a PASSWORD on Store that ends before it, + /// SetData() writes the STRING's VALUE. + #[test] + fn set_refuses_a_string_beside_a_password_of_var_store_id_0() { + let written = resp(PATH, KEYWORD, "0000000000420041"); + let set_of = |password: Vec, reach| { + let ifr = form_set(&[ + var_store(1, STORE_GUID, "Store", 48), + password, + string_on(7, 3, (1, 40), (0, 4), &[]), + ]); + let interface = match reach { + VariableReach::BootServices => VariableInterface::VariableServices, + VariableReach::Runtime => VariableInterface::Efivarfs, + }; + let variables = Rc::new(MemoryVariables::new(reach, interface)); + let key = VariableKey { + name: "Store".to_owned(), + guid: Guid::from(STORE_GUID), + }; + variables.insert(key, ATTRIBUTES, &[0; 48]); + let handler = MemoryKeywordHandler::default(); + let got = set_keywords( + &[list_of_ifr(PATH, &ifr)], + &written, + IncludedStates::empty(), + UnsafeOperations::Allow, + VolatileWrites::Refuse, + Box::new(Rc::clone(&variables)), + &handler, + ) + .map(|(values, _)| values.to_string()) + .map_err(|error| format!("{error:#}")); + (got, variables.reads().len(), handler.calls()) + }; + let record = "PASSWORD question 0x0009 in FormSet 11111111-1111-1111-1111-111111111111: \ + VarStoreId 0 names no VarStore"; + + for flags in [0x00, 0x04] { + for (reach, end) in [ + ( + VariableReach::BootServices, + "; as the variable of that password is unknown, the UEFI Shell build reads \ + and writes no variable of this HiiDB", + ), + ( + VariableReach::Runtime, + "; as a PASSWORD there may keep its password in any variable, uefisettings \ + reads and writes no variable of this HiiDB", + ), + ] { + let (got, reads, calls) = set_of(flagged_password_on(9, 9, 0, 0, flags), reach); + assert!( + got.as_ref() + .is_err_and(|error| error.contains(record) && error.ends_with(end)), + "Flags {flags:#04x}, {reach:?}: {got:?}" + ); + assert_eq!(reads, 0, "Flags {flags:#04x}, {reach:?}: reads"); + assert_eq!(calls, vec![], "Flags {flags:#04x}, {reach:?}: calls"); + } + } + let (got, _, calls) = set_of( + flagged_password_on(9, 9, 1, 0, 0x00), + VariableReach::BootServices, + ); + assert_eq!(got, Ok(written.clone()), "beside a password on Store"); + assert_eq!( + calls, + vec![ + KeywordCall::SetData(written.clone()), + get_call(PATH, KEYWORD) + ], + "beside a password on Store: calls" + ); + } + /// Beside a PASSWORD of VarStoreId 0, whose password may be in any variable, /// EFI_IFR_FLAG_CALLBACK or not, get and set by keyword refuse the HiiDB before any call, as /// get and set do, set with UnsafeOperations::Allow too; beside one on Store they read the From 709eb97c7d4fc96f144909c929ef19a950ff6cae Mon Sep 17 00:00:00 2001 From: Dmitrii Okunev Date: Wed, 11 Nov 2026 09:27:01 +0000 Subject: [PATCH 02/56] test(qemu): STRING keywords in shell-keyword-kinds publish-hiidb's keyword-kinds-forms gains KindString, a STRING of MaxSize 4 at VarOffset 34 of KindStore, and KindText, a multi-line STRING (EFI_IFR_STRING_MULTI_LINE) of MaxSize 4 at 42, each with an x-UEFI-ns keyword. KindStore's VarStore, and the variable that the driver creates, grow to 50 bytes, the STRINGs empty. In the UEFI Shell, shell-keyword-kinds now: - sets KindString to "AB" with VALUE=0000000000420041, which dmpstore shows as 41 00 42 00 00 00 00 00 at offset 34 and `hii keyword get` reads back as the same VALUE; - sets KindText to "A", LF, "B" with VALUE=00000042000a0041; - refuses before any call VALUEs of KindString of 4 characters, of a character after the NULL, of U+E000, of a UTF-16 surrogate, of a line feed, of 2 CHAR16s and with &READONLY, each with its error, and dmpstore shows KindStore unchanged after each. dmpstore-before checks the new DataSize and the empty STRINGs. Test Plan: - make -k test-e2e-qemu-shell-x86_64 SCENARIOS=shell-keyword-kinds passes every check of its 38 steps. - QEMU carries over from runs before a conflict-free rebase onto the current main and before the previous commit's test-only and README changes: shell-keyword-kinds passed every check on x86_64, i686 and aarch64. - Before an earlier conflict-free rebase, onto 5 later commits of main, and before the previous commit gained a unit test and a doc comment: the whole make -k test-e2e-qemu-x86_64 and test-e2e-qemu-shell-x86_64 sets passed, in 6 runs of their scenarios: 108 boot, and 4 that only i686 boots skip. With this commit alone on the previous commit's parent then, x86_64 failed 13 checks, where the steps assert keyword set of a STRING: set-string (status and line), dmpstore-string, get-string, which reads VALUE=0000000000000000, set-text (status and line), dmpstore-text, and the errors of set-string-too-long, set-string-unwritten, set-string-private-use, set-string-surrogate, set-string-line-break and set-string-width, each "uefisettings does not yet implement keyword set of EFI_IFR_STRING". set-string-read-only-resp passed there, as &READONLY is refused before the kind. - make -k lint passes; make -k test: 1579 passed; make test-i686: 1576 passed; cargo test --lib: 1485 passed. - The Windows emulation of the tests: 884 passed, where the previous commit's parent passes 879; the one test that it passes and this tree lacks is the one that the previous commit replaces. Signed-off-by: Dmitrii Okunev --- tests/qemu/README.md | 18 +++--- tests/qemu/publish-hiidb/src/hii_package.rs | 1 + .../publish-hiidb/src/keyword_kinds_forms.rs | 52 ++++++++++++---- tests/qemu/shell/keyword-kinds.nsh | 42 ++++++++++++- tests/qemu/shell/keyword-kinds.tsv | 61 +++++++++++++++++-- 5 files changed, 149 insertions(+), 25 deletions(-) diff --git a/tests/qemu/README.md b/tests/qemu/README.md index 56f4d49..a6dac29 100644 --- a/tests/qemu/README.md +++ b/tests/qemu/README.md @@ -316,14 +316,18 @@ question beside it reads, and in the UEFI Shell writes. `publish-hiidb.efi keyword-kinds-forms` creates the variable `KindStore` unless it exists, with its VarStore's Attributes and Size, and registers the forms of -[`publish-hiidb/src/keyword_kinds_forms.rs`](publish-hiidb/src/keyword_kinds_forms.rs) for a -new handle of a vendor device path: a DATE that suppresses its year, a TIME that suppresses its -second, an ORDERED_LIST of at most 3 UINT8 options under EFI_IFR_UNIQUE_SET and a REF5 on -`KindStore`, each with an x-UEFI-ns keyword. In the scenario `shell-keyword-kinds`, -`uefisettings.efi hii keyword set` writes the DATE, the TIME and the ORDERED_LIST through edk2's -Config Keyword Handler, one at a time and two in one SetData(), and refuses before any call a +[`publish-hiidb/src/keyword_kinds_forms.rs`](publish-hiidb/src/keyword_kinds_forms.rs) for a new +handle of a vendor device path: a DATE that suppresses its year, a TIME that suppresses its +second, an ORDERED_LIST of at most 3 UINT8 options under EFI_IFR_UNIQUE_SET, a REF5, and a +STRING and a multi-line STRING of MaxSize 4 on `KindStore`, each with an x-UEFI-ns keyword. In +the scenario `shell-keyword-kinds`, `uefisettings.efi hii keyword set` writes the DATE, the +TIME, the ORDERED_LIST and the STRINGs through edk2's Config Keyword Handler, one at a time and +two in one SetData(), and `hii keyword get` reads the STRING back. It refuses before any call a VALUE that changes a suppressed field, an hour out of range, a container after an empty one, an -option twice and the REF, which `dmpstore` shows wrote nothing. +option twice and the REF, and of a STRING a VALUE of 4 characters, of a character after the +NULL, of a Private Use Area character, of a UTF-16 surrogate, of a line feed in the single-line +STRING, of fewer bytes than MaxSize CHAR16s or with &READONLY, which `dmpstore` shows wrote +nothing. `publish-hiidb.efi name-value-password-forms` registers the package lists of [`publish-hiidb/src/name_value_password_forms.rs`](publish-hiidb/src/name_value_password_forms.rs) diff --git a/tests/qemu/publish-hiidb/src/hii_package.rs b/tests/qemu/publish-hiidb/src/hii_package.rs index f539693..0e89e65 100644 --- a/tests/qemu/publish-hiidb/src/hii_package.rs +++ b/tests/qemu/publish-hiidb/src/hii_package.rs @@ -52,6 +52,7 @@ pub(crate) const EFI_IFR_REF_OP: u8 = 0x0F; pub(crate) const EFI_IFR_AND_OP: u8 = 0x15; pub(crate) const EFI_IFR_DATE_OP: u8 = 0x1A; pub(crate) const EFI_IFR_TIME_OP: u8 = 0x1B; +pub(crate) const EFI_IFR_STRING_OP: u8 = 0x1C; pub(crate) const EFI_IFR_DISABLE_IF_OP: u8 = 0x1E; pub(crate) const EFI_IFR_MAP_OP: u8 = 0x22; pub(crate) const EFI_IFR_ORDERED_LIST_OP: u8 = 0x23; diff --git a/tests/qemu/publish-hiidb/src/keyword_kinds_forms.rs b/tests/qemu/publish-hiidb/src/keyword_kinds_forms.rs index 55cac2d..259be1b 100644 --- a/tests/qemu/publish-hiidb/src/keyword_kinds_forms.rs +++ b/tests/qemu/publish-hiidb/src/keyword_kinds_forms.rs @@ -13,8 +13,9 @@ //! The package list that `publish-hiidb.efi keyword-kinds-forms` registers for a handle of //! [`DRIVER_PATH`], for the UEFI Shell scenario `shell-keyword-kinds` of //! `tests/qemu/scenarios.tsv`: a form set with a DATE that suppresses its year, a TIME that -//! suppresses its second, an ORDERED_LIST of at most 3 UINT8 options under EFI_IFR_UNIQUE_SET and -//! a REF5, each with an x-UEFI-ns keyword, on the VARSTORE_EFI KindStore, which the driver creates +//! suppresses its second, an ORDERED_LIST of at most 3 UINT8 options under EFI_IFR_UNIQUE_SET, a +//! REF5, a STRING and a multi-line STRING, each with an x-UEFI-ns keyword, on the VARSTORE_EFI +//! KindStore, which the driver creates //! with the VarStore's Attributes and Size when it is missing. The Keyword Handler finds the //! questions by the device path package that the HII database adds for that handle, and writes //! KindStore through SetVariable() with the VarStore's Attributes (edk2-stable202608 @@ -42,6 +43,7 @@ use crate::hii_package::EFI_IFR_FORM_SET_OP; use crate::hii_package::EFI_IFR_ONE_OF_OPTION_OP; use crate::hii_package::EFI_IFR_ORDERED_LIST_OP; use crate::hii_package::EFI_IFR_REF_OP; +use crate::hii_package::EFI_IFR_STRING_OP; use crate::hii_package::EFI_IFR_TIME_OP; use crate::hii_package::EFI_IFR_VARSTORE_EFI_OP; use crate::vendor_device_path; @@ -68,31 +70,37 @@ const KIND_DATE: u16 = 2; const KIND_TIME: u16 = 3; const KIND_LIST: u16 = 4; const KIND_REF: u16 = 5; -const TITLE: u16 = 6; -const HELP: u16 = 7; -const FIRST: u16 = 8; -const SECOND: u16 = 9; -const THIRD: u16 = 10; -const STRINGS: [&str; 10] = [ +const KIND_STRING: u16 = 6; +const KIND_TEXT: u16 = 7; +const TITLE: u16 = 8; +const HELP: u16 = 9; +const FIRST: u16 = 10; +const SECOND: u16 = 11; +const THIRD: u16 = 12; +const STRINGS: [&str; 12] = [ "English", "Kind date", "Kind time", "Kind list", "Kind ref", + "Kind string", + "Kind text", "uefisettings QEMU keyword kinds forms", - "A DATE, a TIME, an ORDERED_LIST and a REF5 with x-UEFI-ns keywords", + "A DATE, a TIME, an ORDERED_LIST, a REF5 and two STRINGs with x-UEFI-ns keywords", "First", "Second", "Third", ]; /// The x-UEFI-ns strings: the language name, as edk2's DriverSample names it, then the keywords /// of the prompts. -const KEYWORDS: [&str; 5] = [ +const KEYWORDS: [&str; 7] = [ "UefiNameSpace", "KindDate", "KindTime", "KindList", "KindRef", + "KindString", + "KindText", ]; // The VarStore and the questions' VarOffsets @@ -101,6 +109,8 @@ const DATE_OFFSET: u16 = 0; const TIME_OFFSET: u16 = 4; const LIST_OFFSET: u16 = 8; const REF_OFFSET: u16 = 12; +const STRING_OFFSET: u16 = 34; +const TEXT_OFFSET: u16 = 42; /// EFI_QF_DATE_YEAR_SUPPRESS (UEFI 2.11 §33.3.8.3.11). const YEAR_SUPPRESS: u8 = 0x01; @@ -110,6 +120,10 @@ const SECOND_SUPPRESS: u8 = 0x04; const UNIQUE_SET: u8 = 0x01; /// The MaxContainers of the ORDERED_LIST. const MAX_CONTAINERS: u8 = 3; +/// The MaxSize of each STRING, in CHAR16s: 3 characters and the NULL. +const STRING_MAX_SIZE: u8 = 4; +/// EFI_IFR_STRING_MULTI_LINE (UEFI 2.11 §33.3.8.3.70). +const MULTI_LINE: u8 = 0x01; /// EFI_IFR_TYPE_NUM_SIZE_8 (UEFI 2.11 §33.3.8.3.51). const UINT8_TYPE: u8 = 0x00; @@ -130,7 +144,8 @@ pub(crate) fn package_list_bytes() -> Vec { /// items 1, 2 and 3 at [`LIST_OFFSET`], and at [`REF_OFFSET`] the EFI_HII_REF of the REF5 itself /// (QuestionId 4, FormId 1, FormSetGuid [`FORM_SET`], DevicePath 0), its last 22 bytes, as edk2's /// HiiConfigToBlock refuses a block past the variable's end (ConfigRouting.c :6383-6386); 0 in -/// bytes 7 and 11, which are in no question. +/// bytes 7 and 11, which are in no question, and the empty strings, all NULL CHAR16s, at +/// [`STRING_OFFSET`] and [`TEXT_OFFSET`]. pub(crate) fn store_data() -> Vec { [ &[0xE8, 0x07, 3, 31][..], @@ -142,6 +157,8 @@ pub(crate) fn store_data() -> Vec { &1u16.to_le_bytes(), &FORM_SET.to_bytes(), &0u16.to_le_bytes(), + &[0; 2 * STRING_MAX_SIZE as usize], + &[0; 2 * STRING_MAX_SIZE as usize], ] .concat() } @@ -197,6 +214,8 @@ fn form_package() -> Vec { false, &question_header(KIND_REF, HELP, 4, STORE, REF_OFFSET), ), + string(5, KIND_STRING, STRING_OFFSET, 0), + string(6, KIND_TEXT, TEXT_OFFSET, MULTI_LINE), ]; let form = [1u16.to_le_bytes(), TITLE.to_le_bytes()].concat(); let body = [ @@ -221,3 +240,14 @@ fn date_time(op_code: u8, id: u16, prompt: u16, offset: u16, flags: u8) -> Vec Vec { + let data = [ + &question_header(prompt, HELP, id, STORE, offset)[..], + &[0, STRING_MAX_SIZE, flags], + ] + .concat(); + op(EFI_IFR_STRING_OP, false, &data) +} diff --git a/tests/qemu/shell/keyword-kinds.nsh b/tests/qemu/shell/keyword-kinds.nsh index a1d9009..1321de8 100644 --- a/tests/qemu/shell/keyword-kinds.nsh +++ b/tests/qemu/shell/keyword-kinds.nsh @@ -2,8 +2,8 @@ echo -off # The boot of the scenario shell-keyword-kinds: publish-hiidb.efi keyword-kinds-forms creates # KindStore and registers the forms of publish-hiidb/src/keyword_kinds_forms.rs for a handle of # the device path 0104140005a5788ca0ba4a4c869f000000001e027fff0400, whose x-UEFI-ns keywords -# KindDate, KindTime, KindList and KindRef uefisettings.efi sets through the Keyword Handler; -# shell/keyword-kinds.tsv holds the checks of the steps. +# KindDate, KindTime, KindList, KindRef, KindString and KindText uefisettings.efi sets through +# the Keyword Handler; shell/keyword-kinds.tsv holds the checks of the steps. fs0: echo uefisettings-shell: start dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 @@ -44,5 +44,43 @@ uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4 echo uefisettings-shell: step set-date-and-list %lasterror% dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 echo uefisettings-shell: step dmpstore-date-and-list %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0000000000420041" +echo uefisettings-shell: step set-string %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string %lasterror% +uefisettings.efi hii keyword get --namespace x-UEFI-ns "PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString" +echo uefisettings-shell: step get-string %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindText&VALUE=00000042000a0041" +echo uefisettings-shell: step set-text %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-text %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0044004300420041" +echo uefisettings-shell: step set-string-too-long %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-too-long %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0000004200000041" +echo uefisettings-shell: step set-string-unwritten %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-unwritten %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=000000000000e000" +echo uefisettings-shell: step set-string-private-use %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-private-use %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=000000000000d800" +echo uefisettings-shell: step set-string-surrogate %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-surrogate %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=00000042000a0041" +echo uefisettings-shell: step set-string-line-break %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-line-break %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=00420041" +echo uefisettings-shell: step set-string-width %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-width %lasterror% +uefisettings.efi hii keyword set "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0000000000420041&READONLY" +echo uefisettings-shell: step set-string-read-only-resp %lasterror% +dmpstore KindStore -guid 8C78A505-BAA0-4C4A-869F-000000001E01 +echo uefisettings-shell: step dmpstore-string-read-only-resp %lasterror% echo uefisettings-shell: done reset -s diff --git a/tests/qemu/shell/keyword-kinds.tsv b/tests/qemu/shell/keyword-kinds.tsv index 422ea10..e4ac12b 100644 --- a/tests/qemu/shell/keyword-kinds.tsv +++ b/tests/qemu/shell/keyword-kinds.tsv @@ -1,13 +1,16 @@ -# The checks of shell/keyword-kinds.nsh: hii keyword set of a DATE, a TIME and an ORDERED_LIST -# through the Keyword Handler, which writes each VALUE as given (UEFI 2.11 §35.3.2), and refusals -# before any call, which dmpstore shows wrote nothing. See check-shell-log.sh. +# The checks of shell/keyword-kinds.nsh: hii keyword set of a DATE, a TIME, an ORDERED_LIST and +# STRINGs through the Keyword Handler, which writes each VALUE as given (UEFI 2.11 §35.3.2), and +# refusals before any call, which dmpstore shows wrote nothing. See check-shell-log.sh. # TerminalDxe prints a character outside ASCII as "?" unless the serial terminal is VT-UTF8, so # the checks of texts that cite "§" compare them with "?" in its place. # KindStore as publish-hiidb creates it: 2024-03-31 at byte 0, 12:34:56 at byte 4, the items 1, 2 -# and 3 at byte 8, and the EFI_HII_REF of QuestionId 4 and FormId 1 at byte 12 +# and 3 at byte 8, the EFI_HII_REF of QuestionId 4 and FormId 1 at byte 12, and the empty STRINGs +# KindString at byte 34 and KindText at byte 42 dmpstore-before status any -dmpstore-before line ^Variable NV\+BS '8C78A505-BAA0-4C4A-869F-000000001E01:KindStore' DataSize = 0x22$ +dmpstore-before line ^Variable NV\+BS '8C78A505-BAA0-4C4A-869F-000000001E01:KindStore' DataSize = 0x32$ dmpstore-before line ^ 00000000: E8 07 03 1F 0C 22 38 00-01 02 03 00 04 00 01 00 +dmpstore-before line ^ 00000020: 00 00 00 00 00 00 00 00-00 00 00 00 00 00 00 00 +dmpstore-before line ^ 00000030: 00 00 +\*\.\.\*$ # the DATE suppresses its year: a VALUE that keeps 2024 sets 2024-04-01 set-date status 0x0 set-date line ^NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindDate&VALUE=010407e8$ @@ -50,3 +53,51 @@ set-date-and-list status 0x0 set-date-and-list line ^NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindDate&VALUE=1f0c07e8&NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindList&VALUE=000103$ dmpstore-date-and-list status any dmpstore-date-and-list line ^ 00000000: E8 07 0C 1F 0D 00 38 00-03 01 00 00 04 00 01 00 +# a STRING's VALUE holds its MaxSize CHAR16s last byte first, NULL-padded: "AB" +set-string status 0x0 +set-string line ^NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0000000000420041$ +dmpstore-string status any +dmpstore-string line ^ 00000020: 00 00 41 00 42 00 00 00-00 00 00 00 00 00 00 00 +get-string status 0x0 +get-string line ^NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindString&VALUE=0000000000420041$ +# the multi-line STRING takes a line feed: "A", LF, "B" +set-text status 0x0 +set-text line ^NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f000000001e027fff0400&KEYWORD=KindText&VALUE=00000042000a0041$ +dmpstore-text status any +dmpstore-text line ^ 00000020: 00 00 41 00 42 00 00 00-00 00 41 00 0A 00 42 00 +dmpstore-text line ^ 00000030: 00 00 +\* +# 4 characters, where the last CHAR16 holds the NULL +set-string-too-long status aborted +set-string-too-long json (.error_message | gsub("§"; "?")) == "keyword KindString: VALUE=0044004300420041 of question \"Kind string\": cannot set question \"Kind string\": the STRING takes at most 3 characters, its last CHAR16 of MaxSize 4 holding the NULL (UEFI 2.11 ?33.2.5.4.9), not 4" +dmpstore-string-too-long status any +dmpstore-string-too-long same dmpstore-text +# a character after the NULL, which set does not write +set-string-unwritten status aborted +set-string-unwritten json .error_message == "keyword KindString: SetData() writes VALUE=0000004200000041 as given, where set writes 0000000000000041 at VarOffset 34 for question \"Kind string\"" +dmpstore-string-unwritten status any +dmpstore-string-unwritten same dmpstore-text +# U+E000, of the Private Use Area +set-string-private-use status aborted +set-string-private-use json (.error_message | gsub("§"; "?")) == "keyword KindString: VALUE=000000000000e000 of question \"Kind string\": cannot set question \"Kind string\": '\\u{e000}' (U+E000) lies in the Private Use Area, U+E000..U+F8FF, which UEFI 2.11 ?33.2.6.2.1 prohibits" +dmpstore-string-private-use status any +dmpstore-string-private-use same dmpstore-text +# a UTF-16 surrogate +set-string-surrogate status aborted +set-string-surrogate json (.error_message | gsub("§"; "?")) == "keyword KindString: VALUE=000000000000d800 of question \"Kind string\": its character 0 is CHAR16 0xd800, a UTF-16 surrogate, which UCS-2 has no character for (UEFI 2.11 ?33.2.6.2.2)" +dmpstore-string-surrogate status any +dmpstore-string-surrogate same dmpstore-text +# a line feed in the single-line STRING +set-string-line-break status aborted +set-string-line-break json (.error_message | gsub("§"; "?")) == "keyword KindString: VALUE=00000042000a0041 of question \"Kind string\": cannot set question \"Kind string\": the STRING is single-line (no EFI_IFR_STRING_MULTI_LINE, UEFI 2.11 ?33.3.8.3.70), so it takes no line break" +dmpstore-string-line-break status any +dmpstore-string-line-break same dmpstore-text +# 2 CHAR16s, where SetData() writes MaxSize 4 +set-string-width status aborted +set-string-width json .error_message == "keyword KindString: VALUE=00420041 is not as wide as question \"Kind string\"" +dmpstore-string-width status any +dmpstore-string-width same dmpstore-text +# &READONLY +set-string-read-only-resp status aborted +set-string-read-only-resp json .error_message == "keyword KindString: its has &READONLY, and SetData() writes no read-only keyword" +dmpstore-string-read-only-resp status any +dmpstore-string-read-only-resp same dmpstore-text From bb4f196ebc1f2bf55e86ba33da447322434a4c25 Mon Sep 17 00:00:00 2001 From: Dmitrii Okunev Date: Wed, 11 Nov 2026 10:48:38 +0000 Subject: [PATCH 03/56] fix(hii): read no variable of a HiiDB with a PASSWORD of an undeclared VarStoreId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PASSWORD's VarStoreId names "a previously declared variable store" (UEFI 2.11 §33.3.8.2.2), and VarStores are declared within a form set (§33.2.5.1). For an id that no VarStore of its form package declares, edk2-stable202608 finds no storage: the Forms Browser looks only in its FormSet's storage list, then ASSERTs and dereferences what it found (SetupBrowserDxe/IfrParse.c:136-153), and the Config Keyword Handler walks only its form package (HiiDatabaseDxe/ConfigKeywordHandler.c FindStorageFromVarId, :1520-1569) and ASSERTs on the NULL that it returns (:2108-2109). Until now such a PASSWORD was unresolved but kept by its IFR (Keeper::Ifr): through BootServices its HiiDB was refused, but through Runtime get, set, list-questions, show-ifr and dump-efivars read and wrote every variable of its HiiDB, with a warning only, and keyword get and set called GetData() and SetData(). It is now driver-held, as one of VarStoreId 0 is: VarStoreIds::keeper gives Keeper::OwnDriver to an id that no VarStore of its form package that decodes declares (FirstVarStores::declared), so check_bounded refuses every variable through Runtime too; through BootServices the HiiDB stays refused as before. - So is one in a disabled form, outside any FormSet, in IFR that uefisettings ignores, whose record names the VarStoreId, or whose question header past its op names such an id, and one whose id only another form package or another package list declares. - FindStorageFromVarId reads the VarStoreId field of a VarStore op whatever the op's Length (:1538-1555), and so does the Forms Browser for one of the question's FormSet outside an unknown opcode's scope (IfrParse.c:1873-1918), so either would take one that does not decode; an id of only such an op is refused too, which fails closed. - An id that another FormSet of its form package, or a VarStore outside any FormSet, declares stays bounded by the first VarStore of that id, which the Config Keyword Handler takes, as does one that its own FormSet declares after it. - Variables that read nothing are not refused, so list-questions and show-ifr with --no-efivars still read such a HiiDB. - --i-am-fine-to-brick-this-system lifts none of it, as it lifts no confidentiality refusal. - README.md says so, and the docs that list the driver-held cases name it. - The test helper of the driver-held refusals checks show-ifr through Runtime too, so it does for PASSWORDs of VarStoreId 0 and for those that edk2 may route to a VARSTORE_NAME_VALUE as well. Test Plan: - With this commit's tests on the product code of its parent, cargo test --locked --lib fails 6 tests and passes 1479: passwords_of_undeclared_var_store_ids_close_every_variable, a_password_of_an_id_that_only_another_form_package_declares_closes_every_variable, a_headless_password_in_a_disabled_form_closes_every_variable, dump_refuses_a_hiidb_with_a_password_of_an_undeclared_var_store_id, get_and_set_refuse_beside_a_password_of_an_undeclared_var_store_id and password_without_a_known_var_store_is_unresolved; there get reads a question beside such a PASSWORD, dump-efivars copies its VarStore, keyword get calls GetData() and its record is not driver-held. Run alone there, the case of an id of only a VarStore op of its FormSet that does not decode fails too: get reads B. - At this commit, cargo check --locked --all-targets passes, and cargo test --locked --lib passes 1485, both with RUSTFLAGS=-D warnings. - 17 mutants each fail named tests: keeper's form-package check off, its package-list fallback taking every id, a form package without a VarStore that decodes counted as declaring every id, the ids of VarStores outside any FormSet left out of FirstVarStores::declared, the ids of the other form packages of the list counted, for any PASSWORD, for one whose question header lies past its op, or for one outside any FormSet, a VarStore op that does not decode counted as declaring, in the PASSWORD's FormSet or anywhere, an undeclared id outside any FormSet kept bounded, an id that another FormSet declares counted as undeclared, add_outside's record of one in IFR that uefisettings ignores, a BootServices read beside a driver-held PASSWORD, variables that read nothing refused, dump-efivars without check_bounded, and show-ifr or list-questions falling back to variables that read nothing where a read through Runtime is refused. Signed-off-by: Dmitrii Okunev --- README.md | 12 +- src/lib/exports.rs | 380 ++++++++++++++++++++++++++-- src/lib/hii/efivar_dump/write.rs | 57 ++++- src/lib/hii/forms.rs | 23 +- src/lib/hii/forms/setup_password.rs | 79 ++++-- src/lib/hii/keyword_access.rs | 89 +++++++ src/lib/hii/package.rs | 1 + 7 files changed, 581 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index f989b10..30a06b3 100644 --- a/README.md +++ b/README.md @@ -113,8 +113,16 @@ write no variable of a HiiDB with one, nor do `hii list-questions` and `hii show its Config Keyword Handler takes, counts as one outside any FormSet, also for the password questions of FormSets of its VarStoreId. IFR after an op that does not fit its form package has no op boundary, so they read and write no variable of such a HiiDB, nor do -`hii list-questions` and `hii show-ifr` but with `--no-efivars`. A password question whose VarStore -uefisettings cannot find withholds no bytes, which they warn of. A password question that edk2 may +`hii list-questions` and `hii show-ifr` but with `--no-efivars`. A password question of a VarStoreId +that no VarStore of its form package that decodes declares names no VarStore (UEFI 2.11 +§33.3.8.2.2 asks for "a previously declared variable store"), and edk2's Forms Browser finds none +for it but a VarStore op of its FormSet, outside an unknown opcode's scope, that does not decode, +whose fields it reads past the op's end, so they read and write no variable of a HiiDB with one, +nor do `hii list-questions` and +`hii show-ifr` but with `--no-efivars`; one whose FormSet declares no VarStore of its VarStoreId, +which another VarStore of its form package declares, keeps its password at its offset in the first +of those, which edk2's Config Keyword Handler takes, where `get` withholds it, and they warn of it. +A password question that edk2 may route to a VARSTORE_NAME_VALUE, whose values its driver exposes, as UEFI 2.11 §33.2.5.6 says, through EFI_HII_CONFIG_ACCESS_PROTOCOL, in storage of its own choosing, may keep its password in any variable, so they read and write no variable of a HiiDB with one, as for IFR without an op boundary, diff --git a/src/lib/exports.rs b/src/lib/exports.rs index 546b6f9..aad9a2a 100644 --- a/src/lib/exports.rs +++ b/src/lib/exports.rs @@ -7446,7 +7446,41 @@ mod tests { /// those that reach Runtime they work, as before. #[test] fn unresolved_password_closes_every_variable_to_boot_services() { - let hiidb = password_hiidb_with(password_on(3, 3, 9, 0)); + // VarStoreId 9 of the VARSTORE AMITSESetup that another FormSet of the form package + // declares, which the Forms Browser of edk2-stable202608 does not find for the PASSWORD + // (SetupBrowserDxe/IfrParse.c:136-148), while FindStorageFromVarId of its + // HiiDatabaseDxe/ConfigKeywordHandler.c does (:1520-1569) + let hiidb = { + let packages = [ + form_package_of( + &[ + form_set_with_guid( + [0x11; 16], + &[ + var_store_efi(1, "Setup", NV_BS_RT, 1), + var_store(2, [0x44; 16], "AMITSESetup", 81), + form( + 1, + &[ + checkbox_on(1, 1, 1), + password_on(3, 3, 9, 0), + checkbox_at(2, 2, 2, 80), + ], + ), + ], + ), + form_set_with_guid( + [0x13; 16], + &[var_store(9, [0x44; 16], "AMITSESetup", 81), form(3, &[])], + ), + ] + .concat(), + ), + en_strings_package_of(&["Q", "P", "Password"]), + END_PACKAGE.to_vec(), + ]; + package_list_with_guid([0x55; 16], &packages.concat()) + }; let closed = "PASSWORD question 0x0003 in FormSet 11111111-1111-1111-1111-111111111111: \ no supported varstore with matching id 0x0009 found; as the variable of \ that password is unknown, the UEFI Shell build reads and writes no variable \ @@ -7792,11 +7826,10 @@ mod tests { /// A PASSWORD op of a disabled form without its question header is unresolved: edk2 reads /// that header from the bytes after the op, here those of its form's End and the next FORM, - /// QuestionId 0x0229 and VarStoreId 0x8601, which no VarStore declares. Through variables that - /// reach BootServices, get reads no variable; through those that reach Runtime it reads the - /// questions of the other forms, O too. + /// QuestionId 0x0229 and VarStoreId 0x8601, which no VarStore declares, so its password may be + /// in any variable: through variables of either reach, get reads no variable. #[test] - fn a_headless_password_in_a_disabled_form_closes_boot_services() { + fn a_headless_password_in_a_disabled_form_closes_every_variable() { let (hiidb, _) = disabled_form_hiidb(operation(PASSWORD, false, &[3, 0, 4, 0])); let boot_services = disabled_form_variables( @@ -7827,18 +7860,25 @@ mod tests { assert_eq!(boot_services.reads(), [], "BootServices: reads"); let runtime = disabled_form_variables(VariableReach::Runtime, VariableInterface::Efivarfs); - for (question, want) in [ - ("Q1", vec!["1"]), - ("Q2", vec![]), - ("Q3", vec!["1"]), - ("O", vec!["1"]), - ] { - assert_eq!( - answers_of(&hiidb, question, &runtime), - want, - "Runtime: get {question}" + for question in ["Q1", "Q3", "O"] { + let got = HiiBackend::get_in( + &hiidb, + question, + None, + &GetOptions::default(), + shared(&runtime), + None, + None, + ) + .map(drop) + .map_err(|error| error.to_string()); + assert!( + got.as_ref() + .is_err_and(|error| error.starts_with(&want) && error.ends_with(ANY_VARIABLE)), + "Runtime: get {question}: {got:?}" ); } + assert_eq!(runtime.reads(), [], "Runtime: reads"); } /// The variables of password_store_hiidb, of `reach`, each byte 0x01. @@ -9566,9 +9606,15 @@ mod tests { /// VarStore NStore, then, on the buffer VarStore AMITSESetup of AMI's layout, `password`, the /// CHECKBOX P at byte 80, and the UINT16 NUMERICs S at byte 40 and X at byte 38. fn snapshot_hiidb_with(password: Vec) -> Vec { - let packages = [ - device_path_package(&DRIVER_PATH), - form_package(&[ + snapshot_hiidb_beside(password, &[]) + } + + /// snapshot_hiidb_with `password`, with the FormSets `form_sets` after its FormSet in its form + /// package. + fn snapshot_hiidb_beside(password: Vec, form_sets: &[Vec]) -> Vec { + let form_set = form_set_with_guid( + [0x11; 16], + &[ var_store(1, N_GUID, "NStore", 3), var_store(2, P_GUID, "AMITSESetup", 81), form( @@ -9589,7 +9635,11 @@ mod tests { uint16_numeric(9, 9, 2, 38), ], ), - ]), + ], + ); + let packages = [ + device_path_package(&DRIVER_PATH), + form_package_of(&[form_set, form_sets.concat()].concat()), en_strings_package_of(&["N", "W", "P", "Password", "O", "Off", "On", "S", "X"]), END_PACKAGE.to_vec(), ]; @@ -9830,8 +9880,14 @@ mod tests { false, ), ( - "a PASSWORD op on VarStore 9, which the FormSet does not declare", - snapshot_hiidb_with(password_on(3, 4, 9, 0)), + "a PASSWORD op on VarStore 9, which only another FormSet declares", + snapshot_hiidb_beside( + password_on(3, 4, 9, 0), + &[form_set_with_guid( + [0x13; 16], + &[var_store(9, P_GUID, "AMITSESetup", 81), form(3, &[])], + )], + ), no_variables(), snapshot_text(), MISSING_N_STORE, @@ -14373,10 +14429,12 @@ mod tests { } } - /// Through Runtime, get of B and O of the name_value_hiidb of `ifr`, from the snapshot too, and - /// set of O with UnsafeOperations::Allow refuse the HiiDB with AnyVariablePasswords naming - /// `record`, set writing nothing; list-questions reads without efivars; through BootServices + /// Through Runtime, get of B and O of the name_value_hiidb of `ifr`, from the snapshot too, + /// show-ifr, and set of O with UnsafeOperations::Allow refuse the HiiDB with + /// AnyVariablePasswords naming `record`, set writing nothing; list-questions reads without + /// efivars; through BootServices /// get refuses it as for a PASSWORD whose VarStore is unknown. + /// Through Runtime list-questions refuses it too. fn assert_closes_every_variable(case: &str, ifr: &[Vec], record: &str) { let hiidb = name_value_hiidb(ifr); let runtime = name_value_variables(VariableReach::Runtime); @@ -14406,6 +14464,32 @@ mod tests { .is_err_and(|error| !error.contains(UNSAFE_OPERATIONS_FLAG)), "{case}: Runtime, with UnsafeOperations::Allow: set O: {set:?}" ); + let shown = HiiBackend::show_ifr_in( + &hiidb, + &ShowIfrOptions::default(), + shared(&runtime), + None, + None, + ) + .map(drop) + .map_err(|error| format!("{error:#}")); + assert!( + refused(shown.as_ref().err()), + "{case}: Runtime: show-ifr: {shown:?}" + ); + let listed_at_runtime = HiiBackend::list_questions_in( + &hiidb, + &ListQuestionsOptions::default(), + shared(&runtime), + None, + None, + ) + .map(drop) + .map_err(|error| format!("{error:#}")); + assert!( + refused(listed_at_runtime.as_ref().err()), + "{case}: Runtime: list-questions: {listed_at_runtime:?}" + ); for (name, guid) in [("Backing", P_GUID), ("Other", OTHER_GUID)] { let key = VariableKey { name: name.to_owned(), @@ -14625,6 +14709,254 @@ mod tests { ); } + /// The middle of the record of a PASSWORD in a FormSet whose VarStoreId 9 no VarStore declares. + const UNDECLARED_9: &str = "PASSWORD question 0x0003 in FormSet \ + 11111111-1111-1111-1111-111111111111: no supported varstore with \ + matching id 0x0009 found"; + + /// A PASSWORD whose VarStoreId no VarStore of its form package that decodes declares names no + /// VarStore: UEFI 2.11 §33.3.8.2.2 asks for "a previously declared variable store", and + /// edk2-stable202608 finds none for it in the Forms Browser (SetupBrowserDxe/IfrParse.c + /// CreateQuestion, :136-148) nor in the Config Keyword Handler (ConfigKeywordHandler.c + /// FindStorageFromVarId, :1520-1569) but a VarStore op that does not decode, which each reads + /// whatever the op's Length: the Forms Browser one of its FormSet outside an unknown opcode's + /// scope (IfrParse.c ParseOpCodes, :1873-1918), FindStorageFromVarId the first in the form + /// package (:1538-1555), so its password may be in any variable, as for VarStoreId 0. So get, + /// set and the snapshot refuse the whole HiiDB, EFI_IFR_FLAG_CALLBACK or not: one in its form, + /// in a disabled form, outside any FormSet, in a FormSet that uefisettings ignores, one whose + /// question header past its op names such an id, one whose id only a VarStore op that does not + /// decode names, in an unknown opcode's scope or in its FormSet, which uefisettings ignores + /// for that op, one in a form package that declares no VarStore, and one whose id only another + /// package list declares. Each names its record. + #[test] + fn passwords_of_undeclared_var_store_ids_close_every_variable() { + type I = NameValueIfr; + let callback = || { + let data = [ + question_header_with(3, 3, 9, 4, 0x04), + [1u8, 0, 20, 0].to_vec(), + ] + .concat(); + operation(PASSWORD, false, &data) + }; + // a CHECKBOX op ending after 4 of the 12 bytes of its data, which disables its form + let disabler = || operation(CHECKBOX, false, &[0x00; 4]); + // a PASSWORD op of Prompt 3 and Help 4 whose question header runs on into the SUBTITLE + // after it, VarStoreId 9 and VarStoreInfo 4 (QuestionId 0x0702: the SUBTITLE's header) + let header_past_op = [ + operation(PASSWORD, false, &[3, 0, 4, 0]), + operation(SUBTITLE, false, &[9, 0, 4, 0, 0]), + ] + .concat(); + let backing_with = |forms: &[Vec]| { + let first = form(1, &[I::b(), I::p()]); + form_set(&[[I::backing(), first].as_slice(), forms].concat()) + }; + // a FormSet that uefisettings ignores, for its short DEFAULTSTORE op, with `password` + let ignored_with = |password: Vec| { + form_set_with_guid( + [0x12; 16], + &[ + operation(DEFAULT_STORE, false, &[0x00, 0x00]), + form(2, &[password]), + ], + ) + }; + let cases = [ + ( + "in its form", + vec![form_set(&[ + I::backing(), + form(1, &[I::b(), I::p(), I::password(9)]), + ])], + UNDECLARED_9, + ), + ( + "of EFI_IFR_FLAG_CALLBACK", + vec![backing_with(&[form(2, &[callback()])])], + UNDECLARED_9, + ), + ( + "in a disabled form", + vec![backing_with(&[form(2, &[I::password(9), disabler()])])], + UNDECLARED_9, + ), + ( + "outside any FormSet", + vec![I::password(9), backing_with(&[])], + "PASSWORD op of package list 55555555-5555-5555-5555-555555555555 outside a \ + FormSet, at offset 0 of a form package", + ), + ( + "in a FormSet that uefisettings ignores", + vec![backing_with(&[]), ignored_with(I::password(9))], + "the PASSWORD op there, whose VarStoreId 0x0009 names no VarStore of its form \ + package", + ), + ( + "its question header past its op", + vec![backing_with(&[form(2, &[header_past_op])])], + "PASSWORD question 0x0702 in FormSet 11111111-1111-1111-1111-111111111111: its \ + op ends before its question header", + ), + ( + "its id only of a VarStore op that does not decode", + vec![form_set(&[ + I::backing(), + in_unknown_scope(&operation( + VAR_STORE, + false, + &[&OTHER_GUID[..], &9u16.to_le_bytes()].concat(), + )), + form(1, &[I::b(), I::p(), I::password(9)]), + ])], + "PASSWORD question 0x0003 in FormSet 11111111-1111-1111-1111-111111111111: \ + failed to parse VarStore's data", + ), + ( + "its id only of a VarStore op of its FormSet that does not decode", + vec![ + backing_with(&[]), + form_set_with_guid( + [0x12; 16], + &[ + operation( + VAR_STORE, + false, + &[&OTHER_GUID[..], &9u16.to_le_bytes()].concat(), + ), + form(2, &[I::password(9)]), + ], + ), + ], + "the PASSWORD op there, whose VarStoreId 0x0009 names no VarStore of its form \ + package", + ), + ( + "in a form package that declares no VarStore", + vec![form_set(&[form(1, &[I::b(), I::p(), I::password(9)])])], + UNDECLARED_9, + ), + ( + "its id declared only by another package list", + vec![backing_with(&[form(2, &[I::password(2)])])], + "PASSWORD question 0x0003 in FormSet 11111111-1111-1111-1111-111111111111: no \ + supported varstore with matching id 0x0002 found", + ), + ]; + for (case, ifr, record) in cases { + assert_closes_every_variable(case, &ifr, record); + } + } + + /// Of a package list with two form packages, a PASSWORD whose VarStoreId only the other + /// package declares names no VarStore of its own: neither the Forms Browser nor + /// FindStorageFromVarId, which walks one form package, finds one, so get refuses the HiiDB, + /// for one in its form, one whose question header past its op names the id, and one outside + /// any FormSet. + #[test] + fn a_password_of_an_id_that_only_another_form_package_declares_closes_every_variable() { + type I = NameValueIfr; + // a PASSWORD op of Prompt 3 and Help 4 whose question header runs on into the SUBTITLE + // after it, VarStoreId 9 and VarStoreInfo 4 (QuestionId 0x0702: the SUBTITLE's header) + let header_past_op = [ + operation(PASSWORD, false, &[3, 0, 4, 0]), + operation(SUBTITLE, false, &[9, 0, 4, 0, 0]), + ] + .concat(); + let cases = [ + ( + "in its form", + form_set(&[I::backing(), form(1, &[I::b(), I::p(), I::password(9)])]), + UNDECLARED_9, + ), + ( + "its question header past its op", + form_set(&[ + I::backing(), + form(1, &[I::b(), I::p()]), + form(2, &[header_past_op]), + ]), + "PASSWORD question 0x0702 in FormSet 11111111-1111-1111-1111-111111111111: its \ + op ends before its question header", + ), + ( + "outside any FormSet", + [ + I::password(9), + form_set(&[I::backing(), form(1, &[I::b(), I::p()])]), + ] + .concat(), + "PASSWORD op of package list 55555555-5555-5555-5555-555555555555 outside a \ + FormSet, at offset 0 of a form package", + ), + ]; + let runtime = name_value_variables(VariableReach::Runtime); + + for (case, ifr, record) in cases { + let hiidb = name_value_hiidb_of_packages(&[ + ifr, + form_set_with_guid( + [0x14; 16], + &[var_store(9, P_GUID, "Backing", 8), form(3, &[])], + ), + ]); + for question in ["B", "O"] { + let got = name_value_get(&hiidb, question, &runtime, None); + assert!( + got.as_ref().is_err_and( + |error| error.contains(record) && error.ends_with(ANY_VARIABLE) + ), + "{case}: get {question}: {got:?}" + ); + } + } + } + + /// A PASSWORD whose VarStoreId a VarStore of its form package declares, though not one of its + /// FormSet before it, stays bounded: FindStorageFromVarId takes that VarStore, the first of + /// the id in the form package (edk2-stable202608 ConfigKeywordHandler.c:1520-1569), so + /// through Runtime the password's bytes there are withheld and the others read, its id declared + /// by another FormSet, by its own after it, or outside any FormSet. + #[test] + fn passwords_of_ids_that_their_form_package_declares_stay_bounded() { + type I = NameValueIfr; + let backing_9 = || var_store(9, P_GUID, "Backing", 8); + let in_its_form = || form(1, &[I::b(), I::p(), I::password(9)]); + let cases = [ + ( + "by another FormSet", + vec![ + form_set(&[I::backing(), in_its_form()]), + form_set_with_guid([0x13; 16], &[backing_9(), form(3, &[])]), + ], + ), + ( + "by its FormSet after it", + vec![form_set(&[I::backing(), in_its_form(), backing_9()])], + ), + ( + "outside any FormSet", + vec![backing_9(), form_set(&[I::backing(), in_its_form()])], + ), + ]; + let runtime = name_value_variables(VariableReach::Runtime); + + for (case, ifr) in cases { + let hiidb = name_value_hiidb(&ifr); + assert_eq!( + name_value_get(&hiidb, "B", &runtime, None), + Ok(vec!["31323".to_owned()]), + "{case}: get B" + ); + assert_eq!( + name_value_get(&hiidb, "P", &runtime, None), + Ok(vec![PASSWORD_BYTES.to_owned()]), + "{case}: get P" + ); + } + } + /// A PASSWORD that edk2 does not route to a VARSTORE_NAME_VALUE keeps its password where its /// IFR says, so through Runtime its bytes alone are withheld, beside one: one on its FormSet's /// VARSTORE of an id that a VARSTORE_NAME_VALUE of a later FormSet declares (VarStoreIds are diff --git a/src/lib/hii/efivar_dump/write.rs b/src/lib/hii/efivar_dump/write.rs index e9385d9..89ebbc2 100644 --- a/src/lib/hii/efivar_dump/write.rs +++ b/src/lib/hii/efivar_dump/write.rs @@ -83,10 +83,13 @@ pub(crate) fn dump_efivars( /// The payload bytes that the questions of `package_lists`, in any state, decode, by efivar /// file, found without reading any, which the dump copies; none that a PASSWORD question that /// uefisettings finds holds, nor any of a variable that an ignored FormSet declares. One whose -/// VarStore is unknown withholds no bytes: of_lists warns of each. IFR past an op where the walk +/// FormSet declares no VarStore of its VarStoreId, which another VarStore of its form package +/// declares, withholds its bytes in the first of those, which FindStorageFromVarId takes: of_lists +/// warns of each. IFR past an op where the walk /// for FormSets ends, or the driver of a VARSTORE_NAME_VALUE that edk2 may route a PASSWORD to, -/// or the callbacks or READ and WRITE ops of a PASSWORD of VarStoreId 0, may keep a password in -/// any variable, so a HiiDB with any of them fails (check_bounded). +/// or the callbacks or READ and WRITE ops of a PASSWORD of VarStoreId 0, or of a VarStoreId that +/// no VarStore of its form package declares, may keep a password in any variable, so a HiiDB with +/// any of them fails (check_bounded). fn decoded_bytes( package_lists: Vec, ) -> Result>> { @@ -1711,4 +1714,52 @@ mod tests { "the STRING After's bytes in the dump" ); } + + /// A PASSWORD whose VarStoreId 9 no VarStore declares names no VarStore, so its password may be + /// in any variable: the dump fails before it copies any; one on Backing leaves the CHECKBOX B + /// on Backing to copy. + #[test] + fn dump_refuses_a_hiidb_with_a_password_of_an_undeclared_var_store_id() { + let decoded = |var_store_id| { + let password = [ + question_header_with(3, 3, var_store_id, 4, 0x00), + [&1u16.to_le_bytes()[..], &20u16.to_le_bytes()].concat(), // MinSize, MaxSize + ] + .concat(); + let packages = [ + form_package(&[ + var_store(1, [0x44; 16], "Backing", 4), + form( + 1, + &[ + checkbox_at(1, 1, 1, 0), + operation(PASSWORD, false, &password), + ], + ), + ]), + en_strings_package_of(&["B", "x", "Password", "Pin"]), + END_PACKAGE.to_vec(), + ]; + let hiidb = package_list_with_guid([0x55; 16], &packages.concat()); + decoded_bytes(read_db(&hiidb).expect("read the HiiDB")) + .map(|decoded| decoded.keys().map(ToString::to_string).collect::>()) + .map_err(|error| format!("{error:#}")) + }; + + let refused = decoded(9); + assert!( + refused.as_ref().is_err_and(|error| error.ends_with( + "; as a PASSWORD there may keep its password in any variable, uefisettings \ + reads and writes no variable of this HiiDB" + )), + "VarStoreId 9: {refused:?}" + ); + assert_eq!( + decoded(1), + Ok(vec![ + "Backing-44444444-4444-4444-4444-444444444444".to_owned() + ]), + "VarStoreId 1" + ); + } } diff --git a/src/lib/hii/forms.rs b/src/lib/hii/forms.rs index 9f9e969..d110dc2 100644 --- a/src/lib/hii/forms.rs +++ b/src/lib/hii/forms.rs @@ -2121,7 +2121,7 @@ pub(super) fn parse_form_sets( &mut malformed_stores, ); } - let (first_var_stores, name_value_first) = first_var_stores(&var_stores, &outside); + let (first_var_stores, name_value_first, declared) = first_var_stores(&var_stores, &outside); Ok(FormSets { roots, ignored, @@ -2130,6 +2130,7 @@ pub(super) fn parse_form_sets( malformed_stores, first_var_stores, name_value_first, + declared, }) } @@ -2142,11 +2143,16 @@ pub(super) fn parse_form_sets( /// the package, as it walks the package op by op (:1520-1567), FORM_SET ops or not, so a FormSet /// nested in another is another FormSet, and steps into the scope of an unknown opcode as into any /// (:1566). An id whose first VarStore is outside any FormSet or in such a scope has no FormSet's: -/// PasswordQuestions withholds those as VarStores outside any FormSet. +/// PasswordQuestions withholds those as VarStores outside any FormSet. Every id of those +/// VarStores comes third: for a question of the package of an id of none, the Forms Browser and +/// FindStorageFromVarId find none but a VarStore op that does not decode, which each reads +/// whatever the op's Length: the Forms Browser one of the question's FormSet outside an unknown +/// opcode's scope (SetupBrowserDxe/IfrParse.c ParseOpCodes, :1873-1918), FindStorageFromVarId the +/// first in the package (:1538-1555). fn first_var_stores( var_stores: &[(u64, Rc>)], outside: &[OutsideOp], -) -> (BTreeMap, BTreeSet) { +) -> (BTreeMap, BTreeSet, BTreeSet) { let in_form_sets = var_stores.iter().filter_map(|(offset, store)| { let (var_store_id, declared) = declared_store(&store.borrow().parsed_data)?; Some((*offset, var_store_id, declared, Some(store))) @@ -2175,11 +2181,12 @@ fn first_var_stores( }); first.insert(var_store_id, declared_by); } + let declared = first.keys().copied().collect(); let first = first .into_iter() .filter_map(|(var_store_id, first)| Some((var_store_id, first?))) .collect(); - (first, name_value_first) + (first, name_value_first, declared) } /// Where an op that parse_form_sets records outside the FormSets that it parses sits. @@ -2565,8 +2572,9 @@ fn form_set_name(op: &IFROperation) -> String { /// The FormSets of a form package: a root for each that parse_form_sets parses, the IFR that it /// ignores, the forms that it drops from those roots, the VarStores and statements outside the /// FormSets that it parses, with the VarStores in the scope of an unknown opcode in those, -/// malformed VarStores apart, the first VarStore of each VarStoreId that a FormSet declares, and -/// the VarStoreIds whose first VarStore is a VARSTORE_NAME_VALUE (first_var_stores). +/// malformed VarStores apart, the first VarStore of each VarStoreId that a FormSet declares, the +/// VarStoreIds whose first VarStore is a VARSTORE_NAME_VALUE, and every VarStoreId that a VarStore +/// that decodes declares (first_var_stores). pub(super) struct FormSets { pub(super) roots: Vec>>, pub(super) ignored: Vec, @@ -2575,6 +2583,7 @@ pub(super) struct FormSets { pub(super) malformed_stores: Vec, pub(super) first_var_stores: BTreeMap, pub(super) name_value_first: BTreeSet, + pub(super) declared: BTreeSet, } /// The FormSets of a form package of a package list, by index in its forms, which the forms that @@ -2591,6 +2600,8 @@ pub(crate) struct FirstVarStores { /// The VarStoreIds whose first VarStore in the package, in a FormSet or not, is a /// VARSTORE_NAME_VALUE pub(crate) name_value_first: BTreeSet, + /// Every VarStoreId that a VarStore of the package that decodes declares, in a FormSet or not + pub(crate) declared: BTreeSet, } /// The first VarStore op of a VarStoreId of a form package, `store`, and the FORM_SET op nearest diff --git a/src/lib/hii/forms/setup_password.rs b/src/lib/hii/forms/setup_password.rs index b2f950f..4cf2557 100644 --- a/src/lib/hii/forms/setup_password.rs +++ b/src/lib/hii/forms/setup_password.rs @@ -79,8 +79,9 @@ pub(crate) struct PasswordQuestions { /// The records of the PASSWORDs that edk2 may route to a VARSTORE_NAME_VALUE, whose values /// its driver exposes, as UEFI 2.11 §33.2.5.6 says, through EFI_HII_CONFIG_ACCESS_PROTOCOL, /// in storage of its own choosing, so in any variable (Keeper::NameValueDriver), and of those - /// of VarStoreId 0, whose callbacks or READ and WRITE ops keep their passwords where - /// uefisettings cannot tell (Keeper::OwnDriver) + /// of VarStoreId 0, or of a VarStoreId that no VarStore of their form package declares, whose + /// callbacks or READ and WRITE ops keep their passwords where uefisettings cannot tell + /// (Keeper::OwnDriver) driver_held: Vec, candidates: Vec, /// The storage that IFR that read_db ignores, or a VarStore outside any FormSet that does not @@ -528,8 +529,17 @@ impl VarStoreIds { /// Who keeps the password of a PASSWORD of this package list whose VarStore is unknown, of /// VarStoreId `id`, in the FormSet of root `root` if any, of the form package of the /// FirstVarStores `package`: Keeper::OwnDriver for VarStoreId 0, which names no VarStore - /// (UEFI 2.11 §33.3.8.2.2), as for one without a question header, whose callers take it; - /// else the driver of a VARSTORE_NAME_VALUE that edk2 may route it to, else its IFR. The + /// (UEFI 2.11 §33.3.8.2.2), as for one without a question header, whose callers take it, and + /// for an id that no VarStore of its form package that decodes declares, which names none + /// either: §33.3.8.2.2 asks for "a previously declared variable store", and edk2-stable202608 + /// finds none for it in the Forms Browser (SetupBrowserDxe/IfrParse.c CreateQuestion, + /// :136-148) nor in the Config Keyword Handler (ConfigKeywordHandler.c FindStorageFromVarId, + /// :1520-1569, :2108-2109) but a VarStore op that does not decode, which each reads whatever + /// the op's Length: the Forms Browser one of the question's FormSet outside an unknown + /// opcode's scope (IfrParse.c ParseOpCodes, :1873-1918), FindStorageFromVarId the first in the + /// form package (:1538-1555), so this fails closed for such an op too; + /// without `package`, by no VarStore of its package list; else the driver of a + /// VARSTORE_NAME_VALUE that edk2 may route it to, else its IFR. The /// Forms Browser gives a /// question of a FormSet the VarStore of its id there (SetupBrowserDxe/IfrParse.c /// CreateQuestion, :136-146), so a VARSTORE_NAME_VALUE of that id there counts, which @@ -539,7 +549,11 @@ impl VarStoreIds { /// first that is a VARSTORE_NAME_VALUE counts, in a FormSet or not, and none of another form /// package. fn keeper(&self, id: u16, root: Option, package: Option<&FirstVarStores>) -> Keeper { - if id == 0 { + let declared = package.map_or_else( + || self.ids.contains_key(&id), + |package| package.declared.contains(&id), + ); + if id == 0 || !declared { return Keeper::OwnDriver; } let undecoded = root @@ -772,8 +786,8 @@ impl PasswordQuestions { /// Only one that passes those checks is added in the FormSet of its root (password_questions), /// with the storage of its VarStore ops that do not decode (withhold_undecoded). An unresolved /// one that edk2 may route to a VARSTORE_NAME_VALUE, in its FormSet or as the first VarStore - /// of its id in its form package, is driver-held, and so is one of VarStoreId 0 - /// (VarStoreIds::keeper). + /// of its id in its form package, is driver-held, and so is one of VarStoreId 0, or of one that + /// no VarStore of its form package declares (VarStoreIds::keeper). fn add_form_set_password( &mut self, op: &IFROperation, @@ -899,7 +913,8 @@ impl PasswordQuestions { /// of each variable that its VarName may name on a VARSTORE_EFI before UEFI 2.3.1 of that id. /// One of VarStoreId 0 names no VarStore, as one in a FormSet does not, and the Keyword /// Handler skips it (ExtractConfigRequest): it is driver-held (Keeper::OwnDriver), with a - /// record of its own in ignored IFR. One whose op ends before its MaxSize keeps every + /// record of its own in ignored IFR, and so is one whose VarStoreId no VarStore of its form + /// package declares (VarStoreIds::keeper). One whose op ends before its MaxSize keeps every /// byte from its VarOffset on, and from byte 0 of each such variable. One in ignored IFR /// (OutsideOp::in_ignored_ifr) is withheld so too, as the Keyword Handler finds it there, and /// its record counts it already. One whose question header lies past its op is read there @@ -916,7 +931,7 @@ impl PasswordQuestions { ids: &mut VarStoreIds, ) { self.declared = true; - let keeper = match PasswordFields::of(&outside.op) { + let (keeper, held_because) = match PasswordFields::of(&outside.op) { Ok(password) if password.question.var_store_id == 0 => { let unresolved = UnresolvedPassword::OwnDriver { form_set: None, @@ -930,7 +945,12 @@ impl PasswordQuestions { } Ok(password) => { self.add_in_package_list(&password, ids, &list.strings); - ids.keeper(password.question.var_store_id, None, package) + let id = password.question.var_store_id; + let held_because = format!( + "the PASSWORD op there, whose VarStoreId {id:#06x} names no VarStore of its \ + form package" + ); + (ids.keeper(id, None, package), held_because) } Err(error) => { debug!( @@ -938,7 +958,10 @@ impl PasswordQuestions { outside.offset ); self.storage.extend(&ids.storage()); - Keeper::OwnDriver + let held_because = "the PASSWORD op there, whose question header does not parse, \ + so whose VarStoreId may be 0, which names no VarStore" + .to_owned(); + (Keeper::OwnDriver, held_because) } }; let (package_list, offset) = (list.guid, outside.offset); @@ -963,9 +986,7 @@ impl PasswordQuestions { (true, Keeper::OwnDriver) => self.driver_held.push(UnresolvedPassword::InIgnoredIfr { package_list, offset, - reason: "the PASSWORD op there, whose question header does not parse, so whose \ - VarStoreId may be 0, which names no VarStore" - .to_owned(), + reason: held_because, }), } } @@ -1015,7 +1036,8 @@ impl PasswordQuestions { /// password, where they are known, unless IFR past an op where the walk for FormSets ends may /// hold one, as read_db failed for such IFR before the first FormSet, or the driver of a /// VARSTORE_NAME_VALUE, or the callbacks or READ and WRITE ops of a PASSWORD of VarStoreId 0, - /// may keep one (driver_held): AnyVariablePasswords; variables that read nothing are not + /// or of a VarStoreId that no VarStore of its form package declares, may keep one + /// (driver_held): AnyVariablePasswords; variables that read nothing are not /// refused so. fn withheld( &self, @@ -1043,7 +1065,8 @@ impl PasswordQuestions { /// Refuses a HiiDB with IFR past an op where the walk for FormSets ends, whose PASSWORD ops, /// with no op boundary, may keep their passwords in any variable, or with a PASSWORD that edk2 /// may route to a VARSTORE_NAME_VALUE, whose driver may keep it in any variable, or of - /// VarStoreId 0, whose callbacks or READ and WRITE ops may (driver_held). + /// VarStoreId 0, or of one that no VarStore of its form package declares, whose callbacks or + /// READ and WRITE ops may (driver_held). pub(crate) fn check_bounded(&self) -> Result<(), AnyVariablePasswords> { let unbounded: Vec = self .unresolved @@ -1738,21 +1761,23 @@ mod tests { } /// A PASSWORD op whose question header does not parse, or whose VarStoreId names no VarStore, - /// leaves its variable unknown. + /// leaves its variable unknown; one whose VarStoreId no VarStore declares is driver-held too + /// (Keeper::OwnDriver). #[test] fn password_without_a_known_var_store_is_unresolved() { + let record = UnresolvedPassword::NoVarStore { + form_set: Some(FORM_SET_GUID), + question_id: 2, + reason: "no supported varstore with matching id 0x0009 found".to_string(), + }; assert_eq!( password_questions(&[password_on(2, 3, 9, 0)]), PasswordQuestions { declared: true, questions: vec![], shared: vec![], - unresolved: vec![UnresolvedPassword::NoVarStore { - form_set: Some(FORM_SET_GUID), - question_id: 2, - reason: "no supported varstore with matching id 0x0009 found".to_string(), - }], - driver_held: vec![], + unresolved: vec![record.clone()], + driver_held: vec![record], candidates: vec![], storage: DeclaredStorage::default(), }, @@ -1920,10 +1945,14 @@ mod tests { /// Through variables that reach BootServices, a PASSWORD op whose VarStore is unknown could /// keep its password in any variable: withheld fails. Through those that reach Runtime the - /// bytes of the passwords with a known VarStore are withheld. + /// bytes of the passwords with a known VarStore are withheld. Here its VarStoreId 9 is of a + /// VarStore of another FormSet only, which edk2's Forms Browser does not find for it. #[test] fn boot_services_fail_closed_on_an_unresolved_password() { - let passwords = password_questions(&[password_on(2, 3, 9, 0), password_on(3, 3, 2, 0)]); + let passwords = PasswordQuestions::of([ + &form_set_of(&[password_on(2, 3, 9, 0), password_on(3, 3, 2, 0)]), + &parse_form_set(&[var_store(9, TSE_GUID, "Shadow", 81)]), + ]); assert_eq!( passwords diff --git a/src/lib/hii/keyword_access.rs b/src/lib/hii/keyword_access.rs index deaf2b7..8fd0f1a 100644 --- a/src/lib/hii/keyword_access.rs +++ b/src/lib/hii/keyword_access.rs @@ -4495,4 +4495,93 @@ mod tests { "beside a password on Store" ); } + + /// Beside a PASSWORD whose VarStoreId 9 no VarStore declares, whose password may be in any + /// variable, get and set by keyword refuse the HiiDB before any call through variables of + /// either reach, set with UnsafeOperations::Allow too; beside one on Store they read the + /// question. + #[test] + fn get_and_set_refuse_beside_a_password_of_an_undeclared_var_store_id() { + let lists = |var_store_id| { + let password = [ + question_header_with(9, 9, var_store_id, 6, 0x00), + [&1u16.to_le_bytes()[..], &20u16.to_le_bytes()].concat(), // MinSize, MaxSize + ] + .concat(); + let password = operation(crate::hii::forms::ifr_fixtures::PASSWORD, false, &password); + [list(PATH, &[questions(), vec![password]].concat())] + }; + let variables_of = |reach, interface| -> Box { + let variables = MemoryVariables::new(reach, interface); + let mut data = [0; 16]; + data[5] = 1; + data[6] = 1; + let key = VariableKey { + name: "Store".to_owned(), + guid: Guid::from(STORE_GUID), + }; + variables.insert(key, ATTRIBUTES, &data); + Box::new(variables) + }; + let get_through = + |lists: &[ParsedPackageList], variables, handler: &MemoryKeywordHandler| { + get_keywords( + lists, + &format!("KEYWORD={KEYWORD}"), + None, + &[], + KeywordGetOptions::default(), + variables, + handler, + ) + .map(|values| values.to_string()) + .map_err(|error| format!("{error:#}")) + }; + let handler = + || MemoryKeywordHandler::with(&[(PATH, KEYWORD, "01"), (PATH, SECOND_KEYWORD, "01")]); + let record = "PASSWORD question 0x0009 in FormSet 11111111-1111-1111-1111-111111111111: \ + no supported varstore with matching id 0x0009 found; "; + + for (reach, interface) in [ + (VariableReach::Runtime, VariableInterface::Efivarfs), + ( + VariableReach::BootServices, + VariableInterface::VariableServices, + ), + ] { + let refused = handler(); + let got = get_through(&lists(9), variables_of(reach, interface), &refused); + assert!( + got.as_ref().is_err_and(|error| error.starts_with(record)), + "{reach:?}: get: {got:?}" + ); + let written = set_keywords( + &lists(9), + &resp(PATH, KEYWORD, "03"), + IncludedStates::empty(), + UnsafeOperations::Allow, + VolatileWrites::Refuse, + variables_of(reach, interface), + &refused, + ) + .map(|(values, _)| values.to_string()) + .map_err(|error| format!("{error:#}")); + assert!( + written + .as_ref() + .is_err_and(|error| error.starts_with(record)), + "{reach:?}: set: {written:?}" + ); + assert_eq!(refused.calls(), vec![], "{reach:?}: calls"); + } + assert_eq!( + get_through( + &lists(1), + variables_of(VariableReach::Runtime, VariableInterface::Efivarfs), + &handler() + ), + Ok(resp(PATH, KEYWORD, "01")), + "beside a password on Store" + ); + } } diff --git a/src/lib/hii/package.rs b/src/lib/hii/package.rs index 21f70d6..3b78596 100644 --- a/src/lib/hii/package.rs +++ b/src/lib/hii/package.rs @@ -489,6 +489,7 @@ fn read_package_list(package_list: &PackageList) -> Result { outside: outside_before..outside_before + form_sets.outside.len(), declared_by: form_sets.first_var_stores, name_value_first: form_sets.name_value_first, + declared: form_sets.declared, }); roots.extend(form_sets.roots); ignored_ifr.extend(form_sets.ignored); From 02319ddcbbea393c2e74c2bca75362463c0fa4d2 Mon Sep 17 00:00:00 2001 From: Dmitrii Okunev Date: Wed, 11 Nov 2026 10:59:24 +0000 Subject: [PATCH 04/56] test(qemu): check a password of an undeclared VarStoreId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit publish-hiidb's undeclared-password-forms registers two package lists for one handle with a device path, as storageless-password-forms does. The first list's form set declares the buffer VARSTORE UdBacking, VarStoreId 1, with a UINT32 NUMERIC over its bytes 0..4, of the x-UEFI-ns keyword UndeclaredSetting, and a PASSWORD of VarStoreId 9, which no VarStore of its form package declares, so it names no VarStore (UEFI 2.11 §33.3.8.2.2 asks for "a previously declared variable store"); the second list's form set has a UINT16 NUMERIC over the buffer VARSTORE Other. vars/undeclared-password.json creates UdBacking, whose bytes 0..4 hold 0x7A4C19E3, standing for a password that the PASSWORD's driver keeps there, and Other. rt-data-undeclared-password-forms publishes the database for Linux too, and export-config-undeclared-password installs ExportConfig()'s results, as export-config does, with a of UdBacking holding 0x7A4C19E3. The PASSWORD's prompt has no x-UEFI-ns keyword, and UdBacking is a buffer VARSTORE, so edk2 resolves the PASSWORD's storage neither when it registers the forms (edk2-stable202608 HiiDatabaseDxe/Database.c:887-1013) nor when it exports their configuration (ConfigRouting.c:5485-5537), where it would ASSERT (SetupBrowserDxe/IfrParse.c:148, ConfigKeywordHandler.c:2109). hii-undeclared-password checks that Linux refuses get of each NUMERIC, list-questions, show-ifr, set, also with --i-am-fine-to-brick-this-system, which warns of nothing, and dump-efivars with the refusal of a HiiDB whose PASSWORD may keep its password in any variable, naming the PASSWORD, writing neither UdBacking nor Other and creating no dump directory, that list-questions --no-efivars lists the questions of both lists, that no output shows 0x7A4C19E3, and that the debug log says that the firmware lists no ExportConfig snapshot; hii-undeclared-password-snapshot checks the same with the snapshot, which the debug log then does not say, and that no output shows a value of it. shell-undeclared-password checks that uefisettings.efi in the UEFI Shell refuses hii get of either list's NUMERIC, keyword get of UndeclaredSetting, list-questions, and keyword set of UndeclaredSetting and set with --i-am-fine-to-brick-this-system, naming the PASSWORD and warning of nothing, that UdBacking stays as it was, and that no output shows 0x7A4C19E3. All three boot on x86_64, i686 and aarch64. Test Plan: - With this commit alone on its parent before main's history was rebased, on x86_64 hii-undeclared-password and hii-undeclared-password-snapshot fail 4 of their 5 tests each: get and list-questions show 2051807715, the value of UdBacking's bytes 0..4, and set and dump-efivars succeed; shell-undeclared-password passes there, as the UEFI Shell build already refuses a HiiDB with a PASSWORD whose VarStore is unknown, so it guards against a regression and shows that the firmware boots this IFR. - With this series on that parent, hii-undeclared-password, hii-undeclared-password-snapshot and shell-undeclared-password pass on x86_64, i686 and aarch64, and every row of tests/qemu/scenarios.tsv passes on x86_64: 116, and 4 that their architecture rule skips, in 130 boots. Those runs carry over: since then the series rebased onto its current parent without conflicts, and "fix(hii): read no variable of a HiiDB with a PASSWORD of an undeclared VarStoreId" changed only doc comments, README.md and unit tests. - At this commit, cargo check --locked --all-targets passes and cargo test --locked --lib passes 1485; make -k lint passes; make -k test: 1579 passed; make test-i686: 1576 passed; the tests of the Windows build, run on Linux under its cfgs: 883 pass, 4 more than the 879 on the parent of "fix(hii): read no variable of a HiiDB with a PASSWORD of an undeclared VarStoreId", and none that passes there fails here; the one that runs only there is the test that it renames. Signed-off-by: Dmitrii Okunev --- tests/qemu/README.md | 16 ++ tests/qemu/guest/Cargo.toml | 4 + .../guest/tests/hii_undeclared_password.rs | 214 ++++++++++++++++++ .../qemu/publish-hiidb/src/exported_config.rs | 15 +- tests/qemu/publish-hiidb/src/main.rs | 60 ++++- .../src/undeclared_password_forms.rs | 186 +++++++++++++++ tests/qemu/scenarios.tsv | 3 + tests/qemu/shell/undeclared-password.nsh | 28 +++ tests/qemu/shell/undeclared-password.tsv | 25 ++ tests/qemu/vars/undeclared-password.json | 17 ++ 10 files changed, 563 insertions(+), 5 deletions(-) create mode 100644 tests/qemu/guest/tests/hii_undeclared_password.rs create mode 100644 tests/qemu/publish-hiidb/src/undeclared_password_forms.rs create mode 100644 tests/qemu/shell/undeclared-password.nsh create mode 100644 tests/qemu/shell/undeclared-password.tsv create mode 100644 tests/qemu/vars/undeclared-password.json diff --git a/tests/qemu/README.md b/tests/qemu/README.md index a6dac29..1a9fa83 100644 --- a/tests/qemu/README.md +++ b/tests/qemu/README.md @@ -362,6 +362,22 @@ export-config-storageless-password` does it before it installs ExportConfig()'s with `--i-am-fine-to-brick-this-system`, and shows no byte of `SlBacking`, nor of the snapshot, while `hii list-questions --no-efivars` lists its questions from Linux. +`publish-hiidb.efi undeclared-password-forms` registers the package lists of +[`publish-hiidb/src/undeclared_password_forms.rs`](publish-hiidb/src/undeclared_password_forms.rs) +for one device path: a form set with a NUMERIC, of the x-UEFI-ns keyword `UndeclaredSetting`, over +the variable `UdBacking`, which [`vars/undeclared-password.json`](vars/undeclared-password.json) +creates, and a PASSWORD question of VarStoreId 9, which no VarStore of its form package declares, +then a form set with a NUMERIC over the variable `Other`. Such a VarStoreId names no VarStore (UEFI +2.11 §33.3.8.2.2 asks for "a previously declared variable store"), so the password may be in any +variable. `publish-hiidb.efi rt-data-undeclared-password-forms` does the same, then publishes the +database as `rt-data` does, and `publish-hiidb.efi export-config-undeclared-password` does it +before it installs ExportConfig()'s results as `export-config` does, with a `` of +`UdBacking`. In the scenarios `shell-undeclared-password`, `hii-undeclared-password` and +`hii-undeclared-password-snapshot` `uefisettings` reads and writes no variable of that HiiDB, in the +UEFI Shell and from Linux, even with `--i-am-fine-to-brick-this-system`, and shows no byte of +`UdBacking`, nor of the snapshot, while `hii list-questions --no-efivars` lists its questions from +Linux. + `publish-hiidb.efi install-config-tables` sets no `HiiDB`: it installs a copy of the HII database, in a zeroed runtime pool a quarter larger, as the EFI configuration table of the HII database protocol's GUID, as EDK2 does with `PcdHiiOsRuntimeSupport`, which OVMF and ArmVirt diff --git a/tests/qemu/guest/Cargo.toml b/tests/qemu/guest/Cargo.toml index c0eaf03..031828f 100644 --- a/tests/qemu/guest/Cargo.toml +++ b/tests/qemu/guest/Cargo.toml @@ -111,6 +111,10 @@ required-features = ["test-vm"] name = "hii_storageless_password" required-features = ["test-vm"] +[[test]] +name = "hii_undeclared_password" +required-features = ["test-vm"] + [[test]] name = "hii_nested_password" required-features = ["test-vm"] diff --git a/tests/qemu/guest/tests/hii_undeclared_password.rs b/tests/qemu/guest/tests/hii_undeclared_password.rs new file mode 100644 index 0000000..71998f5 --- /dev/null +++ b/tests/qemu/guest/tests/hii_undeclared_password.rs @@ -0,0 +1,214 @@ +// Copyright 2023 Meta Platforms, Inc. and affiliates. +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +//! The boots of the scenarios hii-undeclared-password and hii-undeclared-password-snapshot: +//! publish-hiidb's `rt-data-undeclared-password-forms`, or `export-config-undeclared-password` +//! and then `rt-data`, registers the package lists of +//! `publish-hiidb/src/undeclared_password_forms.rs` for one device path before it publishes the +//! HII database, and vars/undeclared-password.json creates UdBacking, whose bytes 0..4 hold +//! 0x7A4C19E3, and Other. The first list's PASSWORD is of VarStoreId 9, which no VarStore of its +//! form package declares, so it names no VarStore (UEFI 2.11 §33.3.8.2.2 asks for "a previously +//! declared variable store"), and its password may be in any variable: through Linux, whose +//! variables read, get, list-questions, show-ifr, set, also with +//! --i-am-fine-to-brick-this-system, which lifts none of that and warns of nothing, and +//! dump-efivars refuse the whole HiiDB, the question of the second list too, naming the +//! PASSWORD, and write nothing, while list-questions without efivars reads; nothing shows +//! 0x7A4C19E3. In hii-undeclared-password-snapshot, the ExportConfig snapshot holds 0x7A4C19E3 +//! for UdBacking, and no question shows a value from it either. + +use std::path::Path; + +use uefisettings_qemu_guest::uefisettings_with_env; +use uefisettings_qemu_guest::variable_data; +use uefisettings_qemu_guest::ErrorObject; +use uefisettings_qemu_guest::Output; +use uefisettings_qemu_guest::Question; +use uefisettings_qemu_guest::PREFIXES; + +/// The efivarfs file of UdBacking, the buffer VARSTORE of the first package list. +const UD_BACKING: &str = "/sys/firmware/efi/efivars/UdBacking-8c78a505-baa0-4c4a-869f-000000002301"; +/// The efivarfs file of Other, the buffer VARSTORE of the second package list. +const OTHER: &str = "/sys/firmware/efi/efivars/Other-8c78a505-baa0-4c4a-869f-000000002303"; +/// The UINT32 NUMERIC over bytes 0..4 of UdBacking. +const BACKING_SETTING: &str = "Undeclared backing setting"; +/// The UINT16 NUMERIC over Other, of the second package list. +const OTHER_SETTING: &str = "Undeclared other setting"; +/// The record of the PASSWORD, which the refusals name. +const PASSWORD_RECORD: &str = "PASSWORD question 0x0002 in FormSet \ + 8C78A505-BAA0-4C4A-869F-000000002300: no supported varstore with matching id 0x0009 found"; +/// The end of the refusal of a HiiDB whose PASSWORD may keep its password in any variable. +const ANY_VARIABLE: &str = "; as a PASSWORD there may keep its password in any variable, \ + uefisettings reads and writes no variable of this HiiDB"; +/// What would show 0x7A4C19E3, which UdBacking and the snapshot hold where a password may be: +/// the UINT32 in decimal, its digits, its little-endian bytes in hex, as a hex dump and as a +/// Debug list. +const PIN_TEXTS: [&str; 5] = [ + "2051807715", + "7a4c19e3", + "e3194c7a", + "e3 19 4c 7a", + "227, 25, 76, 122", +]; +/// What tags a value from the ExportConfig snapshot. +const SNAPSHOT_TAG: &str = " "; +/// What the debug log says where the configuration table lists no ExportConfig snapshot. +const NO_SNAPSHOT: &str = "no ExportConfig snapshot"; +/// What starts the line of an operation that --i-am-fine-to-brick-this-system let through. +const WARNING: &str = "warning: "; +/// The environment of the runs, which logs all that uefisettings logs. +const TRACE: [(&str, &str); 1] = [("RUST_LOG", "trace")]; + +/// Whether the firmware lists the ExportConfig snapshot. +#[derive(Clone, Copy, Debug)] +enum Snapshot { + Absent, + Listed, +} + +/// Runs `uefisettings` with `args` and TRACE; panics if its standard output or error holds any +/// of PIN_TEXTS, in any case, or SNAPSHOT_TAG. +#[track_caller] +fn showing_no_pin(args: &[&str]) -> Output { + let output = uefisettings_with_env(args, &TRACE); + let (stdout, stderr) = ( + output.stdout.to_ascii_lowercase(), + output.stderr.to_ascii_lowercase(), + ); + for text in PIN_TEXTS { + assert!( + !stdout.contains(text) && !stderr.contains(text), + "{args:?} shows {text:?}: {output:#?}" + ); + } + assert!( + !output.stdout.contains(SNAPSHOT_TAG) && !output.stderr.contains(SNAPSHOT_TAG), + "{args:?} shows a value of the snapshot: {output:#?}" + ); + output +} + +/// Runs `uefisettings` with `args` as showing_no_pin does; panics unless it fails with the +/// refusal of the HiiDB that names the PASSWORD. +#[track_caller] +fn refused(args: &[&str]) -> Output { + let output = showing_no_pin(args); + let error: ErrorObject = output.json(1); + assert!( + error.error_message.contains(PASSWORD_RECORD) + && error.error_message.ends_with(ANY_VARIABLE), + "{args:?}: {}", + error.error_message + ); + output +} + +/// get refuses each question, of either package list. +fn get_refuses_every_question() { + for prefix in PREFIXES { + for question in [BACKING_SETTING, OTHER_SETTING] { + refused(&[prefix, &["get", "--json", question]].concat()); + } + } +} + +/// list-questions and show-ifr refuse the HiiDB, and the debug log tells whether the firmware +/// lists the snapshot: list-questions reads it before the refusal. +fn list_questions_refuses_beside_the_snapshot(snapshot: Snapshot) { + let listed = refused(&["hii", "list-questions", "--json"]); + let logged = listed.stderr.contains(NO_SNAPSHOT); + match snapshot { + Snapshot::Absent => assert!(logged, "list-questions: {listed:#?}"), + Snapshot::Listed => assert!(!logged, "list-questions: {listed:#?}"), + } + refused(&["hii", "show-ifr"]); +} + +/// list-questions without efivars reads no variable, so it lists the questions of both package +/// lists. +fn list_questions_reads_without_efivars() { + let output = showing_no_pin(&["hii", "list-questions", "--json", "--no-efivars"]); + let listed: Vec = output.json(0); + for name in [BACKING_SETTING, OTHER_SETTING] { + assert!( + listed.iter().any(|question| question.name == name), + "list-questions --no-efivars: {name}: {listed:#?}" + ); + } +} + +/// set, also with --i-am-fine-to-brick-this-system, refuses the question over UdBacking, warns +/// of no operation that the flag let through, and writes neither UdBacking nor Other. +fn set_refuses_and_writes_nothing() { + let before = [UD_BACKING, OTHER].map(|path| variable_data(path).expect("the variable exists")); + for prefix in PREFIXES { + for flag in [&[][..], &["--i-am-fine-to-brick-this-system"]] { + let args = [prefix, &["set", "--json"], flag, &[BACKING_SETTING, "1"]].concat(); + let output = refused(&args); + assert!( + !output.stderr.lines().any(|line| line.starts_with(WARNING)), + "{args:?} warns: {output:#?}" + ); + } + } + for (path, before) in [UD_BACKING, OTHER].iter().zip(before) { + assert_eq!( + variable_data(path), + Some(before), + "{path} after the refused sets" + ); + } +} + +/// The redacted dump-efivars refuses the HiiDB before it creates its directory. +fn dump_efivars_refuses_without_a_directory() { + let dir = "/tmp/undeclared-password-efivars"; + assert!(!Path::new(dir).exists(), "{dir} before dump-efivars"); + refused(&["hii", "dump-efivars", dir]); + assert!(!Path::new(dir).exists(), "{dir} after dump-efivars"); +} + +/// The tests of each scenario, in a module named after whether its firmware lists the snapshot. +macro_rules! undeclared_password_tests { + ($($module:ident: $snapshot:expr),* $(,)?) => {$( + mod $module { + #[test] + fn get_refuses_every_question() { + super::get_refuses_every_question(); + } + + #[test] + fn list_questions_refuses_beside_the_snapshot() { + super::list_questions_refuses_beside_the_snapshot($snapshot); + } + + #[test] + fn list_questions_reads_without_efivars() { + super::list_questions_reads_without_efivars(); + } + + #[test] + fn set_refuses_and_writes_nothing() { + super::set_refuses_and_writes_nothing(); + } + + #[test] + fn dump_efivars_refuses_without_a_directory() { + super::dump_efivars_refuses_without_a_directory(); + } + } + )*}; +} + +undeclared_password_tests!( + without_snapshot: super::Snapshot::Absent, + with_snapshot: super::Snapshot::Listed, +); diff --git a/tests/qemu/publish-hiidb/src/exported_config.rs b/tests/qemu/publish-hiidb/src/exported_config.rs index de38c23..683969f 100644 --- a/tests/qemu/publish-hiidb/src/exported_config.rs +++ b/tests/qemu/publish-hiidb/src/exported_config.rs @@ -23,7 +23,10 @@ //! [`name_value_password_forms`] and the `` of their VARSTORE_NAME_VALUE, which holds //! the value of the name that their PASSWORD is on. `export-config-storageless-password` does it //! with those of [`storageless_password_forms`] and the `` of their VARSTORE -//! SlBacking, whose bytes stand for a password that their PASSWORDs of VarStoreId 0 keep there. +//! SlBacking, whose bytes stand for a password that their PASSWORDs of VarStoreId 0 keep there, +//! and `export-config-undeclared-password` with those of [`undeclared_password_forms`] and the +//! `` of their VARSTORE UdBacking, whose bytes stand for a password that the driver of +//! their PASSWORD of an undeclared VarStoreId keeps there. use alloc::format; use alloc::string::String; @@ -48,7 +51,9 @@ use crate::password_forms; use crate::register_name_value_password_forms; use crate::register_package_list_for; use crate::register_storageless_password_forms; +use crate::register_undeclared_password_forms; use crate::storageless_password_forms; +use crate::undeclared_password_forms; use crate::vendor_device_path; use crate::Failure; @@ -99,6 +104,14 @@ pub(crate) fn export_storageless_config() -> Result<(), Failure> { export_config_with(&storageless_password_forms::backing_resp(), "SlBacking") } +/// Registers the package lists of [`undeclared_password_forms`] for a handle of their device +/// path, then installs ExportConfig()'s results with the `` of their VARSTORE +/// UdBacking after them, and logs where. +pub(crate) fn export_undeclared_config() -> Result<(), Failure> { + register_undeclared_password_forms()?; + export_config_with(&undeclared_password_forms::backing_resp(), "UdBacking") +} + /// Installs ExportConfig()'s results with `resp`, the `` of `storage`, after them, /// and logs where. fn export_config_with(resp: &str, storage: &str) -> Result<(), Failure> { diff --git a/tests/qemu/publish-hiidb/src/main.rs b/tests/qemu/publish-hiidb/src/main.rs index 1593b4f..a0a00df 100644 --- a/tests/qemu/publish-hiidb/src/main.rs +++ b/tests/qemu/publish-hiidb/src/main.rs @@ -12,7 +12,7 @@ //! UEFI driver for the QEMU tests of the `uefisettings` HII backend, which a //! `Driver####` load option runs as -//! `publish-hiidb.efi [rt-data|reserved|acpi-nvs|loader-data|rt-data-global-variable-forms|rt-data-readwrite-forms|wakeup-time-forms|password-forms|rt-data-password-forms|nested-password-forms|rt-data-nested-password-forms|enhanced-auth-forms|dropped-password-forms|control-text-forms|formset-skip-forms|disabled-form-forms|short-var-store-forms|rt-data-short-var-store-forms|outside-form-set-password-forms|rt-data-outside-form-set-password-forms|first-store-password-forms|rt-data-first-store-password-forms|keyword-kinds-forms|name-value-password-forms|rt-data-name-value-password-forms|storageless-password-forms|rt-data-storageless-password-forms|padded-strings|string-blocks|acpi-table|acpi-table-hiid|acpi-table-junk-qhii|install-config-tables|export-config|export-config-name-value-password|export-config-storageless-password|rt-properties-no-set-variable|rt-properties-all|rt-properties-version-2|rt-properties-version-2-all|json-config-data|json-config-data-past-region|json-config-data-boot-services|json-config-data-mmio|json-capsule-result|set-auth-variables=PATH[,PATH]...]`: +//! `publish-hiidb.efi [rt-data|reserved|acpi-nvs|loader-data|rt-data-global-variable-forms|rt-data-readwrite-forms|wakeup-time-forms|password-forms|rt-data-password-forms|nested-password-forms|rt-data-nested-password-forms|enhanced-auth-forms|dropped-password-forms|control-text-forms|formset-skip-forms|disabled-form-forms|short-var-store-forms|rt-data-short-var-store-forms|outside-form-set-password-forms|rt-data-outside-form-set-password-forms|first-store-password-forms|rt-data-first-store-password-forms|keyword-kinds-forms|name-value-password-forms|rt-data-name-value-password-forms|storageless-password-forms|rt-data-storageless-password-forms|undeclared-password-forms|rt-data-undeclared-password-forms|padded-strings|string-blocks|acpi-table|acpi-table-hiid|acpi-table-junk-qhii|install-config-tables|export-config|export-config-name-value-password|export-config-storageless-password|export-config-undeclared-password|rt-properties-no-set-variable|rt-properties-all|rt-properties-version-2|rt-properties-version-2-all|json-config-data|json-config-data-past-region|json-config-data-boot-services|json-config-data-mmio|json-capsule-result|set-auth-variables=PATH[,PATH]...]`: //! it copies the HII database into pages of that memory type, one byte in, and points //! the volatile `HiiDB` variable at the copy as `{ u32 length; u64 address }`, the way //! OCP firmware does for `src/lib/hii/extract.rs`. The x86 EFI stub reports @@ -77,6 +77,10 @@ //! whose PASSWORDs are of VarStoreId 0, for a handle with a device path, for `uefisettings.efi` in //! the UEFI Shell; `rt-data-storageless-password-forms` is `rt-data`, after it does the same, so //! that Linux finds them too. +//! `undeclared-password-forms` registers the package lists of [`undeclared_password_forms`], +//! whose PASSWORD is of a VarStoreId that no VarStore declares, for a handle with a device path, +//! for `uefisettings.efi` in the UEFI Shell; `rt-data-undeclared-password-forms` is `rt-data`, +//! after it does the same, so that Linux finds them too. //! `padded-strings` publishes nothing either: it registers the package list of //! [`padded_strings`], which a later `publish-hiidb.efi` exports with the database. //! `string-blocks` does the same with the package list of [`string_blocks`]. @@ -95,9 +99,10 @@ //! handle with a device path and appends a `` of marker bytes for its password //! VarStore: [`exported_config`]. A later `publish-hiidb.efi rt-data` publishes that list too. //! `export-config-name-value-password` does the same with the package lists of -//! [`name_value_password_forms`] and a `` of their VARSTORE_NAME_VALUE, and +//! [`name_value_password_forms`] and a `` of their VARSTORE_NAME_VALUE, //! `export-config-storageless-password` with those of [`storageless_password_forms`] and a -//! `` of their VARSTORE SlBacking. +//! `` of their VARSTORE SlBacking, and `export-config-undeclared-password` with +//! those of [`undeclared_password_forms`] and a `` of their VARSTORE UdBacking. //! `rt-properties-no-set-variable` publishes nothing: it installs an EFI_RT_PROPERTIES_TABLE //! (UEFI 2.11 §4.6.2) whose RuntimeServicesSupported lists every runtime service but //! SetVariable(), as firmware that cannot write variables at runtime publishes, and @@ -145,6 +150,7 @@ mod readwrite_forms; mod short_var_store_forms; mod storageless_password_forms; mod string_blocks; +mod undeclared_password_forms; mod wakeup_time_forms; use alloc::vec::Vec; @@ -376,6 +382,16 @@ fn efi_main() -> Status { {address:#x}" ); }), + Some(Task::RegisterUndeclaredPasswordForms) => register_undeclared_password_forms() + .map(|()| println!("publish-hiidb: registered the undeclared password forms")), + Some(Task::PublishUndeclaredPasswordForms) => register_undeclared_password_forms() + .and_then(|()| publish(MemoryType::RUNTIME_SERVICES_DATA, Database::Exported)) + .map(|(address, length)| { + println!( + "publish-hiidb: undeclared password forms registered; {length} bytes at \ + {address:#x}" + ); + }), Some(Task::RegisterPaddedStrings) => register_package_list( &padded_strings::package_list_bytes(), "registering the padded strings", @@ -411,6 +427,7 @@ fn efi_main() -> Status { Some(Task::ExportConfig) => exported_config::export_config(), Some(Task::ExportNameValueConfig) => exported_config::export_name_value_config(), Some(Task::ExportStoragelessConfig) => exported_config::export_storageless_config(), + Some(Task::ExportUndeclaredConfig) => exported_config::export_undeclared_config(), Some(Task::InstallRtProperties(version, supported)) => { install_rt_properties(version, supported) } @@ -431,9 +448,10 @@ fn efi_main() -> Status { |first-store-password-forms|rt-data-first-store-password-forms|keyword-kinds-forms\ |name-value-password-forms|rt-data-name-value-password-forms\ |storageless-password-forms|rt-data-storageless-password-forms\ + |undeclared-password-forms|rt-data-undeclared-password-forms\ |padded-strings|string-blocks|acpi-table|acpi-table-hiid|acpi-table-junk-qhii\ |install-config-tables|export-config|export-config-name-value-password\ - |export-config-storageless-password\ + |export-config-storageless-password|export-config-undeclared-password\ |rt-properties-no-set-variable\ |rt-properties-all\ |rt-properties-version-2|rt-properties-version-2-all|json-config-data\ @@ -510,6 +528,12 @@ enum Task { /// Do what RegisterStoragelessPasswordForms does, then publish the database in runtime-data /// pages. PublishStoragelessPasswordForms, + /// Register the package lists of [`undeclared_password_forms`] in the HII database for a + /// handle with their device path. + RegisterUndeclaredPasswordForms, + /// Do what RegisterUndeclaredPasswordForms does, then publish the database in runtime-data + /// pages. + PublishUndeclaredPasswordForms, /// Register the package list of [`padded_strings`] in the HII database. RegisterPaddedStrings, /// Create the variable of [`readwrite_forms`] if missing, register its package list in the @@ -538,6 +562,10 @@ enum Task { /// a block of their VARSTORE SlBacking, as the HII Configuration Routing Protocol's /// configuration table. ExportStoragelessConfig, + /// Do what RegisterUndeclaredPasswordForms does, then install ExportConfig()'s results, with + /// a block of their VARSTORE UdBacking, as the HII Configuration Routing Protocol's + /// configuration table. + ExportUndeclaredConfig, /// Install an EFI_RT_PROPERTIES_TABLE of this Version and RuntimeServicesSupported. InstallRtProperties(u16, u32), /// Install this JSON configuration table. @@ -633,6 +661,12 @@ fn load_option() -> Option { Ok(option) if option == cstr16!("rt-data-storageless-password-forms") => { Some(Task::PublishStoragelessPasswordForms) } + Ok(option) if option == cstr16!("undeclared-password-forms") => { + Some(Task::RegisterUndeclaredPasswordForms) + } + Ok(option) if option == cstr16!("rt-data-undeclared-password-forms") => { + Some(Task::PublishUndeclaredPasswordForms) + } Ok(option) if option == cstr16!("padded-strings") => Some(Task::RegisterPaddedStrings), Ok(option) if option == cstr16!("string-blocks") => Some(Task::RegisterStringBlocks), Ok(option) if option == cstr16!("acpi-table") => { @@ -652,6 +686,9 @@ fn load_option() -> Option { Ok(option) if option == cstr16!("export-config-storageless-password") => { Some(Task::ExportStoragelessConfig) } + Ok(option) if option == cstr16!("export-config-undeclared-password") => { + Some(Task::ExportUndeclaredConfig) + } Ok(option) if option == cstr16!("rt-properties-no-set-variable") => { Some(Task::InstallRtProperties( RT_PROPERTIES_TABLE_VERSION, @@ -921,6 +958,21 @@ fn register_storageless_password_forms() -> Result<(), Failure> { }) } +/// Registers the package lists of [`undeclared_password_forms`] for a new handle with their +/// device path, as one driver registers several. +fn register_undeclared_password_forms() -> Result<(), Failure> { + let driver = install_device_path(&undeclared_password_forms::DRIVER_PATH)?; + undeclared_password_forms::package_lists() + .iter() + .try_for_each(|list| { + register_package_list_for( + list, + Some(driver), + "registering the undeclared password forms", + ) + }) +} + /// Creates the variables of [`nested_password_forms`] unless they exist, then registers its /// package list. fn register_nested_password_forms() -> Result<(), Failure> { diff --git a/tests/qemu/publish-hiidb/src/undeclared_password_forms.rs b/tests/qemu/publish-hiidb/src/undeclared_password_forms.rs new file mode 100644 index 0000000..b74df3e --- /dev/null +++ b/tests/qemu/publish-hiidb/src/undeclared_password_forms.rs @@ -0,0 +1,186 @@ +// Copyright 2023 Meta Platforms, Inc. and affiliates. +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +//! The HII package lists that `undeclared-password-forms`, `rt-data-undeclared-password-forms` +//! and `export-config-undeclared-password` register in the HII database for a handle with the +//! device path [`DRIVER_PATH`], for the UEFI Shell scenario `shell-undeclared-password` and the +//! Linux scenarios `hii-undeclared-password` and `hii-undeclared-password-snapshot` of +//! `tests/qemu/scenarios.tsv`. The first list's form set declares the buffer VARSTORE UdBacking, +//! VarStoreId 1, with a UINT32 NUMERIC over its bytes 0..4, of the x-UEFI-ns keyword +//! UndeclaredSetting, and a PASSWORD of VarStoreId 9, which no VarStore of its form package +//! declares, so it names no VarStore: UEFI 2.11 §33.3.8.2.2 asks for "a previously declared +//! variable store"; the second list's form set has a UINT16 NUMERIC over the buffer VARSTORE +//! Other. Its driver, which may serve several package lists, keeps that password where it +//! chooses, so in any variable: `tests/qemu/vars/undeclared-password.json` creates UdBacking +//! holding [`PIN`] where the NUMERIC reads, and Other. The PASSWORD's prompt has no x-UEFI-ns +//! keyword, so no keyword request selects it, and UdBacking is a buffer VARSTORE, not a +//! VARSTORE_EFI, so edk2 resolves the PASSWORD's storage neither when it registers the forms nor +//! when it exports their configuration. + +use alloc::format; +use alloc::string::String; +use alloc::vec::Vec; + +use uefi::guid; +use uefi::Guid; + +use crate::hii_package::end; +use crate::hii_package::form_set_op; +use crate::hii_package::language_strings_package; +use crate::hii_package::op; +use crate::hii_package::package; +use crate::hii_package::package_list; +use crate::hii_package::question_header; +use crate::hii_package::strings_package; +use crate::hii_package::uint16_numeric; +use crate::hii_package::uint32_numeric; +use crate::hii_package::var_store; +use crate::hii_package::EFI_HII_PACKAGE_FORMS; +use crate::hii_package::EFI_IFR_FORM_OP; +use crate::hii_package::EFI_IFR_PASSWORD_OP; +use crate::vendor_device_path; + +/// The GUID of the first package list and of its form set. +const UNDECLARED_FORM_SET: Guid = guid!("8c78a505-baa0-4c4a-869f-000000002300"); +/// The vendor GUID of UdBacking. +const UD_BACKING_VENDOR: Guid = guid!("8c78a505-baa0-4c4a-869f-000000002301"); +/// The GUID of the second package list and of its form set. +const OTHER_FORM_SET: Guid = guid!("8c78a505-baa0-4c4a-869f-000000002302"); +/// The vendor GUID of Other. +const OTHER_VENDOR: Guid = guid!("8c78a505-baa0-4c4a-869f-000000002303"); +/// The Vendor_GUID of the Vendor Device Path node of [`DRIVER_PATH`]. +const DRIVER_VENDOR: Guid = guid!("8c78a505-baa0-4c4a-869f-000000002304"); +/// The device path of the handle that registers both package lists. +pub(crate) static DRIVER_PATH: [u8; 24] = vendor_device_path(DRIVER_VENDOR); +/// The value that `tests/qemu/vars/undeclared-password.json` puts in UdBacking's bytes 0..4, +/// little-endian, standing for a password that the PASSWORD's driver keeps there, and that the +/// `` of [`backing_resp`] holds. +const PIN: u32 = 0x7A4C_19E3; + +// The strings of the first package list, from string ID 2, after the language name. +const BACKING_SETTING: u16 = 2; +const TITLE: u16 = 3; +const HELP: u16 = 4; +const PASSWORD_PROMPT: u16 = 5; +const STRINGS: [&str; 5] = [ + "English", + "Undeclared backing setting", + "uefisettings QEMU undeclared password forms", + "A password question of a VarStoreId that no VarStore declares", + "Undeclared password", +]; +/// The language and the strings, from string ID 1, of the first package list's x-UEFI-ns +/// strings package: the keyword of "Undeclared backing setting" (UEFI 2.11 §35.2.1.6). +const KEYWORD_LANGUAGE: &[u8] = b"x-UEFI-ns\0"; +const KEYWORD_STRINGS: [&str; 2] = ["UefiNameSpace", "UndeclaredSetting"]; + +// The strings of the second package list, likewise. +const OTHER_SETTING: u16 = 2; +const OTHER_TITLE: u16 = 3; +const OTHER_HELP: u16 = 4; +const OTHER_STRINGS: [&str; 4] = [ + "English", + "Undeclared other setting", + "uefisettings QEMU undeclared password other forms", + "A question of another package list of the driver of the undeclared password", +]; + +/// The VarStoreIds of UdBacking and of Other, and the one of the PASSWORD, which no VarStore of +/// its form package declares. +const UD_BACKING_ID: u16 = 1; +const OTHER_ID: u16 = 2; +const UNDECLARED_ID: u16 = 9; +/// The name and the size of UdBacking, and the size of Other. +const UD_BACKING_NAME: &str = "UdBacking"; +const UD_BACKING_SIZE: u16 = 4; +const OTHER_SIZE: u16 = 2; +/// The MinSize and MaxSize of the PASSWORD, in characters. +const PASSWORD_MIN_SIZE: u16 = 4; +const PASSWORD_MAX_SIZE: u16 = 8; + +/// The package lists, as NewPackageList() takes them. +pub(crate) fn package_lists() -> [Vec; 2] { + let keywords = language_strings_package(KEYWORD_LANGUAGE, KEYWORD_STRINGS); + [ + package_list( + UNDECLARED_FORM_SET, + &[strings_package(STRINGS), keywords, undeclared_forms()], + ), + package_list( + OTHER_FORM_SET, + &[strings_package(OTHER_STRINGS), other_forms()], + ), + ] +} + +/// The `` of UdBacking that the driver at [`DRIVER_PATH`] would export, as edk2's +/// HiiConstructConfigHdr and BlockToConfig write one: its NAME in hex CHAR16s and its bytes +/// holding [`PIN`], VALUE's digits most significant first (UEFI 2.11 §35.4.6). +pub(crate) fn backing_resp() -> String { + let hex = |bytes: &[u8]| -> String { bytes.iter().map(|byte| format!("{byte:02x}")).collect() }; + let name: String = UD_BACKING_NAME + .encode_utf16() + .map(|unit| format!("{unit:04x}")) + .collect(); + format!( + "GUID={}&NAME={name}&PATH={}&OFFSET=0&WIDTH={UD_BACKING_SIZE:x}&VALUE={PIN:08x}", + hex(&UD_BACKING_VENDOR.to_bytes()), + hex(&DRIVER_PATH), + ) +} + +/// The first list's form package: its form set, UdBacking and one form that holds the UINT32 +/// NUMERIC over UdBacking and the PASSWORD of VarStoreId 9. +fn undeclared_forms() -> Vec { + let form = [1u16.to_le_bytes(), TITLE.to_le_bytes()].concat(); + let password = [ + question_header(PASSWORD_PROMPT, HELP, 2, UNDECLARED_ID, 0), + [ + PASSWORD_MIN_SIZE.to_le_bytes(), + PASSWORD_MAX_SIZE.to_le_bytes(), + ] + .concat(), + ] + .concat(); + let body = [ + form_set_op(UNDECLARED_FORM_SET, TITLE, HELP), + var_store( + UD_BACKING_ID, + UD_BACKING_VENDOR, + UD_BACKING_SIZE, + &[UD_BACKING_NAME.as_bytes(), &[0]].concat(), + ), + op(EFI_IFR_FORM_OP, true, &form), + uint32_numeric(question_header(BACKING_SETTING, HELP, 1, UD_BACKING_ID, 0)), + op(EFI_IFR_PASSWORD_OP, false, &password), + end(), // of the form + end(), // of the form set + ] + .concat(); + package(EFI_HII_PACKAGE_FORMS, &body) +} + +/// The second list's form package: its form set, Other and one form that holds the UINT16 +/// NUMERIC over Other. +fn other_forms() -> Vec { + let form = [1u16.to_le_bytes(), OTHER_TITLE.to_le_bytes()].concat(); + let body = [ + form_set_op(OTHER_FORM_SET, OTHER_TITLE, OTHER_HELP), + var_store(OTHER_ID, OTHER_VENDOR, OTHER_SIZE, b"Other\0"), + op(EFI_IFR_FORM_OP, true, &form), + uint16_numeric(question_header(OTHER_SETTING, OTHER_HELP, 1, OTHER_ID, 0)), + end(), // of the form + end(), // of the form set + ] + .concat(); + package(EFI_HII_PACKAGE_FORMS, &body) +} diff --git a/tests/qemu/scenarios.tsv b/tests/qemu/scenarios.tsv index d17d4b6..a905666 100644 --- a/tests/qemu/scenarios.tsv +++ b/tests/qemu/scenarios.tsv @@ -46,6 +46,8 @@ hii-name-value-password /publish-hiidb.efi rt-data-name-value-password-forms nam hii-name-value-password-snapshot /publish-hiidb.efi export-config-name-value-password;/publish-hiidb.efi rt-data name-value-password.json hii_name_value_password/with_snapshot:: - hii-storageless-password /publish-hiidb.efi rt-data-storageless-password-forms storageless-password.json hii_storageless_password/without_snapshot:: - hii-storageless-password-snapshot /publish-hiidb.efi export-config-storageless-password;/publish-hiidb.efi rt-data storageless-password.json hii_storageless_password/with_snapshot:: - +hii-undeclared-password /publish-hiidb.efi rt-data-undeclared-password-forms undeclared-password.json hii_undeclared_password/without_snapshot:: - +hii-undeclared-password-snapshot /publish-hiidb.efi export-config-undeclared-password;/publish-hiidb.efi rt-data undeclared-password.json hii_undeclared_password/with_snapshot:: - hii-time-storage /DriverSample.efi;/publish-hiidb.efi wakeup-time-forms my-ifr-nv-data.json hii_time_storage - hii-acpi /DriverSample.efi;/publish-hiidb.efi acpi-table my-ifr-nv-data.json hii_acpi/qhii:: - hii-acpi-hiid /DriverSample.efi;/publish-hiidb.efi acpi-table-hiid my-ifr-nv-data.json hii_acpi/hiid:: - @@ -67,6 +69,7 @@ shell-first-store-password /publish-hiidb.efi first-store-password-forms - shell shell-keyword-kinds /publish-hiidb.efi keyword-kinds-forms - shell:keyword-kinds - shell-name-value-password /publish-hiidb.efi name-value-password-forms name-value-password.json shell:name-value-password - shell-storageless-password /publish-hiidb.efi storageless-password-forms storageless-password.json shell:storageless-password - +shell-undeclared-password /publish-hiidb.efi undeclared-password-forms undeclared-password.json shell:undeclared-password - shell-time-storage /DriverSample.efi;/publish-hiidb.efi wakeup-time-forms my-ifr-nv-data.json shell:time-storage - shell-auth27 /DriverSample.efi my-ifr-nv-data-auth27.json shell:auth27 - hii-auth87 /publish-hiidb.efi enhanced-auth-forms;/publish-hiidb.efi rt-data enhanced-auth-87.json hii_auth87 - diff --git a/tests/qemu/shell/undeclared-password.nsh b/tests/qemu/shell/undeclared-password.nsh new file mode 100644 index 0000000..990fad8 --- /dev/null +++ b/tests/qemu/shell/undeclared-password.nsh @@ -0,0 +1,28 @@ +echo -off +# The boot of the scenario shell-undeclared-password: publish-hiidb.efi undeclared-password-forms +# registers, for one device path, a package list with a PASSWORD of VarStoreId 9, which no VarStore +# of its form package declares, beside the NUMERIC "Undeclared backing setting", of the x-UEFI-ns +# keyword UndeclaredSetting, over bytes 0..4 of UdBacking, and a package list with the NUMERIC +# "Undeclared other setting" over Other; vars/undeclared-password.json creates UdBacking, whose +# bytes 0..4 hold 0x7A4C19E3, and Other. shell/undeclared-password.tsv holds the checks of the +# steps. +fs0: +echo uefisettings-shell: start +dmpstore UdBacking -guid 8C78A505-BAA0-4C4A-869F-000000002301 +echo uefisettings-shell: step dmpstore-before %lasterror% +uefisettings.efi hii get --json "Undeclared backing setting" +echo uefisettings-shell: step get %lasterror% +uefisettings.efi hii get --json "Undeclared other setting" +echo uefisettings-shell: step get-other %lasterror% +uefisettings.efi hii keyword get --namespace x-UEFI-ns "PATH=0104140005a5788ca0ba4a4c869f0000000023047fff0400&KEYWORD=UndeclaredSetting" +echo uefisettings-shell: step keyword-get %lasterror% +uefisettings.efi hii keyword set --i-am-fine-to-brick-this-system "NAMESPACE=x-UEFI-ns&PATH=0104140005a5788ca0ba4a4c869f0000000023047fff0400&KEYWORD=UndeclaredSetting&VALUE=00000001" +echo uefisettings-shell: step keyword-set %lasterror% +uefisettings.efi hii list-questions --json +echo uefisettings-shell: step list-questions %lasterror% +uefisettings.efi hii set --json --i-am-fine-to-brick-this-system "Undeclared backing setting" 1 +echo uefisettings-shell: step set-bypassed %lasterror% +dmpstore UdBacking -guid 8C78A505-BAA0-4C4A-869F-000000002301 +echo uefisettings-shell: step dmpstore-after %lasterror% +echo uefisettings-shell: done +reset -s diff --git a/tests/qemu/shell/undeclared-password.tsv b/tests/qemu/shell/undeclared-password.tsv new file mode 100644 index 0000000..6c835f0 --- /dev/null +++ b/tests/qemu/shell/undeclared-password.tsv @@ -0,0 +1,25 @@ +# The checks of shell/undeclared-password.nsh. See check-shell-log.sh. +# A PASSWORD of VarStoreId 9, which no VarStore of its form package declares, names no VarStore, +# so its password may be in any variable: get, keyword get, keyword set, list-questions and set +# read and write none, of either package list, even with --i-am-fine-to-brick-this-system, which +# lifts none of that, so set and keyword set warn of nothing, and nothing shows 0x7A4C19E3, which +# UdBacking holds where the password may be +dmpstore-before status any +dmpstore-before line ^Variable NV\+RT\+BS '8C78A505-BAA0-4C4A-869F-000000002301:UdBacking' DataSize = 0x04$ +get status aborted +get json . == {"error_message": "PASSWORD question 0x0002 in FormSet 8C78A505-BAA0-4C4A-869F-000000002300: no supported varstore with matching id 0x0009 found; as the variable of that password is unknown, the UEFI Shell build reads and writes no variable of this HiiDB"} +get none 2051807715|7[aA]4[cC]19[eE]3|[eE]3194[cC]7[aA]|[eE]3 19 4[cC] 7[aA]|227, 25, 76, 122 +get-other status aborted +get-other same get +keyword-get status aborted +keyword-get same get +keyword-get none 2051807715|7[aA]4[cC]19[eE]3|[eE]3194[cC]7[aA]|[eE]3 19 4[cC] 7[aA]|227, 25, 76, 122 +keyword-set status aborted +keyword-set same get +keyword-set none 2051807715|7[aA]4[cC]19[eE]3|[eE]3194[cC]7[aA]|[eE]3 19 4[cC] 7[aA]|227, 25, 76, 122 +list-questions status aborted +list-questions same get +set-bypassed status aborted +set-bypassed same get +dmpstore-after status any +dmpstore-after same dmpstore-before diff --git a/tests/qemu/vars/undeclared-password.json b/tests/qemu/vars/undeclared-password.json new file mode 100644 index 0000000..d2a5b37 --- /dev/null +++ b/tests/qemu/vars/undeclared-password.json @@ -0,0 +1,17 @@ +{ + "version": 2, + "variables": [ + { + "name": "UdBacking", + "guid": "8c78a505-baa0-4c4a-869f-000000002301", + "attr": 7, + "data": "E3194C7A" + }, + { + "name": "Other", + "guid": "8c78a505-baa0-4c4a-869f-000000002303", + "attr": 7, + "data": "2211" + } + ] +} From 425599dc23611d0c1da2967a99683991db40de24 Mon Sep 17 00:00:00 2001 From: Dmitrii Okunev Date: Wed, 11 Nov 2026 07:49:12 +0000 Subject: [PATCH 05/56] feat(hii): show STRING questions of name/value storage from the snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UEFI 2.11 §33.2.5.4.9 gives the results of a string question as a hex dump of the string, and a holds a of 4 hex digits per (§35.2.1.2, §35.2.1.4). The ExportConfig() snapshot kept the settings of name/value storage as numbers only, so a STRING there showed no value. The snapshot now keeps each as given, and a question reads it as a number or, for a STRING, as CHAR16s, most significant digit first, as edk2-stable202608 SetupBrowserDxe/Setup.c BufferToValue reads them (:1564-1579): up to a NULL or MaxSize characters, NULLs past them dropped, an empty setting an empty string. Digits that are no , whose last CHAR16 BufferToValue would take from fewer than 4 digits (:1571-1573), or characters other than NULL past MaxSize, which it would cut (:1557-1561), give no value; the decoder of those digits is the one that reads NAME, OPTIONSTRING and DISPLAYNAME. A numeric question of an empty setting gives none either, as BufferToValue writes no number for it (:1580-1590). A PASSWORD that edk2 may route to a VARSTORE_NAME_VALUE, one of VarStoreId 0, or one whose VarStore is unknown keeps the snapshot off as before, so no STRING of any VARSTORE_NAME_VALUE shows beside it. Test Plan: - cargo check --locked --all-targets and cargo test --locked --lib, RUSTFLAGS='-D warnings': 1492 passed, 3 of them new or changed: - nv_config_reads_its_digits_as_a_string; - name_value_strings_take_their_characters_from_the_snapshot, which replaces name_value_strings_take_no_snapshot_value, with a CHECKBOX of an empty setting and an ORDERED_LIST, which reads its setting as a number; - name_value_strings_keep_to_the_password_refusals: STRINGs beside no PASSWORD, one on Backing, one routed to a VARSTORE_NAME_VALUE (on that store, on a second one, on the PASSWORD's own name, and from another package list of the driver), one of VarStoreId 0 in its form and one outside any FormSet, and one of an undeclared VarStoreId, which refuses the HiiDB through variables that read anything too, through get, list-questions and show-ifr, through Runtime and through variables that read nothing. - With the parent's product code, both exports tests fail on the numbers-only reason: 1489 passed, 2 failed. With this product code on the parent of d48dfd4, which refused PASSWORDs of VarStoreId 0, each VarStoreId 0 case shows "AB " through Runtime. - Mutants of this commit's product code, each killed by a named failing test (22): CHAR16s stored big-endian; characters past MaxSize taken, and NULLs past it refused; MaxSize taken in bytes; one character past MaxSize taken; the last character within MaxSize dropped; digits that fill no skipped; the digits of each swapped; the s read in reverse order; digits that are no read as an empty string; an empty setting dropped; a missing setting read as zeros; an empty numeric setting read as 0; the first setting of a name taken; padding with spaces; a setting longer than MaxSize tagged as a read error; an ORDERED_LIST read as a string; the snapshot read beside a PASSWORD whose VarStore is unknown; a PASSWORD that edk2 may route to a VARSTORE_NAME_VALUE kept by its IFR, and leaving the snapshot on; a PASSWORD of VarStoreId 0 in a FormSet, and one outside any, kept by its IFR. Signed-off-by: Dmitrii Okunev --- README.md | 2 +- src/lib/exports.rs | 428 ++++++++++++++++++++++++++++++++- src/lib/hii/config_snapshot.rs | 53 ++-- src/lib/hii/config_string.rs | 39 ++- src/lib/hii/forms/storage.rs | 54 ++++- 5 files changed, 525 insertions(+), 51 deletions(-) diff --git a/README.md b/README.md index 30a06b3..eb31af1 100644 --- a/README.md +++ b/README.md @@ -313,7 +313,7 @@ But this tool is an unified opensource approach to manipulate UEFI settings on a * A date or time question whose UEFI Storage is the system time or the wake-up time, its EFI Date/Time Storage (UEFI 2.11 §33.2.5.6), keeps its value there, in addition to its VarStore, if any. In the UEFI Shell, `get` shows the fields of GetTime()'s or GetWakeupTime()'s time, or the status of the service that fails, and `set` reads that time first, changes the question's fields, keeps the others, the TimeZone and Daylight too, and writes it back with SetTime() or SetWakeupTime(TRUE, …), then writes the question's VarStore, if any; where that write fails, the error names the time written. `set` of a wake-up time thus arms the alarm, which "causes the system to wake up or power on at the set time" (UEFI 2.11 §8.3.4), as EDK2's setup browser does (edk2-stable202608 SetupBrowserDxe/Setup.c:2051). Before any write, `set` refuses a time outside the ranges of UEFI 2.11 §8.3.1, such as a year before 1900. A GET or SET of a date or time without a VarStore, which conditions and WRITE expressions may hold, reads or writes a field of GetTime()'s time, as EDK2's setup browser does. On Linux, Windows and macOS, which reach no Time Services, `get` shows `` (GetWakeupTime() for a wake-up time), also where the ExportConfig snapshot holds the bytes of its VarStore, as that snapshot holds no EFI_TIME, and `set` refuses the question, with every other question that it selects, before any write. * An ordered-list question (UEFI 2.11 §33.2.5.4.8, §33.3.8.3.53) holds its items in up to MaxContainers containers, each as wide as the Type of its first option, as EDK2's setup browser sizes them (edk2-stable202608 SetupBrowserDxe/IfrParse.c), and a container of 0 ends them. `get` and `hii list-questions` show the texts of the options of its items in order, joined by `, `, each with the state of its option, such as `USB