Skip to content

Commit

Permalink
Resolve an injection vulnerability in EKU creation
Browse files Browse the repository at this point in the history
  • Loading branch information
alex committed Mar 24, 2023
1 parent 482575b commit 332311b
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 70 deletions.
5 changes: 5 additions & 0 deletions openssl/src/asn1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use crate::bio::MemBio;
use crate::bn::{BigNum, BigNumRef};
use crate::error::ErrorStack;
use crate::nid::Nid;
use crate::stack::Stackable;
use crate::string::OpensslString;
use crate::{cvt, cvt_p};
use openssl_macros::corresponds;
Expand Down Expand Up @@ -592,6 +593,10 @@ foreign_type_and_impl_send_sync! {
pub struct Asn1ObjectRef;
}

impl Stackable for Asn1Object {
type StackType = ffi::stack_st_ASN1_OBJECT;
}

impl Asn1Object {
/// Constructs an ASN.1 Object Identifier from a string representation of the OID.
#[corresponds(OBJ_txt2obj)]
Expand Down
92 changes: 22 additions & 70 deletions openssl/src/x509/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
//! ```
use std::fmt::Write;

use crate::asn1::Asn1Object;
use crate::error::ErrorStack;
use crate::nid::Nid;
use crate::x509::{Asn1Object, GeneralName, Stack, X509Extension, X509v3Context};
use crate::x509::{GeneralName, Stack, X509Extension, X509v3Context};
use foreign_types::ForeignType;

/// An extension which indicates whether a certificate is a CA certificate.
Expand Down Expand Up @@ -223,18 +224,7 @@ impl KeyUsage {
/// for which the certificate public key can be used for.
pub struct ExtendedKeyUsage {
critical: bool,
server_auth: bool,
client_auth: bool,
code_signing: bool,
email_protection: bool,
time_stamping: bool,
ms_code_ind: bool,
ms_code_com: bool,
ms_ctl_sign: bool,
ms_sgc: bool,
ms_efs: bool,
ns_sgc: bool,
other: Vec<String>,
items: Vec<String>,
}

impl Default for ExtendedKeyUsage {
Expand All @@ -248,18 +238,7 @@ impl ExtendedKeyUsage {
pub fn new() -> ExtendedKeyUsage {
ExtendedKeyUsage {
critical: false,
server_auth: false,
client_auth: false,
code_signing: false,
email_protection: false,
time_stamping: false,
ms_code_ind: false,
ms_code_com: false,
ms_ctl_sign: false,
ms_sgc: false,
ms_efs: false,
ns_sgc: false,
other: vec![],
items: vec![],
}
}

Expand All @@ -271,101 +250,74 @@ impl ExtendedKeyUsage {

/// Sets the `serverAuth` flag to `true`.
pub fn server_auth(&mut self) -> &mut ExtendedKeyUsage {
self.server_auth = true;
self
self.other("serverAuth")
}

/// Sets the `clientAuth` flag to `true`.
pub fn client_auth(&mut self) -> &mut ExtendedKeyUsage {
self.client_auth = true;
self
self.other("clientAuth")
}

/// Sets the `codeSigning` flag to `true`.
pub fn code_signing(&mut self) -> &mut ExtendedKeyUsage {
self.code_signing = true;
self
self.other("codeSigning")
}

/// Sets the `emailProtection` flag to `true`.
pub fn email_protection(&mut self) -> &mut ExtendedKeyUsage {
self.email_protection = true;
self
self.other("emailProtection")
}

/// Sets the `timeStamping` flag to `true`.
pub fn time_stamping(&mut self) -> &mut ExtendedKeyUsage {
self.time_stamping = true;
self
self.other("timeStamping")
}

/// Sets the `msCodeInd` flag to `true`.
pub fn ms_code_ind(&mut self) -> &mut ExtendedKeyUsage {
self.ms_code_ind = true;
self
self.other("msCodeInd")
}

/// Sets the `msCodeCom` flag to `true`.
pub fn ms_code_com(&mut self) -> &mut ExtendedKeyUsage {
self.ms_code_com = true;
self
self.other("msCodeCom")
}

/// Sets the `msCTLSign` flag to `true`.
pub fn ms_ctl_sign(&mut self) -> &mut ExtendedKeyUsage {
self.ms_ctl_sign = true;
self
self.other("msCTLSign")
}

/// Sets the `msSGC` flag to `true`.
pub fn ms_sgc(&mut self) -> &mut ExtendedKeyUsage {
self.ms_sgc = true;
self
self.other("msSGC")
}

/// Sets the `msEFS` flag to `true`.
pub fn ms_efs(&mut self) -> &mut ExtendedKeyUsage {
self.ms_efs = true;
self
self.other("msEFS")
}

/// Sets the `nsSGC` flag to `true`.
pub fn ns_sgc(&mut self) -> &mut ExtendedKeyUsage {
self.ns_sgc = true;
self
self.other("nsSGC")
}

/// Sets a flag not already defined.
pub fn other(&mut self, other: &str) -> &mut ExtendedKeyUsage {
self.other.push(other.to_owned());
self.items.push(other.to_string());
self
}

/// Return the `ExtendedKeyUsage` extension as an `X509Extension`.
pub fn build(&self) -> Result<X509Extension, ErrorStack> {
let mut value = String::new();
let mut first = true;
append(&mut value, &mut first, self.critical, "critical");
append(&mut value, &mut first, self.server_auth, "serverAuth");
append(&mut value, &mut first, self.client_auth, "clientAuth");
append(&mut value, &mut first, self.code_signing, "codeSigning");
append(
&mut value,
&mut first,
self.email_protection,
"emailProtection",
);
append(&mut value, &mut first, self.time_stamping, "timeStamping");
append(&mut value, &mut first, self.ms_code_ind, "msCodeInd");
append(&mut value, &mut first, self.ms_code_com, "msCodeCom");
append(&mut value, &mut first, self.ms_ctl_sign, "msCTLSign");
append(&mut value, &mut first, self.ms_sgc, "msSGC");
append(&mut value, &mut first, self.ms_efs, "msEFS");
append(&mut value, &mut first, self.ns_sgc, "nsSGC");
for other in &self.other {
append(&mut value, &mut first, true, other);
let mut stack = Stack::new()?;
for item in &self.items {
stack.push(Asn1Object::from_str(item)?)?;
}
unsafe {
X509Extension::new_internal(Nid::EXT_KEY_USAGE, self.critical, stack.as_ptr().cast())
}
X509Extension::new_nid(None, None, Nid::EXT_KEY_USAGE, &value)
}
}

Expand Down
8 changes: 8 additions & 0 deletions openssl/src/x509/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,14 @@ fn x509_extension_to_der() {
}
}

#[test]
fn eku_invalid_other() {
assert!(ExtendedKeyUsage::new()
.other("1.1.1.1.1,2.2.2.2.2")
.build()
.is_err());
}

#[test]
fn x509_req_builder() {
let pkey = pkey();
Expand Down

0 comments on commit 332311b

Please sign in to comment.