Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto, mTLS: allow certificate chain for trusted_client_ca #511

Merged
merged 4 commits into from
Feb 8, 2023
Merged

crypto, mTLS: allow certificate chain for trusted_client_ca #511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9446c4
crypto, mTLS: allow certificate chain for trusted_client_ca
THS-on Feb 1, 2023
db2d1b4
clippy: fix needless_borrow warnings
THS-on Feb 7, 2023
b885bb1
clippy: fix uninlined_format_args warnings
THS-on Feb 8, 2023
d500481
algorithms: fix typo
THS-on Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions keylime-agent/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,7 @@ impl TryFrom<&[u8]> for SymmKey {
Ok(SymmKey { bytes: v.to_vec() })
}
other => Err(format!(
"key length {} does not correspond to valid GCM cipher",
other
"key length {other} does not correspond to valid GCM cipher"
)),
}
}
Expand Down
2 changes: 1 addition & 1 deletion keylime-agent/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ fn config_translate_keywords(
let mut revocation_cert = config_get_file_path(
&config.agent.revocation_cert,
&keylime_dir,
&format!("secure/unzipped/{}", DEFAULT_REVOCATION_CERT),
&format!("secure/unzipped/{DEFAULT_REVOCATION_CERT}"),
);

let tpm_ownerpassword = match config.agent.tpm_ownerpassword {
Expand Down
31 changes: 19 additions & 12 deletions keylime-agent/src/crypto.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ use crate::{

// Read a X509 cert or cert chain and outputs the first certificate
pub(crate) fn load_x509(input_cert_path: &Path) -> Result<X509> {
let contents = read_to_string(input_cert_path)?;
let mut cert_chain = X509::stack_from_pem(contents.as_bytes())?;
let mut cert_chain = load_x509_cert_chain(input_cert_path)?;

if cert_chain.len() != 1 {
return Err(Error::Other(
Expand All @@ -44,6 +43,14 @@ pub(crate) fn load_x509(input_cert_path: &Path) -> Result<X509> {
Ok(cert)
}

pub(crate) fn load_x509_cert_chain(
input_cert_path: &Path,
) -> Result<Vec<X509>> {
let contents = read_to_string(input_cert_path)?;

X509::stack_from_pem(contents.as_bytes()).map_err(Error::Crypto)
}

/// Write a X509 certificate to a file in PEM format
pub(crate) fn write_x509(cert: &X509, file_path: &Path) -> Result<()> {
let mut file = std::fs::File::create(file_path)?;
Expand Down Expand Up @@ -125,8 +132,7 @@ pub(crate) fn pkey_pub_from_priv(
PKey::from_rsa(rsa).map_err(Error::Crypto)
}
id => Err(Error::Other(format!(
"pkey_pub_from_priv not yet implemented for key type {:?}",
id
"pkey_pub_from_priv not yet implemented for key type {id:?}"
))),
}
}
Expand Down Expand Up @@ -161,7 +167,7 @@ pub(crate) fn generate_x509(key: &PKey<Private>, uuid: &str) -> Result<X509> {
pub(crate) fn generate_mtls_context(
mtls_cert: &X509,
key: &PKey<Private>,
keylime_ca_cert: X509,
keylime_ca_certs: Vec<X509>,
) -> Result<SslAcceptorBuilder> {
let mut ssl_context_builder =
SslAcceptor::mozilla_intermediate(SslMethod::tls())?;
Expand All @@ -170,7 +176,10 @@ pub(crate) fn generate_mtls_context(

// Build verification cert store.
let mut mtls_store_builder = X509StoreBuilder::new()?;
mtls_store_builder.add_cert(keylime_ca_cert)?;
for cert in keylime_ca_certs {
mtls_store_builder.add_cert(cert)?;
}

let mtls_store = mtls_store_builder.build();
ssl_context_builder.set_verify_cert_store(mtls_store);

Expand Down Expand Up @@ -313,8 +322,7 @@ pub(crate) fn decrypt_aead(key: &[u8], data: &[u8]) -> Result<Vec<u8>> {
AES_256_KEY_LEN => Cipher::aes_256_gcm(),
other => {
return Err(Error::Other(format!(
"key length {} does not correspond to valid GCM cipher",
other
"key length {other} does not correspond to valid GCM cipher"
)))
}
};
Expand Down Expand Up @@ -385,9 +393,8 @@ pub mod testing {
AES_256_KEY_LEN => Cipher::aes_256_gcm(),
other => {
return Err(Error::Other(format!(
"key length {} does not correspond to valid GCM cipher",
other
)))
"key length {other} does not correspond to valid GCM cipher"
)))
}
};
if iv.len() != AES_BLOCK_SIZE {
Expand Down Expand Up @@ -484,7 +491,7 @@ mod tests {
.join("test-data")
.join("test-rsa.pem");

let (pub_key, priv_key) = rsa_import_pair(&rsa_key_path)
let (pub_key, priv_key) = rsa_import_pair(rsa_key_path)
.expect("unable to import RSA key pair");
let plaintext = b"0123456789012345";
let ciphertext = rsa_oaep_encrypt(&pub_key, &plaintext[..])
Expand Down
11 changes: 4 additions & 7 deletions keylime-agent/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ impl Error {
match self {
Error::Registrar { addr, code } => Ok(*code),
other => Err(Error::Other(format!(
"cannot get http code for Error type {}",
other
"cannot get http code for Error type {other}"
))),
}
}
Expand All @@ -106,8 +105,7 @@ impl Error {
match self {
Error::Execution(code, _) => Ok(code.to_owned()),
other => Err(Error::Other(format!(
"cannot get execution status code for Error type {}",
other
"cannot get execution status code for Error type {other}"
))),
}
}
Expand All @@ -116,8 +114,7 @@ impl Error {
match self {
Error::Execution(_, stderr) => Ok(stderr.to_owned()),
other => Err(Error::Other(format!(
"cannot get stderr for Error type {}",
other
"cannot get stderr for Error type {other}"
))),
}
}
Expand All @@ -139,7 +136,7 @@ impl From<tss_esapi::Error> for Error {
} else {
None
};
let message = format!("{}", err);
let message = format!("{err}");

Error::Tss2 { err, kind, message }
}
Expand Down
7 changes: 3 additions & 4 deletions keylime-agent/src/errors_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,15 @@ pub(crate) async fn app_default(req: HttpRequest) -> impl Responder {
http::Method::GET => {
error = 400;
message = format!(
"Not Implemented: Use /version or /{}/ interfaces",
API_VERSION
"Not Implemented: Use /version or /{API_VERSION}/ interfaces"
);
response = HttpResponse::BadRequest()
.json(JsonWrapper::error(error, &message));
}
http::Method::POST => {
error = 400;
message =
format!("Not Implemented: Use /{}/ interface", API_VERSION);
format!("Not Implemented: Use /{API_VERSION}/ interface");
response = HttpResponse::BadRequest()
.json(JsonWrapper::error(error, &message));
}
Expand Down Expand Up @@ -204,7 +203,7 @@ pub(crate) async fn version_not_supported(
req: HttpRequest,
version: web::Path<APIVersion>,
) -> impl Responder {
let message = format!("API version not supported: {}", version);
let message = format!("API version not supported: {version}");

warn!("{} returning 400 response. {}", req.head().method, message);

Expand Down
21 changes: 9 additions & 12 deletions keylime-agent/src/keys_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,11 @@ mod tests {
App::new()
.app_data(quotedata.clone())
.route(
&format!("/{}/keys/ukey", API_VERSION),
&format!("/{API_VERSION}/keys/ukey"),
web::post().to(u_key),
)
.route(
&format!("/{}/keys/vkey", API_VERSION),
&format!("/{API_VERSION}/keys/vkey"),
web::post().to(v_key),
),
)
Expand Down Expand Up @@ -374,7 +374,7 @@ mod tests {
};

let req = test::TestRequest::post()
.uri(&format!("/{}/keys/ukey", API_VERSION,))
.uri(&format!("/{API_VERSION}/keys/ukey"))
.set_json(&ukey)
.to_request();

Expand All @@ -389,7 +389,7 @@ mod tests {
};

let req = test::TestRequest::post()
.uri(&format!("/{}/keys/vkey", API_VERSION,))
.uri(&format!("/{API_VERSION}/keys/vkey"))
.set_json(&vkey)
.to_request();

Expand Down Expand Up @@ -438,13 +438,13 @@ mod tests {
let quotedata = web::Data::new(QuoteData::fixture().unwrap()); //#[allow_ci]
let mut app =
test::init_service(App::new().app_data(quotedata.clone()).route(
&format!("/{}/keys/pubkey", API_VERSION),
&format!("/{API_VERSION}/keys/pubkey"),
web::get().to(pubkey),
))
.await;

let req = test::TestRequest::get()
.uri(&format!("/{}/keys/pubkey", API_VERSION,))
.uri(&format!("/{API_VERSION}/keys/pubkey"))
.to_request();

let resp = test::call_service(&app, req).await;
Expand All @@ -471,26 +471,23 @@ mod tests {

let mut app =
test::init_service(App::new().app_data(quotedata.clone()).route(
&format!("/{}/keys/verify", API_VERSION),
&format!("/{API_VERSION}/keys/verify"),
web::get().to(verify),
))
.await;

let challenge = "1234567890ABCDEFGHIJ";

let req = test::TestRequest::get()
.uri(&format!(
"/{}/keys/verify?challenge={}",
API_VERSION, challenge
))
.uri(&format!("/{API_VERSION}/keys/verify?challenge={challenge}"))
.to_request();

let resp = test::call_service(&app, req).await;
assert!(resp.status().is_success());

let result: JsonWrapper<KeylimeHMAC> =
test::read_body_json(resp).await;
let response_hmac = hex::decode(&result.results.hmac).unwrap(); //#[allow_ci]
let response_hmac = hex::decode(result.results.hmac).unwrap(); //#[allow_ci]

// The expected result is an HMAC-SHA384 using:
// key (hexadecimal): 000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f
Expand Down
34 changes: 17 additions & 17 deletions keylime-agent/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub(crate) fn optional_unzip_payload(

info!("Unzipping payload {} to {:?}", &zipped_payload, unzipped);

let mut source = fs::File::open(&zipped_payload_path)?;
let mut source = fs::File::open(zipped_payload_path)?;
uncompress_archive(&mut source, unzipped, Ownership::Ignore)?;
}

Expand Down Expand Up @@ -479,8 +479,7 @@ async fn main() -> Result<()> {
ctx.as_mut().tr_set_auth(Hierarchy::Endorsement.into(), auth)
.map_err(|e| {
Error::Configuration(format!(
"Failed to set TPM context password for Endorsement Hierarchy: {}",
e
"Failed to set TPM context password for Endorsement Hierarchy: {e}"
))
})?;
};
Expand Down Expand Up @@ -691,23 +690,24 @@ async fn main() -> Result<()> {
)));
}

let keylime_ca_cert = match crypto::load_x509(&ca_cert_path) {
Ok(t) => Ok(t),
Err(e) => {
error!(
"Failed to load trusted CA certificate {}: {}",
ca_cert_path.display(),
e
);
Err(e)
}
}?;
let keylime_ca_certs =
match crypto::load_x509_cert_chain(&ca_cert_path) {
Ok(t) => Ok(t),
Err(e) => {
error!(
"Failed to load trusted CA certificate {}: {}",
ca_cert_path.display(),
e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy should complain here too, shouldn't?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because you cannot call functions currently in a fromat string, I think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. But I mean "e"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't know. I would keep it this way for now, because mixing the two styles looks wrong.

);
Err(e)
}
}?;

mtls_cert = Some(&cert);
ssl_context = Some(crypto::generate_mtls_context(
&cert,
&nk_priv,
keylime_ca_cert,
keylime_ca_certs,
)?);
} else {
mtls_cert = None;
Expand Down Expand Up @@ -848,7 +848,7 @@ async fn main() -> Result<()> {
.error_handler(errors_handler::path_parser_error),
)
.service(
web::scope(&format!("/{}", API_VERSION))
web::scope(&format!("/{API_VERSION}"))
.service(
web::scope("/keys")
.service(web::resource("/pubkey").route(
Expand Down Expand Up @@ -1007,7 +1007,7 @@ mod testing {
.join("test-rsa.pem");

let (nk_pub, nk_priv) =
crypto::testing::rsa_import_pair(&rsa_key_path)?;
crypto::testing::rsa_import_pair(rsa_key_path)?;

let mut encr_payload = Vec::new();

Expand Down
6 changes: 3 additions & 3 deletions keylime-agent/src/notifications_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub async fn revocation(
Err(e) => {
return HttpResponse::BadRequest().json(JsonWrapper::error(
400,
format!("JSON parsing error: {}", e),
format!("JSON parsing error: {e}"),
));
}
};
Expand Down Expand Up @@ -107,7 +107,7 @@ mod tests {

let mut app =
test::init_service(App::new().app_data(quotedata.clone()).route(
&format!("/{}/notifications/revocation", API_VERSION),
&format!("/{API_VERSION}/notifications/revocation"),
web::post().to(revocation),
))
.await;
Expand All @@ -127,7 +127,7 @@ mod tests {
};

let req = test::TestRequest::post()
.uri(&format!("/{}/notifications/revocation", API_VERSION,))
.uri(&format!("/{API_VERSION}/notifications/revocation",))
.set_json(&revocation)
.to_request();

Expand Down
8 changes: 3 additions & 5 deletions keylime-agent/src/permissions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl TryFrom<&str> for UserIds {
let parts = value.split(':').collect::<Vec<&str>>();

if parts.len() != 2 {
let e = format!("Invalid parameter format: {} cannot be parsed as 'user:group'", value);
let e = format!("Invalid parameter format: {value} cannot be parsed as 'user:group'");
error!("{}", e);
return Err(Error::Conversion(e));
}
Expand All @@ -56,8 +56,7 @@ impl TryFrom<&str> for UserIds {
unsafe { (*p) }
} else {
return Err(Error::Conversion(format!(
"Failed to convert {} to CString",
group
"Failed to convert {group} to CString"
)));
};

Expand All @@ -72,8 +71,7 @@ impl TryFrom<&str> for UserIds {
unsafe { (*p) }
} else {
return Err(Error::Conversion(format!(
"Failed to convert {} to CString",
user
"Failed to convert {user} to CString"
)));
};

Expand Down
Loading