rust/rustls
rust
/
rustls
mirror of https://github.com/ctz/rustls
1
0
Fork 0
Code Issues Releases Wiki Activity

Use webpki 0.18 to validate SNI hostname before cert selection. 

Temporarily use webpki from its Git repo, until the next version of it
is released.
This commit is contained in:
Brian Smith 2017-08-29 14:32:01 -10:00
parent 91f249c00d
commit 270b792594
7 changed files with 92 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Cargo.toml
View File

@ -14,7 +14,7 @@ untrusted = "0.5.1"
base64 = "0.6"
log = { version = "0.3.6", optional = true }
ring = { version = "0.12", features = ["rsa_signing"] }
webpki = "0.17"
webpki = { git = "https://github.com/briansmith/webpki" }
sct = "0.2"
[features]
@ -29,7 +29,7 @@ mio = "0.6"
docopt = "0.8"
serde = "1.0"
serde_derive = "1.0"
webpki-roots = "0.13"
webpki-roots = { git = "https://github.com/briansmith/webpki-roots", branch = "webpki-github" }
ct-logs = "0.2"
regex = "0.2"

4
src/error.rs
View File

@ -63,6 +63,9 @@ pub enum TLSError {
/// We failed to figure out what time it currently is.
FailedToGetCurrentTime,
/// A syntactically-invalid DNS hostname was given.
InvalidDNSName(String),
}
fn join<T: fmt::Debug>(items: &[T]) -> String {
@ -122,6 +125,7 @@ impl Error for TLSError {
TLSError::InvalidSCT(_) => "invalid certificate timestamp",
TLSError::General(_) => "unexpected error", // (please file a bug),
TLSError::FailedToGetCurrentTime => "failed to get current time",
TLSError::InvalidDNSName(_) => "Invalid DNS name",
}
}
}

15
src/msgs/handshake.rs
View File

