From 1dbad87139223fff9497dc4e6df166acee73c63c Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Tue, 25 Apr 2023 14:20:05 +0200 Subject: [PATCH] cbe: fix propagation of results of unlock attempts Fixes a regression with the cbe_init_trust_anchor component that prevented reacting to a failed unlock attempt in the File Vault. The regression was caused by new semantics in the vfs that did not allow for using the file operation result as indicator for whether the unlock attempt failed or succeeded. The correct and hereby applied approach is to check for the data read from the file after having written the unlock command. The data reads either "ok" or "failed". --- .../app/cbe_init_trust_anchor/component.cc | 25 +++++++++++-------- repos/gems/src/app/cbe_tester/trust_anchor.cc | 20 +++++++++++---- repos/gems/src/app/cbe_tester/trust_anchor.h | 3 ++- .../gems/src/lib/vfs/cbe_trust_anchor/vfs.cc | 25 +++++++++++++++++-- 4 files changed, 54 insertions(+), 19 deletions(-) diff --git a/repos/gems/src/app/cbe_init_trust_anchor/component.cc b/repos/gems/src/app/cbe_init_trust_anchor/component.cc index d8272ad1da..a9ad5dec80 100644 --- a/repos/gems/src/app/cbe_init_trust_anchor/component.cc +++ b/repos/gems/src/app/cbe_init_trust_anchor/component.cc @@ -44,7 +44,7 @@ class Main : Vfs::Env::User Vfs::File_system &_vfs { _vfs_env.root_dir() }; - using Passphrase = Genode::String<32 + 1>; + using Initialize_file_buf = Genode::String<32 + 1>; using String_path = Genode::String<256>; @@ -82,7 +82,7 @@ class Main : Vfs::Env::User Genode::Constructible _io_job { }; Util::Io_job::Buffer _io_buffer { }; - Passphrase _passphrase { }; + Initialize_file_buf _initialize_file_buf { }; File(char const *base_path, char const *name, @@ -112,14 +112,14 @@ class Main : Vfs::Env::User _vfs.close(_vfs_handle); } - void write_passphrase(Passphrase const &passphrase) + void write_passphrase(Initialize_file_buf const &passphrase) { /* copy */ - _passphrase = passphrase; + _initialize_file_buf = passphrase; _io_buffer = { - .base = const_cast(_passphrase.string()), - .size = _passphrase.length() + .base = const_cast(_initialize_file_buf.string()), + .size = _initialize_file_buf.length() }; _io_job.construct(*_vfs_handle, Util::Io_job::Operation::WRITE, @@ -129,8 +129,8 @@ class Main : Vfs::Env::User void queue_read() { _io_buffer = { - .base = nullptr, - .size = 0, + .base = (char *)(_initialize_file_buf.string()), + .size = _initialize_file_buf.length() }; _io_job.construct(*_vfs_handle, Util::Io_job::Operation::READ, @@ -153,7 +153,9 @@ class Main : Vfs::Env::User Completed read_complete() { - return { _io_job->completed(), _io_job->succeeded() }; + return { + _io_job->completed(), + _io_job->succeeded() && _initialize_file_buf == "ok"}; } void drop_io_job() @@ -210,8 +212,9 @@ class Main : Vfs::Env::User { Xml_node const &config { _config_rom.xml() }; - Passphrase const passphrase = - config.attribute_value("passphrase", Passphrase()); + Initialize_file_buf const passphrase = + config.attribute_value("passphrase", Initialize_file_buf()); + if (!passphrase.valid()) { error("mandatory 'passphrase' attribute missing"); throw Missing_config_attribute(); diff --git a/repos/gems/src/app/cbe_tester/trust_anchor.cc b/repos/gems/src/app/cbe_tester/trust_anchor.cc index 260096cc80..96f209aca5 100644 --- a/repos/gems/src/app/cbe_tester/trust_anchor.cc +++ b/repos/gems/src/app/cbe_tester/trust_anchor.cc @@ -132,7 +132,8 @@ void Trust_anchor::_execute_write_read_operation(Vfs_handle &file, void Trust_anchor::_execute_write_operation(Vfs_handle &file, String<128> const &file_path, char const *write_buf, - bool &progress) + bool &progress, + bool result_via_read) { switch (_job.state) { case Job_state::WRITE_PENDING: @@ -168,7 +169,12 @@ void Trust_anchor::_execute_write_operation(Vfs_handle &file, } _job.state = Job_state::READ_PENDING; _job.fl_offset = 0; - _job.fl_size = 0; + + if (result_via_read) + _job.fl_size = sizeof(_read_buf); + else + _job.fl_size = 0; + progress = true; return; @@ -210,7 +216,6 @@ void Trust_anchor::_execute_write_operation(Vfs_handle &file, _job.fl_offset += read_bytes; _job.fl_size -= read_bytes; - _job.request.success(true); if (_job.fl_size > 0) { @@ -218,6 +223,11 @@ void Trust_anchor::_execute_write_operation(Vfs_handle &file, progress = true; return; } + if (result_via_read) { + _job.request.success(!strcmp(_read_buf, "ok", 3)); + } else + _job.request.success(true); + _job.state = Job_state::COMPLETE; progress = true; return; @@ -440,7 +450,7 @@ void Trust_anchor::execute(bool &progress) _execute_write_operation( _initialize_file, _initialize_path, - _job.passphrase.string(), progress); + _job.passphrase.string(), progress, true); break; @@ -448,7 +458,7 @@ void Trust_anchor::execute(bool &progress) _execute_write_operation( _hashsum_file, _hashsum_path, - _job.hash.values, progress); + _job.hash.values, progress, false); break; diff --git a/repos/gems/src/app/cbe_tester/trust_anchor.h b/repos/gems/src/app/cbe_tester/trust_anchor.h index 4f3e598c52..4cea4aa41b 100644 --- a/repos/gems/src/app/cbe_tester/trust_anchor.h +++ b/repos/gems/src/app/cbe_tester/trust_anchor.h @@ -74,7 +74,8 @@ class Trust_anchor void _execute_write_operation(Vfs::Vfs_handle &file, Genode::String<128> const &file_path, char const *write_buf, - bool &progress); + bool &progress, + bool result_via_read); void _execute_read_operation(Vfs::Vfs_handle &file, Genode::String<128> const &file_path, diff --git a/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc b/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc index 0de045feb2..cdda056f18 100644 --- a/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc +++ b/repos/gems/src/lib/vfs/cbe_trust_anchor/vfs.cc @@ -1781,7 +1781,8 @@ class Vfs_cbe_trust_anchor::Initialize_file_system : public Vfs::Single_file_sys _trust_anchor { ta } { } - Read_result read(Byte_range_ptr const &, size_t &) override + Read_result read(Byte_range_ptr const &buf, + size_t &nr_of_read_bytes) override { if (_state != State::PENDING) { return READ_ERR_INVALID; @@ -1799,7 +1800,27 @@ class Vfs_cbe_trust_anchor::Initialize_file_system : public Vfs::Single_file_sys _state = State::NONE; _init_pending = false; - return cr.success ? READ_OK : READ_ERR_IO; + if (cr.success) { + + char const *str { "ok" }; + if (buf.num_bytes < 3) { + Genode::error("read buffer too small ", buf.num_bytes); + return READ_ERR_IO; + } + memcpy(buf.start, str, 3); + nr_of_read_bytes = buf.num_bytes; + + } else { + + char const *str { "failed" }; + if (buf.num_bytes < 7) { + Genode::error("read buffer too small ", buf.num_bytes); + return READ_ERR_IO; + } + memcpy(buf.start, str, 7); + nr_of_read_bytes = buf.num_bytes; + } + return READ_OK; } Write_result write(Const_byte_range_ptr const &src, size_t &out_count) override