Skip to content

Commit

Permalink
Merge pull request #283 from cgwalters/ima-sign-enhancements
Browse files Browse the repository at this point in the history 
ima: various fixes
  • Loading branch information
@cgwalters
cgwalters committed Apr 19, 2022
2 parents 1de5ced + 06583e7 commit 77600ca
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 61 deletions.
15 changes: 11 additions & 4 deletions lib/src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,11 @@ struct ImaSignOpts {
/// Digest algorithm
algorithm: String,
/// Path to IMA key
key: String,
key: Utf8PathBuf,

#[structopt(long)]
/// Overwrite any existing signatures
overwrite: bool,
}

/// Options for internal testing
Expand Down Expand Up @@ -541,17 +545,20 @@ async fn container_history(repo: &ostree::Repo, imgref: &OstreeImageReference) -

/// Add IMA signatures to an ostree commit, generating a new commit.
fn ima_sign(cmdopts: &ImaSignOpts) -> Result<()> {
let cancellable = gio::NONE_CANCELLABLE;
let signopts = crate::ima::ImaOpts {
algorithm: cmdopts.algorithm.clone(),
key: cmdopts.key.clone(),
overwrite: cmdopts.overwrite,
};
let tx = cmdopts.repo.auto_transaction(cancellable)?;
let signed_commit = crate::ima::ima_sign(&cmdopts.repo, cmdopts.src_rev.as_str(), &signopts)?;
cmdopts.repo.set_ref_immediate(
cmdopts.repo.transaction_set_ref(
None,
cmdopts.target_ref.as_str(),
Some(signed_commit.as_str()),
gio::NONE_CANCELLABLE,
)?;
);
let _stats = tx.commit(cancellable)?;
println!("{} => {}", cmdopts.target_ref, signed_commit);
Ok(())
}
Expand Down
97 changes: 40 additions & 57 deletions lib/src/ima.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use crate::objgv::*;
use anyhow::{Context, Result};
use camino::Utf8PathBuf;
use cap_std_ext::rustix::fd::BorrowedFd;
use fn_error_context::context;
use gio::glib;
Expand All @@ -20,12 +21,11 @@ use std::fs::File;
use std::ops::DerefMut;
use std::os::unix::io::AsRawFd;
use std::process::{Command, Stdio};
use std::rc::Rc;
use std::{convert::TryInto, io::Seek};

/// Extended attribute keys used for IMA.
const IMA_XATTRS: &[&str] = &["security.ima", "security.evm"];
const SELINUX_XATTR: &[u8] = b"security.selinux\0";
const IMA_XATTR: &str = "security.ima";
const IMA_XATTR_C: &[u8] = b"security.ima\0";

/// Attributes to configure IMA signatures.
#[derive(Debug, Clone)]
Expand All @@ -34,7 +34,10 @@ pub struct ImaOpts {
pub algorithm: String,

/// Path to IMA key
pub key: String,
pub key: Utf8PathBuf,

/// Replace any existing IMA signatures.
pub overwrite: bool,
}

/// Convert a GVariant of type `a(ayay)` to a mutable map
Expand Down Expand Up @@ -69,8 +72,8 @@ struct CommitRewriter<'a> {
repo: &'a ostree::Repo,
ima: &'a ImaOpts,
tempdir: tempfile::TempDir,
/// Files that we already changed
rewritten_files: HashMap<String, Rc<str>>,
/// Maps content object sha256 hex string to a signed object sha256 hex string
rewritten_files: HashMap<String, String>,
}

