Skip to content

Commit

Permalink
Address misc security issues (#47)
Browse files Browse the repository at this point in the history
* Limit the size of attestation certs

* Use authority returned from x509 approval response

* x509 authorization checks authority for local-db mode

* Fix test configs
  • Loading branch information
timweri authored Jan 9, 2024
1 parent 7f7bf49 commit fd388b9
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 12 deletions.
Binary file modified examples/example.db
Binary file not shown.
2 changes: 1 addition & 1 deletion rustica-agent-cli/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ pub async fn configure() -> Result<RusticaAgentAction, ConfigurationError> {
return gitconfig::configure_git_config(git_config);
}

if let Some(x509_config) = matches.subcommand_matches("refresh-x509") {
if let Some(x509_config) = matches.subcommand_matches("refresh-attested-x509") {
return refresh_attested_x509_certificate::configure_refresh_x509_certificate(x509_config)
.await;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
-- Your SQL goes here
CREATE TABLE x509_authorizations (
user TEXT NOT NULL,
user TEXT NOT NULL,
hsm_serial TEXT NOT NULL,
require_touch BOOLEAN NOT NULL,
authority TEXT NOT NULL,
PRIMARY KEY (user, hsm_serial)
);
);
2 changes: 1 addition & 1 deletion rustica/src/auth/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl LocalDatabase {

let authorization: Vec<_> = {
use schema::x509_authorizations::dsl::*;
let results = x509_authorizations.filter(user.eq(mtls_user).and(hsm_serial.eq(att_serial.to_string())))
let results = x509_authorizations.filter(user.eq(mtls_user).and(authority.eq(&auth_props.authority).and(hsm_serial.eq(att_serial.to_string()))))
.load::<models::X509Authorization>(&mut conn)
.expect("Error loading authorized hosts");

Expand Down
3 changes: 2 additions & 1 deletion rustica/src/auth/database/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ pub struct X509Authorization {
pub user: String,
pub hsm_serial: String,
pub require_touch: bool,
}
pub authority: String,
}
1 change: 1 addition & 0 deletions rustica/src/auth/database/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ table! {
user -> Text,
hsm_serial -> Text,
require_touch -> Bool,
authority -> Text,
}
}

Expand Down
9 changes: 5 additions & 4 deletions rustica/src/auth/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,7 @@ impl AuthServer {
auth_props: &X509AuthorizationRequestProperties,
) -> Result<X509Authorization, AuthorizationError> {
let mut authorization_request = HashMap::new();
// TODO: This is an anachronistic hold over and should be updated from
// quorum_mtls to rustica_mtls or rustica_x509
authorization_request.insert(format!("type"), "quorum_mtls".to_string());
authorization_request.insert(format!("type"), "rustica_mtls".to_string());
authorization_request.insert("authority".to_string(), auth_props.authority.clone());

// Identities
Expand Down Expand Up @@ -390,7 +388,10 @@ impl AuthServer {

// Success, build the response
return Ok(X509Authorization {
authority: auth_props.authority.clone(),
authority: response
.get("authority")
.ok_or(AuthorizationError::AuthorizerError)?
.to_string(),
issuer: response
.get("issuer")
.unwrap_or(&"Rustica".to_owned())
Expand Down
6 changes: 5 additions & 1 deletion rustica/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ pub enum RusticaServerError {
BadCertOptions = 5,
NotAuthorized = 6,
BadRequest = 7,
PivClientCertTooBig = 8,
PivIntermediateCertTooBig = 9,
U2fAttestationTooBig = 10,
U2fIntermediateCertTooBig = 11,
Unknown = 9001,
}

Expand All @@ -23,4 +27,4 @@ impl From<AuthorizationError> for RusticaServerError {
_ => RusticaServerError::Unknown,
}
}
}
}
27 changes: 27 additions & 0 deletions rustica/src/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,29 @@ use sshcerts::{
};
use std::convert::TryFrom;

// For Yubikey 5 Nano:
// - PIV intermediate cert size is approx 800 bytes
// - PIV client cert is approx 700 bytes
// - U2F intermediate cert is approx 800 bytes
// - U2F attestation statement is approx 256 bytes
const CERT_MAX_SIZE: usize = 1024 * 2; // 2 KiB

/// Verify a provided yubikey attestation certification and intermediate
/// certificate are valid against the Yubico attestation Root CA.
pub fn verify_piv_certificate_chain(
client: &[u8],
intermediate: &[u8],
) -> Result<Key, RusticaServerError> {
// Restrict the max size of certificates
// For Yubikey 5 Nano, actual intermediate cert size is approx 800 bytes
if intermediate.len() > CERT_MAX_SIZE {
return Err(RusticaServerError::PivIntermediateCertTooBig);
}
// For Yubikey 5 Nano, actual client cert size is approx 700 bytes
if client.len() > CERT_MAX_SIZE {
return Err(RusticaServerError::PivClientCertTooBig);
}

// Extract the certificate public key and convert to an sshcerts PublicKey
let validated_piv_data = verify_certificate_chain(client, intermediate, None)
.map_err(|_| RusticaServerError::InvalidKey)?;
Expand Down Expand Up @@ -47,6 +64,16 @@ pub fn verify_u2f_certificate_chain(
application: &[u8],
u2f_challenge_hashed: bool,
) -> Result<Key, RusticaServerError> {
// Restrict the max size for the attestation data and intermediate certificate
// For Yubikey 5 Nano, actual intermediate cert size is approx 800 bytes
if intermediate.len() > CERT_MAX_SIZE {
return Err(RusticaServerError::U2fIntermediateCertTooBig);
}
// For Yubikey 5 Nano, actual auth_data size is approx 256 bytes
if auth_data.len() > CERT_MAX_SIZE {
return Err(RusticaServerError::U2fAttestationTooBig);
}

// Take all the provided data and validate it up to the Yubico U2F Root CA

let challenge = if u2f_challenge_hashed {
Expand Down
4 changes: 2 additions & 2 deletions tests/test_configs/rustica_local_file_alt.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ ECAwQ=
'''

x509_private_key = "MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDDOLp3ZkQZasW1BKZ+fG3ODQgNThvI7pV38DOEFCz6c+gr8whSiV6EHWT04VrddShehZANiAARKbU0hcFy5+9qqHxGx/FBQb2dh6u+pAYh4ASh7skBkPv5DK/46FH6pvyPp6Gfkp8gagcFsr9nAKbwjkVTtBopuhh45KUM5k4VqIqaNox7g+XCrgG29oVqA5WZpW8DFH2c="
x509_private_key_alg = "p384"
x509_private_key_algorithm = "p384"

client_certificate_authority_private_key = "MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDDOLp3ZkQZasW1BKZ+fG3ODQgNThvI7pV38DOEFCz6c+gr8whSiV6EHWT04VrddShehZANiAARKbU0hcFy5+9qqHxGx/FBQb2dh6u+pAYh4ASh7skBkPv5DK/46FH6pvyPp6Gfkp8gagcFsr9nAKbwjkVTtBopuhh45KUM5k4VqIqaNox7g+XCrgG29oVqA5WZpW8DFH2c="
client_certificate_authority_private_key_algorithm = "p384"
Expand All @@ -103,4 +103,4 @@ client_certificate_authority_common_name = "RusticaAccess"
[logging."stdout"]

[authorization."database"]
path = "examples/example.db"
path = "examples/example.db"

0 comments on commit fd388b9

Please sign in to comment.