@ -371,21 +371,6 @@ impl ConvertServerNameList for ServerNameRequest {
}
}
pub fn same_hostname_or_both_none(a: Option<&ServerName>,
b: Option<&ServerName>) -> bool {
match (a, b) {
(Some(a), Some(b)) => {
match (&a.payload, &b.payload) {
(&ServerNamePayload::HostName(ref a_str),
&ServerNamePayload::HostName(ref b_str)) => a_str == b_str,
(_, _) => false,
}
},
(None, None) => true,
_ => false,
}
}
pub type ProtocolNameList = VecU16OfPayloadU8;
pub trait ConvertProtocolNameList {

17
src/msgs/persist.rs
View File

@ -1,10 +1,13 @@
use msgs::handshake::SessionID;
use msgs::enums::{CipherSuite, ProtocolVersion};
use msgs::codec::{Reader, Codec};
use msgs::handshake::{CertificatePayload, ServerName};
use msgs::handshake::CertificatePayload;
use msgs::base::{PayloadU8, PayloadU16};
use msgs::codec;
use webpki;
use untrusted;
use std::mem;
use std::cmp;
@ -153,7 +156,7 @@ pub type ServerSessionKey = SessionID;
#[derive(Debug)]
pub struct ServerSessionValue {
pub sni: Option<ServerName>,
pub sni: Option<webpki::DNSName>,
pub version: ProtocolVersion,
pub cipher_suite: CipherSuite,
pub master_secret: PayloadU8,
@ -165,7 +168,8 @@ impl Codec for ServerSessionValue {
fn encode(&self, bytes: &mut Vec<u8>) {
if let &Some(ref sni) = &self.sni {
codec::encode_u8(1, bytes);
sni.encode(bytes);
let sni_bytes: &str = sni.as_ref().into();
PayloadU8::new(Vec::from(sni_bytes)).encode(bytes);
} else {
codec::encode_u8(0, bytes);
}
@ -181,7 +185,10 @@ impl Codec for ServerSessionValue {
fn read(r: &mut Reader) -> Option<ServerSessionValue> {
let has_sni = try_ret!(codec::read_u8(r));
let sni = if has_sni == 1 {
Some(try_ret!(ServerName::read(r)))
let dns_name = try_ret!(PayloadU8::read(r));
let dns_name = try_ret!(webpki::DNSNameRef::try_from_ascii(
untrusted::Input::from(&dns_name.0)).ok());
Some(dns_name.into())
} else {
None
};
@ -207,7 +214,7 @@ impl Codec for ServerSessionValue {
}
impl ServerSessionValue {
pub fn new(sni: Option<&ServerName>,
pub fn new(sni: Option<&webpki::DNSName>,
v: ProtocolVersion,
cs: CipherSuite,
ms: Vec<u8>,

97
src/server/hs.rs
View File

@ -7,8 +7,7 @@ use msgs::base::{Payload, PayloadU8};
use msgs::handshake::{HandshakePayload, SupportedSignatureSchemes};
use msgs::handshake::{HandshakeMessagePayload, ServerHelloPayload, Random};
use msgs::handshake::{ClientHelloPayload, ServerExtension, SessionID};
use msgs::handshake::{ConvertProtocolNameList, ConvertServerNameList,
same_hostname_or_both_none, ServerName};
use msgs::handshake::{ConvertProtocolNameList, ConvertServerNameList};
use msgs::handshake::{NamedGroups, SupportedGroups, ClientExtension};
use msgs::handshake::{ECPointFormatList, SupportedPointFormats};
use msgs::handshake::{ServerECDHParams, DigitallySignedStruct};
@ -91,12 +90,26 @@ fn can_resume(sess: &ServerSessionImpl,
resume.cipher_suite == sess.common.get_suite().suite &&
(resume.extended_ms == handshake.using_ems ||
(resume.extended_ms && !handshake.using_ems)) &&
same_hostname_or_both_none(resume.sni.as_ref(), sess.sni.as_ref())
same_dns_name_or_both_none(resume.sni.as_ref(), sess.sni.as_ref())
} else {
false
}
}
// Require an exact match for the purpose of comparing SNI DNS Names from two
// client hellos, even though a case-insensitive comparison might also be OK.
fn same_dns_name_or_both_none(a: Option<&webpki::DNSName>,
b: Option<&webpki::DNSName>) -> bool {
match (a, b) {
(Some(a), Some(b)) => {
let a: &str = a.as_ref().into();
let b: &str = b.as_ref().into();
a == b
},
(None, None) => true,
_ => false,
}
}
// Changing the keys must not span any fragmented handshake
// messages. Otherwise the defragmented messages will have
@ -724,7 +737,7 @@ impl ExpectClientHello {
fn start_resumption(mut self,
sess: &mut ServerSessionImpl,
client_hello: &ClientHelloPayload,
sni: Option<&ServerName>,
sni: Option<&webpki::DNSName>,
id: &SessionID,
resumedata: persist::ServerSessionValue)
-> StateResult {
@ -750,7 +763,7 @@ impl ExpectClientHello {
emit_ccs(sess);
emit_finished(&mut self.handshake, sess);
assert!(same_hostname_or_both_none(sni, sess.get_sni()));
assert!(same_dns_name_or_both_none(sni, sess.get_sni()));
Ok(self.into_expect_tls12_ccs())
}
@ -806,10 +819,8 @@ impl ExpectClientHello {
}
}
self.cross_check_certificate_and_save_sni(sess,
client_hello.get_sni_extension()
.and_then(|sni| sni.get_hostname()),
&server_key)?;
let sni = self.get_sni_dns_name(sess, client_hello)?;
self.cross_check_certificate_and_save_sni(sess, sni, &server_key)?;
let chosen_group = chosen_group.unwrap();
let chosen_share = shares_ext.iter()
@ -887,9 +898,23 @@ impl ExpectClientHello {
}
}
fn get_sni_dns_name(&self, sess: &mut ServerSessionImpl,
client_hello: &ClientHelloPayload)
-> Result<Option<webpki::DNSName>, TLSError> {
match client_hello.get_sni_extension()
.and_then(|sni| sni.get_hostname())
.and_then(|sni| sni.get_hostname_str()) {
Some(sni) =>
webpki::DNSNameRef::try_from_ascii_str(sni)
.map(|dns_name_ref| Some(webpki::DNSName::from(dns_name_ref)))
.map_err(|()| illegal_param(sess, "ClientHello SNI DNS name is invalid.")),
None => Ok(None),
}
}
fn cross_check_certificate_and_save_sni(&self,
sess: &mut ServerSessionImpl,
sni: Option<&ServerName>,
sni: Option<webpki::DNSName>,
certkey: &sign::CertifiedKey) -> Result<(), TLSError> {
// Always reject an empty certificate chain.
let end_entity_cert = certkey.end_entity_cert().map_err(|()| {
@ -905,10 +930,7 @@ impl ExpectClientHello {
"end-entity certificate in certificate chain is syntactically invalid".to_string())
})?;
if let Some(sni) = sni {
let sni_str = sni.get_hostname_str()
.unwrap();
if let Some(ref sni) = sni {
// If SNI was offered then the certificate must be valid for
// that hostname. Note that this doesn't fully validate that the
// certificate is valid; it only validates that the name is one
@ -916,18 +938,17 @@ impl ExpectClientHello {
// valid. Indirectly, this also verifies that the SNI is a
// syntactically-valid hostname, according to Web PKI rules,
// which may differ from DNS and/or URL rules.
if !end_entity_cert.verify_is_valid_for_dns_name(
untrusted::Input::from(sni_str.as_bytes())).is_ok() {
if !end_entity_cert.verify_is_valid_for_dns_name(sni.as_ref()).is_ok() {
sess.common.send_fatal_alert(AlertDescription::InternalError);
return Err(TLSError::General(
"The server certificate is not valid for the given SNI name".to_string()));
}
// Save the SNI into the session after it's been validated.
sess.set_sni(sni);
sess.set_sni(sni.clone());
}
assert!(same_hostname_or_both_none(sni, sess.get_sni()));
assert!(same_dns_name_or_both_none(sni.as_ref(), sess.get_sni()));
Ok(())
}
}
@ -979,22 +1000,28 @@ impl State for ExpectClientHello {
// Common to TLS1.2 and TLS1.3: ciphersuite and certificate selection.
let default_sigschemes_ext = SupportedSignatureSchemes::default();
let sni = client_hello.get_sni_extension()
.and_then(|sni| sni.get_hostname());
// Extract and validate the SNI DNS name, if any, before giving it to
// the cert resolver. In particular, if it is invalid then we should
// send an Illegal Parameter alert instead of the Internal Error alert
// (or whatever) that we'd send if this were checked later or in a
// different way.
let sni = self.get_sni_dns_name(sess, client_hello)?;
let sigschemes_ext = client_hello.get_sigalgs_extension()
.unwrap_or(&default_sigschemes_ext);
// Choose a certificate.
let sni_str = sni.and_then(|sni| sni.get_hostname_str());
// XXX: Ideally we'd verify that the SNI hostname is syntactically valid
// before logging it and before giving it to the cert_resolver.
debug!("sni {:?}", sni_str);
let sigschemes_ext = client_hello.get_sigalgs_extension()
.unwrap_or(&default_sigschemes_ext);
debug!("sig schemes {:?}", sigschemes_ext);
let certkey = sess.config.cert_resolver.resolve(sni_str, sigschemes_ext);
let mut certkey = certkey.ok_or_else(|| {
sess.common.send_fatal_alert(AlertDescription::AccessDenied);
TLSError::General("no server certificate chain resolved".to_string())
})?;
let mut certkey = {
let sni_str: Option<&str> =
sni.as_ref().map(|dns_name| dns_name.as_ref().into());
debug!("sni {:?}", sni_str);
debug!("sig schemes {:?}", sigschemes_ext);
let certkey = sess.config.cert_resolver.resolve(sni_str, sigschemes_ext);
certkey.ok_or_else(|| {
sess.common.send_fatal_alert(AlertDescription::AccessDenied);
TLSError::General("no server certificate chain resolved".to_string())
})?
};
// Reduce our supported ciphersuites by the certificate.
// (no-op for TLS1.3)
@ -1030,7 +1057,7 @@ impl State for ExpectClientHello {
}
// -- TLS1.2 only from hereon in --
self.cross_check_certificate_and_save_sni(sess, sni, &certkey)?;
self.cross_check_certificate_and_save_sni(sess, sni.clone(), &certkey)?;
self.handshake.transcript.add_message(&m);
// Save their Random.
@ -1080,7 +1107,7 @@ impl State for ExpectClientHello {
if can_resume(sess, &self.handshake, &maybe_resume) {
return self.start_resumption(sess,
client_hello, sni,
client_hello, sni.as_ref(),
&client_hello.session_id,
maybe_resume.unwrap());
} else {
@ -1098,7 +1125,7 @@ impl State for ExpectClientHello {
if can_resume(sess, &self.handshake, &maybe_resume) {
return self.start_resumption(sess,
client_hello, sni,
client_hello, sni.as_ref(),
&client_hello.session_id,
maybe_resume.unwrap());
}

13
src/server/mod.rs
View File

@ -2,7 +2,7 @@ use session::{Session, SessionSecrets, SessionCommon};
use suites::{SupportedCipherSuite, ALL_CIPHERSUITES};
use msgs::enums::{ContentType, SignatureScheme};
use msgs::enums::{AlertDescription, HandshakeType, ProtocolVersion};
use msgs::handshake::{SessionID, ServerName};
use msgs::handshake::SessionID;
use msgs::message::Message;
use msgs::codec::Codec;
use error::TLSError;
@ -11,6 +11,7 @@ use sign;
use verify;
use anchors;
use key;
use webpki;
use std::collections;
use std::sync::{Arc, Mutex};
@ -400,7 +401,7 @@ pub struct ServerSessionImpl {
pub config: Arc<ServerConfig>,
pub secrets: Option<SessionSecrets>,
pub common: SessionCommon,
sni: Option<ServerName>,
sni: Option<webpki::DNSName>,
pub alpn_protocol: Option<String>,
pub error: Option<TLSError>,
pub state: Option<Box<hs::State + Send>>,
@ -556,14 +557,14 @@ impl ServerSessionImpl {
self.common.negotiated_version
}
pub fn get_sni(&self)-> Option<&ServerName> {
pub fn get_sni(&self)-> Option<&webpki::DNSName> {
self.sni.as_ref()
}
pub fn set_sni(&mut self, value: &ServerName) {
pub fn set_sni(&mut self, value: webpki::DNSName) {
// The SNI hostname is immutable once set.
assert!(self.sni.is_none());
self.sni = Some(value.clone())
self.sni = Some(value)
}
}
@ -601,7 +602,7 @@ impl ServerSession {
/// The SNI hostname is also used to match sessions during session
/// resumption.
pub fn get_sni_hostname(&self)-> Option<&str> {
self.imp.get_sni().and_then(|s| s.get_hostname_str())
self.imp.get_sni().map(|s| s.as_ref().into())
}
}

6
src/verify.rs
View File

@ -93,7 +93,11 @@ impl ServerCertVerifier for WebPKIVerifier {
info!("Unvalidated OCSP response: {:?}", ocsp_response.to_vec());
}
cert.verify_is_valid_for_dns_name(untrusted::Input::from(dns_name.as_bytes()))
let dns_name = webpki::DNSNameRef::try_from_ascii(
untrusted::Input::from(dns_name.as_bytes()))
.map_err(|()| TLSError::InvalidDNSName(dns_name.into()))?;
cert.verify_is_valid_for_dns_name(dns_name)
.map_err(TLSError::WebPKIError)
.map(|_| ServerCertVerified::assertion())
}