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.

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

WIP: client certificate revocation checking support. #323

Closed
wants to merge 11 commits into from
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
target
Cargo.lock
/build
.idea
.venv
9 changes: 6 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ read_buf = ["rustls/read_buf"]

[dependencies]
# Keep in sync with RUSTLS_CRATE_VERSION in build.rs
rustls = { version = "=0.21.0", features = [ "dangerous_configuration" ] }
rustls = { version = "=0.21.2", features = [ "dangerous_configuration" ] }
webpki = "0.22"
libc = "0.2"
sct = "0.7"
rustls-pemfile = "0.2.1"
rustls-pemfile = "1.0.3"
log = "0.4.17"

[lib]
name = "rustls_ffi"
crate-type = ["lib", "staticlib"]

[patch.crates-io]
rustls = { git = 'https://github.com/cpu/rustls', branch = 'cpu-crl-support' }
webpki = { package='rustls-webpki', git = 'https://github.com/rustls/webpki.git' }
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::io::Write;
use std::{env, fs, path::PathBuf};

// Keep in sync with Cargo.toml.
const RUSTLS_CRATE_VERSION: &str = "0.21.0";
const RUSTLS_CRATE_VERSION: &str = "0.21.2";

fn main() {
let out_dir = PathBuf::from(env::var_os("OUT_DIR").unwrap());
Expand Down
77 changes: 63 additions & 14 deletions src/cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@ use std::ptr::null;
use std::slice;
use std::sync::Arc;

use rustls::server::{AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient};
use rustls::server::{
AllowAnyAnonymousOrAuthenticatedClient, AllowAnyAuthenticatedClient, UnparsedCertRevocationList,
};
use rustls::sign::CertifiedKey;
use rustls::{
Certificate, PrivateKey, RootCertStore, SupportedCipherSuite, ALL_CIPHER_SUITES,
DEFAULT_CIPHER_SUITES,
};
use rustls_pemfile::{certs, pkcs8_private_keys, rsa_private_keys};

use crate::error::rustls_result;
use crate::error::{map_error, rustls_result};
use crate::rslice::{rustls_slice_bytes, rustls_str};
use crate::{
ffi_panic_boundary, try_box_from_ptr, try_mut_from_ptr, try_ref_from_ptr, try_slice,
ArcCastPtr, BoxCastPtr, CastConstPtr, CastPtr,
ArcCastPtr, BoxCastPtr, CastPtr,
};
use rustls_result::NullParameter;
use std::ops::Deref;
Expand Down Expand Up @@ -517,10 +519,11 @@ pub struct rustls_client_cert_verifier {
_private: [u8; 0],
}

impl CastConstPtr for rustls_client_cert_verifier {
impl CastPtr for rustls_client_cert_verifier {
type RustType = AllowAnyAuthenticatedClient;
}

impl BoxCastPtr for rustls_client_cert_verifier {}
impl ArcCastPtr for rustls_client_cert_verifier {}
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a little bit skeptical these cahnges to the *Ptr traits for the verifiers are correct. Earmarked to dig into the associated macros for closer study.

In particular: worried the change from Arc -> Box on the constructor side will interact poorly with multiple handles to a verifier being used to add CRLs, then being turned back to Arc to give to Webpki 🤔


impl rustls_client_cert_verifier {
Expand All @@ -533,11 +536,35 @@ impl rustls_client_cert_verifier {
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_new(
store: *const rustls_root_cert_store,
) -> *const rustls_client_cert_verifier {
) -> *mut rustls_client_cert_verifier {
ffi_panic_boundary! {
let store: &RootCertStore = try_ref_from_ptr!(store);
let client_cert_verifier = AllowAnyAuthenticatedClient::new(store.clone());
return Arc::into_raw(client_cert_verifier.boxed()) as *const _;
BoxCastPtr::to_mut_ptr(client_cert_verifier)
}
}

/// Add one or more certificate revocation lists (CRLs) to the client certificate
/// verifier using PEM encoded data.
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_add_crl_pem(
verifier: *mut rustls_client_cert_verifier,
pem: *const u8,
pem_len: size_t,
) -> rustls_result {
ffi_panic_boundary! {
let verifier: &mut AllowAnyAuthenticatedClient = try_mut_from_ptr!(verifier);
let crl_pem: &[u8] = try_slice!(pem, pem_len);
let crls_der = match rustls_pemfile::crls(&mut Cursor::new(crl_pem)) {
Ok(vv) => vv,
Err(_) => return rustls_result::CertificateParseError, // TODO(XXX): Better err.
};
for crl_der in crls_der {
if let Err(e) = verifier.add_crl(UnparsedCertRevocationList(crl_der)) {
return map_error(e);
}
}
rustls_result::Ok
}
}

Expand All @@ -548,9 +575,7 @@ impl rustls_client_cert_verifier {
/// consider this pointer unusable after "free"ing it.
/// Calling with NULL is fine. Must not be called twice with the same value.
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_free(
verifier: *const rustls_client_cert_verifier,
) {
pub extern "C" fn rustls_client_cert_verifier_free(verifier: *mut rustls_client_cert_verifier) {
ffi_panic_boundary! {
rustls_client_cert_verifier::free(verifier);
}
Expand All @@ -568,10 +593,11 @@ pub struct rustls_client_cert_verifier_optional {
_private: [u8; 0],
}

impl CastConstPtr for rustls_client_cert_verifier_optional {
impl CastPtr for rustls_client_cert_verifier_optional {
type RustType = AllowAnyAnonymousOrAuthenticatedClient;
}

impl BoxCastPtr for rustls_client_cert_verifier_optional {}
impl ArcCastPtr for rustls_client_cert_verifier_optional {}

impl rustls_client_cert_verifier_optional {
Expand All @@ -583,13 +609,36 @@ impl rustls_client_cert_verifier_optional {
/// ownership of the pointed-to data.
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_optional_new(
store: *const rustls_root_cert_store,
store: *mut rustls_root_cert_store,
) -> *const rustls_client_cert_verifier_optional {
ffi_panic_boundary! {
let store: &RootCertStore = try_ref_from_ptr!(store);
let client_cert_verifier = AllowAnyAnonymousOrAuthenticatedClient::new(store.clone());
return Arc::into_raw(client_cert_verifier.boxed())
as *const _;
BoxCastPtr::to_mut_ptr(client_cert_verifier)
}
}

/// Add one or more certificate revocation lists (CRLs) to the client certificate
/// verifier using PEM encoded data.
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_optional_add_crl_pem(
verifier: *mut rustls_client_cert_verifier_optional,
pem: *const u8,
pem_len: size_t,
) -> rustls_result {
ffi_panic_boundary! {
let verifier: &mut AllowAnyAnonymousOrAuthenticatedClient = try_mut_from_ptr!(verifier);
let crl_pem: &[u8] = try_slice!(pem, pem_len);
let crls_der = match rustls_pemfile::crls(&mut Cursor::new(crl_pem)) {
Ok(vv) => vv,
Err(_) => return rustls_result::CertificateParseError, // TODO(XXX): Better err.
};
for crl_der in crls_der {
if let Err(e) = verifier.add_crl(UnparsedCertRevocationList(crl_der)) {
return map_error(e);
}
}
rustls_result::Ok
}
}

Expand All @@ -601,7 +650,7 @@ impl rustls_client_cert_verifier_optional {
/// Calling with NULL is fine. Must not be called twice with the same value.
#[no_mangle]
pub extern "C" fn rustls_client_cert_verifier_optional_free(
verifier: *const rustls_client_cert_verifier_optional,
verifier: *mut rustls_client_cert_verifier_optional,
) {
ffi_panic_boundary! {
rustls_client_cert_verifier_optional::free(verifier);
Expand Down
6 changes: 2 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ impl ServerCertVerifier for NoneVerifier {
_end_entity: &Certificate,
_intermediates: &[Certificate],
_server_name: &rustls::ServerName,
_scts: &mut dyn Iterator<Item = &[u8]>,
_ocsp_response: &[u8],
_now: SystemTime,
) -> Result<ServerCertVerified, rustls::Error> {
Expand Down Expand Up @@ -232,7 +231,6 @@ impl rustls::client::ServerCertVerifier for Verifier {
end_entity: &Certificate,
intermediates: &[Certificate],
server_name: &rustls::ServerName,
_scts: &mut dyn Iterator<Item = &[u8]>,
ocsp_response: &[u8],
_now: SystemTime,
) -> Result<ServerCertVerified, rustls::Error> {
Expand Down Expand Up @@ -327,7 +325,7 @@ impl rustls_client_config_builder {
ffi_panic_boundary! {
let builder = try_mut_from_ptr!(config_builder);
let root_store: &RootCertStore = try_ref_from_ptr!(roots);
builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(root_store.clone(), None));
builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(root_store.clone()));
rustls_result::Ok
}
}
Expand Down Expand Up @@ -371,7 +369,7 @@ impl rustls_client_config_builder {
return rustls_result::CertificateParseError;
}

config_builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(roots, None));
config_builder.verifier = Arc::new(rustls::client::WebPkiVerifier::new(roots));
rustls_result::Ok
}
}
Expand Down
47 changes: 9 additions & 38 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,14 @@ u32_enum_builder! {
AlertUnknownPSKIdentity => 7231,
AlertCertificateRequired => 7232,
AlertNoApplicationProtocol => 7233,
AlertUnknown => 7234,

// https://docs.rs/sct/latest/sct/enum.Error.html
CertSCTMalformed => 7319,
CertSCTInvalidSignature => 7320,
CertSCTTimestampInFuture => 7321,
CertSCTUnsupportedVersion => 7322,
CertSCTUnknownLog => 7323
AlertUnknown => 7234

// Reserved from previous use.
// CertSCTMalformed => 7319,
// CertSCTInvalidSignature => 7320,
// CertSCTTimestampInFuture => 7321,
// CertSCTUnsupportedVersion => 7322,
// CertSCTUnknownLog => 7323
}
}

Expand Down Expand Up @@ -214,11 +214,6 @@ impl rustls_result {
| CertInvalidPurpose
| CertApplicationVerificationFailure
| CertOtherError
| CertSCTMalformed
| CertSCTInvalidSignature
| CertSCTTimestampInFuture
| CertSCTUnsupportedVersion
| CertSCTUnknownLog
)
}
}
Expand All @@ -244,11 +239,6 @@ pub(crate) fn cert_result_to_error(result: rustls_result) -> rustls::Error {
InvalidCertificate(CertificateError::ApplicationVerificationFailure)
}
CertOtherError => InvalidCertificate(CertificateError::Other(Arc::from(Box::from("")))),
CertSCTMalformed => InvalidSct(sct::Error::MalformedSct),
CertSCTInvalidSignature => InvalidSct(sct::Error::InvalidSignature),
CertSCTTimestampInFuture => InvalidSct(sct::Error::TimestampInFuture),
CertSCTUnsupportedVersion => InvalidSct(sct::Error::UnsupportedSctVersion),
CertSCTUnknownLog => InvalidSct(sct::Error::UnknownLog),
_ => rustls::Error::General("".into()),
}
}
Expand Down Expand Up @@ -279,17 +269,12 @@ fn test_rustls_result_is_cert_error() {
for id in 7121..=7131 {
assert!(rustls_result::rustls_result_is_cert_error(id));
}

// Test SCTError range
for id in 7319..=7323 {
assert!(rustls_result::rustls_result_is_cert_error(id));
}
}

// TODO(@cpu): update for CRL errors.
pub(crate) fn map_error(input: rustls::Error) -> rustls_result {
use rustls::AlertDescription as alert;
use rustls_result::*;
use sct::Error as sct;

match input {
Error::InappropriateMessage { .. } => InappropriateMessage,
Expand Down Expand Up @@ -387,13 +372,6 @@ pub(crate) fn map_error(input: rustls::Error) -> rustls_result {
alert::Unknown(_) => AlertUnknown,
_ => AlertUnknown,
},
Error::InvalidSct(e) => match e {
sct::MalformedSct => CertSCTMalformed,
sct::InvalidSignature => CertSCTInvalidSignature,
sct::TimestampInFuture => CertSCTTimestampInFuture,
sct::UnsupportedSctVersion => CertSCTUnsupportedVersion,
sct::UnknownLog => CertSCTUnknownLog,
},
_ => General,
}
}
Expand All @@ -402,7 +380,6 @@ impl Display for rustls_result {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
use rustls::AlertDescription as alert;
use rustls_result::*;
use sct::Error as sct;

match self {
// These variants are local to this glue layer.
Expand Down Expand Up @@ -578,12 +555,6 @@ impl Display for rustls_result {
AlertCertificateRequired => Error::AlertReceived(alert::CertificateRequired).fmt(f),
AlertNoApplicationProtocol => Error::AlertReceived(alert::NoApplicationProtocol).fmt(f),
AlertUnknown => Error::AlertReceived(alert::Unknown(0)).fmt(f),

CertSCTMalformed => Error::InvalidSct(sct::MalformedSct).fmt(f),
CertSCTInvalidSignature => Error::InvalidSct(sct::InvalidSignature).fmt(f),
CertSCTTimestampInFuture => Error::InvalidSct(sct::TimestampInFuture).fmt(f),
CertSCTUnsupportedVersion => Error::InvalidSct(sct::UnsupportedSctVersion).fmt(f),
CertSCTUnknownLog => Error::InvalidSct(sct::UnknownLog).fmt(f),
}
}
}
29 changes: 20 additions & 9 deletions src/rustls.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ enum rustls_result {
RUSTLS_RESULT_ALERT_CERTIFICATE_REQUIRED = 7232,
RUSTLS_RESULT_ALERT_NO_APPLICATION_PROTOCOL = 7233,
RUSTLS_RESULT_ALERT_UNKNOWN = 7234,
RUSTLS_RESULT_CERT_SCT_MALFORMED = 7319,
RUSTLS_RESULT_CERT_SCT_INVALID_SIGNATURE = 7320,
RUSTLS_RESULT_CERT_SCT_TIMESTAMP_IN_FUTURE = 7321,
RUSTLS_RESULT_CERT_SCT_UNSUPPORTED_VERSION = 7322,
RUSTLS_RESULT_CERT_SCT_UNKNOWN_LOG = 7323,
};
typedef uint32_t rustls_result;

Expand Down Expand Up @@ -952,7 +947,15 @@ void rustls_root_cert_store_free(struct rustls_root_cert_store *store);
* This copies the contents of the rustls_root_cert_store. It does not take
* ownership of the pointed-to memory.
*/
const struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const struct rustls_root_cert_store *store);
struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const struct rustls_root_cert_store *store);

/**
* Add one or more certificate revocation lists (CRLs) to the client certificate
* verifier using PEM encoded data.
*/
rustls_result rustls_client_cert_verifier_add_crl_pem(struct rustls_client_cert_verifier *verifier,
const uint8_t *pem,
size_t pem_len);

/**
* "Free" a verifier previously returned from
Expand All @@ -962,7 +965,7 @@ const struct rustls_client_cert_verifier *rustls_client_cert_verifier_new(const
* consider this pointer unusable after "free"ing it.
* Calling with NULL is fine. Must not be called twice with the same value.
*/
void rustls_client_cert_verifier_free(const struct rustls_client_cert_verifier *verifier);
void rustls_client_cert_verifier_free(struct rustls_client_cert_verifier *verifier);

/**
* Create a new rustls_client_cert_verifier_optional for the root store. The
Expand All @@ -972,7 +975,15 @@ void rustls_client_cert_verifier_free(const struct rustls_client_cert_verifier *
* This copies the contents of the rustls_root_cert_store. It does not take
* ownership of the pointed-to data.
*/
const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_optional_new(const struct rustls_root_cert_store *store);
const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_optional_new(struct rustls_root_cert_store *store);

/**
* Add one or more certificate revocation lists (CRLs) to the client certificate
* verifier using PEM encoded data.
*/
rustls_result rustls_client_cert_verifier_optional_add_crl_pem(struct rustls_client_cert_verifier_optional *verifier,
const uint8_t *pem,
size_t pem_len);

/**
* "Free" a verifier previously returned from
Expand All @@ -982,7 +993,7 @@ const struct rustls_client_cert_verifier_optional *rustls_client_cert_verifier_o
* consider this pointer unusable after "free"ing it.
* Calling with NULL is fine. Must not be called twice with the same value.
*/
void rustls_client_cert_verifier_optional_free(const struct rustls_client_cert_verifier_optional *verifier);
void rustls_client_cert_verifier_optional_free(struct rustls_client_cert_verifier_optional *verifier);

/**
* Create a rustls_client_config_builder. Caller owns the memory and must
Expand Down
12 changes: 12 additions & 0 deletions testdata/test.crl.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-----BEGIN X509 CRL-----
MIIBxjCBrwIBATANBgkqhkiG9w0BAQsFADAgMR4wHAYDVQQDExVtaW5pY2Egcm9v
dCBjYSAxMGE3YTAXDTIzMDcwMzE3MTgxMloXDTIzMDcxMDE3MTgxMlowKTAnAghT
E+2CSHaYmBcNMjMwNzAzMTcxNzU5WjAMMAoGA1UdFQQDCgEBoDAwLjAfBgNVHSME
GDAWgBQ19H4hMuTID22xvBfISviOa+S+EDALBgNVHRQEBAICEAEwDQYJKoZIhvcN
AQELBQADggEBAF5fOpNZGLsHGAUasx5Il79My6EU66igE0YZWVzgX8EaCt1RMCFx
osumXkaPiohICSsczFlnJolpwacsHx/K/IMYvthna8lbAxhuWharRqoHUK+BdTDD
wtThMBC2dCNoLro/6cIpMov9OXjh8291ogIy0qIiSm20JiaWTB+0V7A6gA7riTXC
yzJTyGECLS9XP6rt+SYmcDn0D1jxfsIli0kYBJdKb3O0xF05oBaWadSLuXbcA41+
Kcw07HACaUrR6BCrR3CjnnlTl6Pr25cQi3zPya7lNDQWqhLNx0sU2jviVZQe1nIA
Ie8Ha2syCv0aa33s0dUY6hOKDbLTGpI8f/E=
-----END X509 CRL-----
Loading