#[allow(unsafe_code)]
Expand Down Expand Up @@ -113,12 +116,8 @@ impl<'a> CommitRewriter<'a> {
/// evmctl can write a separate file but it picks the name...so
/// we do this hacky dance of `--xattr-user` instead.
#[allow(unsafe_code)]
#[context("Invoking evmctl")]
fn ima_sign(
&self,
instream: &gio::InputStream,
selinux: Option<&Vec<u8>>,
) -> Result<HashMap<Vec<u8>, Vec<u8>>> {
#[context("IMA signing object")]
fn ima_sign(&self, instream: &gio::InputStream) -> Result<HashMap<Vec<u8>, Vec<u8>>> {
let mut tempf = tempfile::NamedTempFile::new_in(self.tempdir.path())?;
// If we're operating on a bare repo, we can clone the file (copy_file_range) directly.
if let Ok(instream) = instream.clone().downcast::<gio::UnixInputStream>() {
Expand All @@ -136,26 +135,11 @@ impl<'a> CommitRewriter<'a> {

let mut proc = Command::new("evmctl");
proc.current_dir(self.tempdir.path())
.args(&[
"sign",
"--portable",
"--xattr-user",
"--key",
self.ima.key.as_str(),
])
.args(&["--hashalgo", self.ima.algorithm.as_str()]);
if let Some(selinux) = selinux {
let selinux = std::str::from_utf8(selinux)
.context("Non-UTF8 selinux value")?
.trim_end_matches('\0');
proc.args(&["--selinux", selinux]);
}

let proc = proc
.arg("--imasig")
.arg(tempf.path().file_name().unwrap())
.stdout(Stdio::null())
.stderr(Stdio::piped());
.stderr(Stdio::piped())
.args(&["ima_sign", "--xattr-user", "--key", self.ima.key.as_str()])
.args(&["--hashalgo", self.ima.algorithm.as_str()])
.arg(tempf.path().file_name().unwrap());
let status = proc.output().context("Spawning evmctl")?;
if !status.status.success() {
return Err(anyhow::anyhow!(
Expand All @@ -165,40 +149,33 @@ impl<'a> CommitRewriter<'a> {
));
}
let mut r = HashMap::new();
for &k in IMA_XATTRS {
let user_k = k.replace("security.", "user.");
let v = steal_xattr(tempf.as_file(), user_k.as_str())?;
// NUL terminate the key
let k = CString::new(k)?.into_bytes_with_nul();
r.insert(k, v);
}
let user_k = IMA_XATTR.replace("security.", "user.");
let v = steal_xattr(tempf.as_file(), user_k.as_str())?;
// NUL terminate the key
let k = CString::new(IMA_XATTR)?.into_bytes_with_nul();
r.insert(k, v);
Ok(r)
}

#[context("Content object {}", checksum)]
fn map_file(&mut self, checksum: &str) -> Result<Rc<str>> {
if let Some(r) = self.rewritten_files.get(checksum) {
return Ok(Rc::clone(r));
}
fn map_file(&mut self, checksum: &str) -> Result<Option<String>> {
let cancellable = gio::NONE_CANCELLABLE;
let (instream, meta, xattrs) = self.repo.load_file(checksum, cancellable)?;
let instream = if let Some(i) = instream {
i
} else {
// If there's no input stream, it must be a symlink. Skip it.
let r: Rc<str> = checksum.into();
self.rewritten_files
.insert(checksum.to_string(), Rc::clone(&r));
return Ok(r);
return Ok(None);
};
let meta = meta.unwrap();
let mut xattrs = xattrs_to_map(&xattrs.unwrap());

let selinux = xattrs.get(SELINUX_XATTR);
let existing_sig = xattrs.remove(IMA_XATTR_C);
if existing_sig.is_some() && !self.ima.overwrite {
return Ok(None);
}

// Now inject the IMA xattr
let xattrs = {
let signed = self.ima_sign(&instream, selinux)?;
let signed = self.ima_sign(&instream)?;
xattrs.extend(signed);
new_variant_a_ayay(&xattrs)
};
Expand All @@ -212,10 +189,7 @@ impl<'a> CommitRewriter<'a> {
.write_content(None, &ostream, size, cancellable)?
.to_hex();

let r: Rc<str> = new_checksum.into();
self.rewritten_files
.insert(checksum.to_string(), Rc::clone(&r));
Ok(r)
Ok(Some(new_checksum))
}

/// Write a dirtree object.
Expand All @@ -237,9 +211,15 @@ impl<'a> CommitRewriter<'a> {
let name = name.to_str();
hex::encode_to_slice(csum, &mut hexbuf)?;
let checksum = std::str::from_utf8(&hexbuf)?;
let mapped = self.map_file(checksum)?;
let mapped = hex::decode(&*mapped)?;
new_files.push((name, mapped));
if let Some(mapped) = self.rewritten_files.get(checksum) {
new_files.push((name, hex::decode(mapped)?));
} else if let Some(mapped) = self.map_file(checksum)? {
let mapped_bytes = hex::decode(&mapped)?;
self.rewritten_files.insert(checksum.into(), mapped);
new_files.push((name, mapped_bytes));
} else {
new_files.push((name, Vec::from(csum)));
}
}

let mut new_dirs = Vec::new();
Expand Down Expand Up @@ -306,6 +286,9 @@ impl<'a> CommitRewriter<'a> {
///
/// The generated commit object will inherit all metadata from the existing commit object
/// such as version, etc.
///
/// This function does not create an ostree transaction; it's recommended to use outside the call
/// to this function.
pub fn ima_sign(repo: &ostree::Repo, ostree_ref: &str, opts: &ImaOpts) -> Result<String> {
let writer = &mut CommitRewriter::new(repo, opts)?;
writer.map_commit(ostree_ref)
Expand Down

0 comments on commit 77600ca

Please sign in to comment.