Skip to content

Commit

Permalink
chore: greater protection for release mode
Browse files Browse the repository at this point in the history
Signed-off-by: Richard Zak <richard@profian.com>
  • Loading branch information
rjzak committed Oct 10, 2022
1 parent 2311adc commit 2be891a
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 22 deletions.
2 changes: 1 addition & 1 deletion src/ext/kvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
fn verify(&self, _cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result<bool> {
if ext.critical {
return Err(anyhow!("kvm extension cannot be critical"));
}
Expand Down
2 changes: 1 addition & 1 deletion src/ext/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool>;
fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result<bool>;
}
2 changes: 1 addition & 1 deletion src/ext/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result<bool> {
if ext.critical {
return Err(anyhow!("sgx extension cannot be critical"));
}
Expand Down
2 changes: 1 addition & 1 deletion src/ext/snp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
fn verify(&self, cri: &CertReqInfo<'_>, ext: &Extension<'_>, dbg: &bool) -> Result<bool> {
if ext.critical {
return Err(anyhow!("snp extension cannot be critical"));
}
Expand Down
101 changes: 83 additions & 18 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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};
Expand Down Expand Up @@ -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<String>,

Expand Down Expand Up @@ -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<String>, hostname: &str) -> anyhow::Result<Self> {
use const_oid::db::rfc5912::SECP_256_R_1 as P256;

Expand Down Expand Up @@ -219,6 +236,8 @@ async fn main() -> anyhow::Result<()> {
let args = confargs::args::<Toml>(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)?,
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -490,8 +527,15 @@ 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());
Expand All @@ -501,6 +545,7 @@ mod tests {
}
}

#[cfg(debug_assertions)]
fn hostname_state() -> State {
State::generate(None, "localhost").unwrap()
}
Expand Down Expand Up @@ -534,6 +579,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();

Expand Down Expand Up @@ -591,10 +637,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)]
Expand All @@ -621,6 +675,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 {
Expand Down Expand Up @@ -658,6 +713,7 @@ mod tests {
assert_eq!(output.issued.len(), five_crs.len());
}

#[cfg(debug_assertions)]
#[rstest]
#[case(PKCS10, false)]
#[case(BUNDLE, true)]
Expand Down Expand Up @@ -686,6 +742,7 @@ mod tests {
}
}

#[cfg(debug_assertions)]
#[rstest]
#[case(PKCS10, false)]
#[case(BUNDLE, true)]
Expand Down Expand Up @@ -715,6 +772,7 @@ mod tests {
}
}

#[cfg(debug_assertions)]
#[rstest]
#[case(PKCS10, false)]
#[case(BUNDLE, true)]
Expand Down Expand Up @@ -745,6 +803,7 @@ mod tests {
attest_response(certificates_state(), response, multi).await;
}

#[cfg(debug_assertions)]
#[rstest]
#[case(PKCS10, false)]
#[case(BUNDLE, true)]
Expand Down Expand Up @@ -776,6 +835,7 @@ mod tests {
attest_response(state, response, multi).await;
}

#[cfg(debug_assertions)]
#[rstest]
#[case(PKCS10, false)]
#[case(BUNDLE, true)]
Expand All @@ -792,6 +852,7 @@ mod tests {
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
}

#[cfg(debug_assertions)]
#[tokio::test]
async fn err_no_attestation_hostname() {
let request = Request::builder()
Expand All @@ -805,6 +866,7 @@ mod tests {
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);
}

#[cfg(debug_assertions)]
#[rstest]
#[case(false)]
#[case(true)]
Expand All @@ -820,6 +882,7 @@ mod tests {
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
}

#[cfg(debug_assertions)]
#[tokio::test]
async fn err_empty_body() {
let request = Request::builder()
Expand All @@ -832,6 +895,7 @@ mod tests {
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
}

#[cfg(debug_assertions)]
#[tokio::test]
async fn err_bad_body() {
let request = Request::builder()
Expand All @@ -844,6 +908,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);
Expand Down
13 changes: 13 additions & 0 deletions testdata/generate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 16 additions & 0 deletions testdata/test.conf
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions testdata/test.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-----BEGIN CERTIFICATE-----
MIIBmzCCAUKgAwIBAgIBYzAKBggqhkjOPQQDAjBWMQswCQYDVQQGEwJVUzEXMBUG
A1UECAwOTm9ydGggQ2Fyb2xpbmExEDAOBgNVBAcMB1JhbGVpZ2gxHDAaBgNVBAMM
E3N0ZXdhcmQucHJvZmlhbi5jb20wIBcNMjIxMDEwMjEzNDI1WhgPMjA1MDAyMjQy
MTM0MjVaMFcxCzAJBgNVBAYTAlVTMRcwFQYDVQQIDA5Ob3J0aCBDYXJvbGluYTEQ
MA4GA1UEBwwHUmFsZWlnaDEdMBsGA1UEAwwUc3Rld2FyZDIucHJvZmlhbi5jb20w
WTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQbabCqJJfsfzPQzHDcMYbtrcj64aHY
Jme3vNPiSsJ2YRYE+DrjxEBOBI4WD03vAXmnpp0jOZNZQe5zz27ZSLc8MAoGCCqG
SM49BAMCA0cAMEQCIHuP3z6/fF6r8jJ03zzvwL1SM7n2QLXLhzynD9o5X1AaAiA3
Zfd0rJN7Y7aYFqro9GsUSSoFwmGBLs/H3uiEvXSOyw==
-----END CERTIFICATE-----
8 changes: 8 additions & 0 deletions testdata/test.csr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-----BEGIN CERTIFICATE REQUEST-----
MIIBEjCBuQIBADBXMQswCQYDVQQGEwJVUzEXMBUGA1UECAwOTm9ydGggQ2Fyb2xp
bmExEDAOBgNVBAcMB1JhbGVpZ2gxHTAbBgNVBAMMFHN0ZXdhcmQyLnByb2ZpYW4u
Y29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEG2mwqiSX7H8z0Mxw3DGG7a3I
+uGh2CZnt7zT4krCdmEWBPg648RATgSOFg9N7wF5p6adIzmTWUHuc89u2Ui3PKAA
MAoGCCqGSM49BAMCA0gAMEUCIQCpgD05/WTApNkLsXoaGPjj/gOEFG6d1KYUcPDH
mc8XLwIgLwmftO4XLlI4evWxvxrGmppi5Bq3IEVugRyRwi9Sg2M=
-----END CERTIFICATE REQUEST-----
5 changes: 5 additions & 0 deletions testdata/test.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgnsotpIzPM5AsQbR3
mjGjYrhOlk76yokFzzklCy8ULeuhRANCAAQbabCqJJfsfzPQzHDcMYbtrcj64aHY
Jme3vNPiSsJ2YRYE+DrjxEBOBI4WD03vAXmnpp0jOZNZQe5zz27ZSLc8
-----END PRIVATE KEY-----

0 comments on commit 2be891a

Please sign in to comment.