Skip to content

Commit

Permalink
[opentitantool] improve user experience
Browse files Browse the repository at this point in the history
When 'opentitiantool image manifest update ...' fails due to any input
or output file name mistyped, or any of the input files does not
exist, the error message is the same:

  Error: No such file or directory (os error 2)

Which does not help much the operator. This patch adds some code which
will print the name of the file causing the error.

Tested by invoking 'image manifest update' with incorrectly specified
various input files.

The same improvement is being made to the bootstrap option, file name
is reported if file read attempt failed.

Signed-off-by: Vadim Bendebury <[email protected]>
  • Loading branch information
Vadim Bendebury committed Sep 20, 2024
1 parent 0df3194 commit 784df87
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 13 deletions.
6 changes: 4 additions & 2 deletions sw/host/opentitanlib/src/image/manifest_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::util::bigint::fixed_size_bigint;
use crate::util::num_de::HexEncoded;
use crate::util::parse_int::ParseInt;

use anyhow::{bail, Result};
use anyhow::{bail, Context, Result};
use serde::{Deserialize, Serialize};
use std::convert::{TryFrom, TryInto};
use std::fmt;
Expand Down Expand Up @@ -96,7 +96,9 @@ macro_rules! manifest_def {

impl ManifestSpec {
pub fn read_from_file(path: &Path) -> Result<ManifestSpec> {
Ok(deser_hjson::from_str(&std::fs::read_to_string(path)?)?)
Ok(deser_hjson::from_str(
&std::fs::read_to_string(path).with_context(|| format!("Failed to open {path:?}"))?,
)?)
}

pub fn overwrite_fields(&mut self, other: ManifestSpec) {
Expand Down
6 changes: 3 additions & 3 deletions sw/host/opentitanlib/src/util/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

use anyhow::{ensure, Result};
use anyhow::{ensure, Context, Result};
use pem_rfc7468::{Decoder, Encoder, LineEnding};
use thiserror::Error;

Expand Down Expand Up @@ -72,7 +72,7 @@ pub trait FromReader: Sized {

/// Reads an instance of `Self` from a binary file at `path`.
fn read_from_file(path: &Path) -> Result<Self> {
let file = File::open(path)?;
let file = File::open(path).with_context(|| format!("Failed to open {path:?}"))?;
Self::from_reader(file)
}
}
Expand All @@ -84,7 +84,7 @@ pub trait ToWriter: Sized {

/// Writes `self` to a file at `path` in binary format.
fn write_to_file(self, path: &Path) -> Result<()> {
let mut file = File::create(path)?;
let mut file = File::create(path).with_context(|| format!("Failed to create {path:?}"))?;
self.to_writer(&mut file)
}
}
Expand Down
5 changes: 3 additions & 2 deletions sw/host/opentitantool/src/command/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

use anyhow::{ensure, Result};
use anyhow::{ensure, Context, Result};
use clap::Args;
use serde_annotate::Annotate;
use std::any::Any;
Expand Down Expand Up @@ -51,7 +51,8 @@ impl BootstrapCommand {
image.parse(&self.filename)?;
image.assemble()
} else {
Ok(std::fs::read(&self.filename[0])?)
Ok(std::fs::read(&self.filename[0])
.with_context(|| format!("Failed to read {}", self.filename[0]))?)
}
}
}
Expand Down
20 changes: 14 additions & 6 deletions sw/host/opentitantool/src/command/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

use anyhow::{ensure, Context, Result};
use anyhow::{bail, ensure, Context, Result};
use clap::{Args, Subcommand};
use serde_annotate::Annotate;
use std::any::Any;
Expand All @@ -26,7 +26,7 @@ use opentitanlib::image::manifest_def::ManifestSpec;
use opentitanlib::image::manifest_ext::{ManifestExtEntry, ManifestExtId, ManifestExtSpec};
use opentitanlib::util::file::{FromReader, ToWriter};
use opentitanlib::util::parse_int::ParseInt;
use sphincsplus::{DecodeKey, SpxDomain, SpxPublicKey, SpxSecretKey};
use sphincsplus::{DecodeKey, SpxDomain, SpxError, SpxPublicKey, SpxSecretKey};

/// Bootstrap the target device.
#[derive(Debug, Args)]
Expand Down Expand Up @@ -232,11 +232,19 @@ impl CommandDispatch for ManifestUpdateCommand {
// Load / write SPX+ public key.
let mut spx_private_key: Option<SpxSecretKey> = None;
if let Some(key) = &self.spx_key {
let (pk, sk) = if let Ok(sk) = SpxSecretKey::read_pem_file(key) {
(SpxPublicKey::from(&sk), Some(sk))
} else {
(SpxPublicKey::read_pem_file(key)?, None)
let (pk, sk) = match SpxSecretKey::read_pem_file(key.clone()) {
Ok(sk) => (SpxPublicKey::from(&sk), Some(sk)),
Err(SpxError::Io(_)) => {
// Return error if file could not be read.
bail!(std::io::Error::new(
std::io::ErrorKind::NotFound,
format!("{} not found", key.display())
));
}
// Maybe it is a public key, try reading it as such.
_ => (SpxPublicKey::read_pem_file(key)?, None),
};

let key_ext = ManifestExtEntry::new_spx_key_entry(&pk)?;
image.add_manifest_extension(key_ext)?;
spx_private_key = sk;
Expand Down

0 comments on commit 784df87

Please sign in to comment.