From cc1a40b0de30a68eb8b5084dd9f5e087b76109e3 Mon Sep 17 00:00:00 2001 From: Richard Zak Date: Mon, 10 Oct 2022 15:45:29 -0400 Subject: [PATCH] chore: greater protection for release mode Signed-off-by: Richard Zak --- src/ext/kvm.rs | 2 +- src/ext/mod.rs | 2 +- src/ext/sgx/mod.rs | 2 +- src/ext/snp/mod.rs | 2 +- src/main.rs | 114 +++++++++++++++++++++++++++++++++++-------- testdata/generate.sh | 13 +++++ testdata/test.conf | 16 ++++++ testdata/test.crt | 11 +++++ testdata/test.csr | 8 +++ testdata/test.key | 5 ++ 10 files changed, 151 insertions(+), 24 deletions(-) create mode 100644 testdata/test.conf create mode 100644 testdata/test.crt create mode 100644 testdata/test.csr create mode 100644 testdata/test.key diff --git a/src/ext/kvm.rs b/src/ext/kvm.rs index fa9a15dd..3a1af82b 100644 --- a/src/ext/kvm.rs +++ b/src/ext/kvm.rs @@ -19,7 +19,7 @@ impl ExtVerifier for Kvm { const OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.6.1.4.1.58270.1.1"); const ATT: bool = true; - fn verify(&self, _cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: bool) -> Result { + fn verify(&self, _cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result { if ext.critical { return Err(anyhow!("kvm extension cannot be critical")); } diff --git a/src/ext/mod.rs b/src/ext/mod.rs index df54f5c9..7943c7ef 100644 --- a/src/ext/mod.rs +++ b/src/ext/mod.rs @@ -28,5 +28,5 @@ pub trait ExtVerifier { /// certificate. Returning `Ok(false)` will allow the certification request /// to continue, but this particular extension will not be included /// in the resulting certificate. - fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: bool) -> Result; + fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result; } diff --git a/src/ext/sgx/mod.rs b/src/ext/sgx/mod.rs index d2828dbd..47db92cb 100644 --- a/src/ext/sgx/mod.rs +++ b/src/ext/sgx/mod.rs @@ -42,7 +42,7 @@ impl ExtVerifier for Sgx { const OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.6.1.4.1.58270.1.2"); const ATT: bool = true; - fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: bool) -> Result { + fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result { if ext.critical { return Err(anyhow!("sgx extension cannot be critical")); } diff --git a/src/ext/snp/mod.rs b/src/ext/snp/mod.rs index 92db26c0..2185bc15 100644 --- a/src/ext/snp/mod.rs +++ b/src/ext/snp/mod.rs @@ -241,7 +241,7 @@ impl ExtVerifier for Snp { const OID: ObjectIdentifier = ObjectIdentifier::new_unwrap("1.3.6.1.4.1.58270.1.3"); const ATT: bool = true; - fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: bool) -> Result { + fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result { if ext.critical { return Err(anyhow!("snp extension cannot be critical")); } diff --git a/src/main.rs b/src/main.rs index a4551224..ccadfe14 100644 --- a/src/main.rs +++ b/src/main.rs @@ -26,12 +26,15 @@ use axum::routing::{get, post}; use axum::Router; use clap::Parser; use confargs::{prefix_char_filter, Toml}; +#[cfg(debug_assertions)] +use const_oid::db::rfc5280::{ID_CE_BASIC_CONSTRAINTS, ID_CE_KEY_USAGE}; use const_oid::db::rfc5280::{ - ID_CE_BASIC_CONSTRAINTS, ID_CE_EXT_KEY_USAGE, ID_CE_KEY_USAGE, ID_CE_SUBJECT_ALT_NAME, - ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH, + ID_CE_EXT_KEY_USAGE, ID_CE_SUBJECT_ALT_NAME, ID_KP_CLIENT_AUTH, ID_KP_SERVER_AUTH, }; use const_oid::db::rfc5912::ID_EXTENSION_REQ; -use der::asn1::{GeneralizedTime, Ia5StringRef, UIntRef}; +#[cfg(debug_assertions)] +use der::asn1::GeneralizedTime; +use der::asn1::{Ia5StringRef, UIntRef}; use der::{Decode, Encode, Sequence}; use ext::kvm::Kvm; use ext::sgx::Sgx; @@ -48,7 +51,9 @@ use tower_http::LatencyUnit; use tracing::{debug, Level}; use x509::attr::Attribute; use x509::ext::pkix::name::GeneralName; -use x509::ext::pkix::{BasicConstraints, ExtendedKeyUsage, KeyUsage, KeyUsages, SubjectAltName}; +#[cfg(debug_assertions)] +use x509::ext::pkix::{BasicConstraints, KeyUsage, KeyUsages}; +use x509::ext::pkix::{ExtendedKeyUsage, SubjectAltName}; use x509::name::RdnSequence; use x509::request::{CertReq, ExtensionReq}; use x509::time::{Time, Validity}; @@ -80,6 +85,7 @@ struct Args { #[clap(short, long, env = "ROCKET_ADDRESS", default_value = "::")] addr: IpAddr, + #[cfg(debug_assertions)] #[clap(short, long, env = "RENDER_EXTERNAL_HOSTNAME")] host: Option, @@ -141,11 +147,22 @@ impl State { // Validate the syntax of the files. PrivateKeyInfo::from_der(key.as_ref())?; + #[cfg(debug_assertions)] Certificate::from_der(crt.as_ref())?; + #[cfg(not(debug_assertions))] + { + let cert = Certificate::from_der(crt.as_ref())?; + let iss = &cert.tbs_certificate; + if iss.issuer_unique_id == iss.subject_unique_id && iss.issuer == iss.subject { + // A self-signed certificate is not appropriate for Release mode. + return Err(anyhow!("invalid certificate")); + } + } Ok(State { crt, san, key }) } + #[cfg(debug_assertions)] pub fn generate(san: Option, hostname: &str) -> anyhow::Result { use const_oid::db::rfc5912::SECP_256_R_1 as P256; @@ -219,6 +236,8 @@ async fn main() -> anyhow::Result<()> { let args = confargs::args::(prefix_char_filter::<'@'>) .context("Failed to parse config") .map(Args::parse_from)?; + + #[cfg(debug_assertions)] let state = match (args.key, args.crt, args.host) { (None, None, Some(host)) => State::generate(args.san, &host)?, (Some(key), Some(crt), _) => State::load(args.san, key, crt)?, @@ -228,6 +247,15 @@ async fn main() -> anyhow::Result<()> { } }; + #[cfg(not(debug_assertions))] + let state = match (args.key, args.crt) { + (Some(key), Some(crt)) => State::load(args.san, key, crt)?, + _ => { + eprintln!("Specify the public key `--crt` and private key `--key`.\nRun with `--help` for more information."); + return Err(anyhow!("invalid configuration")); + } + }; + #[cfg(not(target_os = "wasi"))] { use std::net::SocketAddr; @@ -316,6 +344,14 @@ fn attest_request( StatusCode::BAD_REQUEST })?; + let dbg = if cfg!(debug_assertions) { + // If the issuer is self-signed, we are in debug mode. + let iss = &issuer.tbs_certificate; + iss.issuer_unique_id == iss.subject_unique_id && iss.issuer == iss.subject + } else { + false + }; + let mut extensions = Vec::new(); let mut attested = false; for Attribute { oid, values } in info.attributes.iter() { @@ -329,16 +365,11 @@ fn attest_request( StatusCode::BAD_REQUEST })?; for ext in Vec::from(ereq) { - // If the issuer is self-signed, we are in debug mode. - let iss = &issuer.tbs_certificate; - let dbg = iss.issuer_unique_id == iss.subject_unique_id; - let dbg = dbg && iss.issuer == iss.subject; - // Validate the extension. let (copy, att) = match ext.extn_id { - Kvm::OID => (Kvm::default().verify(&info, &ext, dbg), Kvm::ATT), - Sgx::OID => (Sgx::default().verify(&info, &ext, dbg), Sgx::ATT), - Snp::OID => (Snp::default().verify(&info, &ext, dbg), Snp::ATT), + Kvm::OID => (Kvm::default().verify(&info, &ext, &dbg), Kvm::ATT), + Sgx::OID => (Sgx::default().verify(&info, &ext, &dbg), Sgx::ATT), + Snp::OID => (Snp::default().verify(&info, &ext, &dbg), Snp::ATT), oid => { debug!("extension `{oid}` is unsupported"); return Err(StatusCode::BAD_REQUEST); @@ -471,16 +502,22 @@ async fn attest( #[cfg(test)] mod tests { mod attest { - use crate::ext::{kvm::Kvm, sgx::Sgx, snp::Snp, ExtVerifier}; + use crate::ext::{kvm::Kvm, ExtVerifier}; + #[cfg(debug_assertions)] + use crate::ext::{sgx::Sgx, snp::Snp}; use crate::*; - use const_oid::db::rfc5912::{ID_EXTENSION_REQ, SECP_256_R_1, SECP_384_R_1}; + #[cfg(debug_assertions)] + use const_oid::db::rfc5912::SECP_384_R_1; + use const_oid::db::rfc5912::{ID_EXTENSION_REQ, SECP_256_R_1}; use const_oid::ObjectIdentifier; use der::{AnyRef, Encode}; use x509::attr::Attribute; use x509::request::{CertReq, CertReqInfo, ExtensionReq}; + #[cfg(debug_assertions)] use x509::PkiPath; use x509::{ext::Extension, name::RdnSequence}; + #[cfg(debug_assertions)] use axum::response::Response; use http::header::CONTENT_TYPE; use http::Request; @@ -490,17 +527,34 @@ mod tests { fn certificates_state() -> State { #[cfg(not(target_os = "wasi"))] - return State::load(None, "testdata/ca.key", "testdata/ca.crt") - .expect("failed to load state"); + { + #[cfg(debug_assertions)] + return State::load(None, "testdata/ca.key", "testdata/ca.crt") + .expect("failed to load state"); + #[cfg(not(debug_assertions))] + return State::load(None, "testdata/test.key", "testdata/test.crt") + .expect("failed to load state"); + } + #[cfg(target_os = "wasi")] { - let crt = std::io::BufReader::new(include_bytes!("../testdata/ca.crt").as_slice()); - let key = std::io::BufReader::new(include_bytes!("../testdata/ca.key").as_slice()); + let (crt, key) = if cfg!(debug_assertions) { + ( + std::io::BufReader::new(include_bytes!("../testdata/ca.crt").as_slice()), + std::io::BufReader::new(include_bytes!("../testdata/ca.key").as_slice()), + ) + } else { + ( + std::io::BufReader::new(include_bytes!("../testdata/test.crt").as_slice()), + std::io::BufReader::new(include_bytes!("../testdata/test.key").as_slice()), + ) + }; State::read(None, key, crt).expect("failed to load state") } } + #[cfg(debug_assertions)] fn hostname_state() -> State { State::generate(None, "localhost").unwrap() } @@ -534,6 +588,7 @@ mod tests { } } + #[cfg(debug_assertions)] async fn attest_response(state: State, response: Response, multi: bool) { let body = hyper::body::to_bytes(response.into_body()).await.unwrap(); @@ -591,10 +646,18 @@ mod tests { .unwrap(); let response = app(certificates_state()).oneshot(request).await.unwrap(); - assert_eq!(response.status(), StatusCode::OK); - attest_response(certificates_state(), response, multi).await; + #[cfg(debug_assertions)] + { + assert_eq!(response.status(), StatusCode::OK); + attest_response(certificates_state(), response, multi).await; + } + #[cfg(not(debug_assertions))] + { + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + } } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -621,6 +684,7 @@ mod tests { // Though similar to the above test, this is the only test which // actually sends many CSRs, versus an array of just one CSR. + #[cfg(debug_assertions)] #[tokio::test] async fn kvm_hostname_many_certs() { let ext = Extension { @@ -658,6 +722,7 @@ mod tests { assert_eq!(output.issued.len(), five_crs.len()); } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -686,6 +751,7 @@ mod tests { } } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -715,6 +781,7 @@ mod tests { } } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -745,6 +812,7 @@ mod tests { attest_response(certificates_state(), response, multi).await; } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -776,6 +844,7 @@ mod tests { attest_response(state, response, multi).await; } + #[cfg(debug_assertions)] #[rstest] #[case(PKCS10, false)] #[case(BUNDLE, true)] @@ -792,6 +861,7 @@ mod tests { assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } + #[cfg(debug_assertions)] #[tokio::test] async fn err_no_attestation_hostname() { let request = Request::builder() @@ -805,6 +875,7 @@ mod tests { assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } + #[cfg(debug_assertions)] #[rstest] #[case(false)] #[case(true)] @@ -820,6 +891,7 @@ mod tests { assert_eq!(response.status(), StatusCode::BAD_REQUEST); } + #[cfg(debug_assertions)] #[tokio::test] async fn err_empty_body() { let request = Request::builder() @@ -832,6 +904,7 @@ mod tests { assert_eq!(response.status(), StatusCode::BAD_REQUEST); } + #[cfg(debug_assertions)] #[tokio::test] async fn err_bad_body() { let request = Request::builder() @@ -844,6 +917,7 @@ mod tests { assert_eq!(response.status(), StatusCode::BAD_REQUEST); } + #[cfg(debug_assertions)] #[tokio::test] async fn err_bad_csr_sig() { let mut cr = cr(SECP_256_R_1, vec![], true); diff --git a/testdata/generate.sh b/testdata/generate.sh index 05a34bbe..48698fe9 100755 --- a/testdata/generate.sh +++ b/testdata/generate.sh @@ -8,3 +8,16 @@ printf "\nGenerating CA certificate\n" openssl req -new -x509 -days 9999 -config ca.conf -key ca.key -out ca.crt printf "\nCA " openssl x509 -noout -text -in ca.crt + +printf "\nGenerating test key\n" +openssl ecparam -genkey -name prime256v1 | openssl pkcs8 -topk8 -nocrypt -out test.key +printf "\nKey " +openssl pkey -noout -text -in test.key + +printf "\nGenerating test cert request\n" +openssl req -new -config test.conf -key test.key -out test.csr + +printf "\nSigning test cert request\n" +openssl x509 -req -in test.csr -days 9999 -CA ca.crt -extfile test.conf -CAkey ca.key -set_serial 99 -out test.crt +printf "\nCert " +openssl x509 -noout -text -in test.crt diff --git a/testdata/test.conf b/testdata/test.conf new file mode 100644 index 00000000..51782dea --- /dev/null +++ b/testdata/test.conf @@ -0,0 +1,16 @@ +[req] +distinguished_name = req_distinguished_name +prompt = no +x509_extensions = v3_ca + +[req_distinguished_name] +C = US +ST = North Carolina +L = Raleigh +CN = steward2.profian.com + +[v3_ca] +basicConstraints = critical,CA:TRUE +keyUsage = cRLSign, keyCertSign +nsComment = "CA certificate" +subjectKeyIdentifier = hash diff --git a/testdata/test.crt b/testdata/test.crt new file mode 100644 index 00000000..96df069e --- /dev/null +++ b/testdata/test.crt @@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBmzCCAUKgAwIBAgIBYzAKBggqhkjOPQQDAjBWMQswCQYDVQQGEwJVUzEXMBUG +A1UECAwOTm9ydGggQ2Fyb2xpbmExEDAOBgNVBAcMB1JhbGVpZ2gxHDAaBgNVBAMM +E3N0ZXdhcmQucHJvZmlhbi5jb20wIBcNMjIxMDEwMjEzNDI1WhgPMjA1MDAyMjQy +MTM0MjVaMFcxCzAJBgNVBAYTAlVTMRcwFQYDVQQIDA5Ob3J0aCBDYXJvbGluYTEQ +MA4GA1UEBwwHUmFsZWlnaDEdMBsGA1UEAwwUc3Rld2FyZDIucHJvZmlhbi5jb20w +WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQbabCqJJfsfzPQzHDcMYbtrcj64aHY +Jme3vNPiSsJ2YRYE+DrjxEBOBI4WD03vAXmnpp0jOZNZQe5zz27ZSLc8MAoGCCqG +SM49BAMCA0cAMEQCIHuP3z6/fF6r8jJ03zzvwL1SM7n2QLXLhzynD9o5X1AaAiA3 +Zfd0rJN7Y7aYFqro9GsUSSoFwmGBLs/H3uiEvXSOyw== +-----END CERTIFICATE----- diff --git a/testdata/test.csr b/testdata/test.csr new file mode 100644 index 00000000..6d36da65 --- /dev/null +++ b/testdata/test.csr @@ -0,0 +1,8 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIIBEjCBuQIBADBXMQswCQYDVQQGEwJVUzEXMBUGA1UECAwOTm9ydGggQ2Fyb2xp +bmExEDAOBgNVBAcMB1JhbGVpZ2gxHTAbBgNVBAMMFHN0ZXdhcmQyLnByb2ZpYW4u +Y29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEG2mwqiSX7H8z0Mxw3DGG7a3I ++uGh2CZnt7zT4krCdmEWBPg648RATgSOFg9N7wF5p6adIzmTWUHuc89u2Ui3PKAA +MAoGCCqGSM49BAMCA0gAMEUCIQCpgD05/WTApNkLsXoaGPjj/gOEFG6d1KYUcPDH +mc8XLwIgLwmftO4XLlI4evWxvxrGmppi5Bq3IEVugRyRwi9Sg2M= +-----END CERTIFICATE REQUEST----- diff --git a/testdata/test.key b/testdata/test.key new file mode 100644 index 00000000..b56b34cb --- /dev/null +++ b/testdata/test.key @@ -0,0 +1,5 @@ +-----BEGIN PRIVATE KEY----- +MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgnsotpIzPM5AsQbR3 +mjGjYrhOlk76yokFzzklCy8ULeuhRANCAAQbabCqJJfsfzPQzHDcMYbtrcj64aHY +Jme3vNPiSsJ2YRYE+DrjxEBOBI4WD03vAXmnpp0jOZNZQe5zz27ZSLc8 +-----END PRIVATE KEY-----