From 3cef1bf053c14ca84206522988fed1408aa19e49 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sun, 2 May 2021 00:39:57 +0200 Subject: [PATCH] Fix name constraints check There were two bugs. webpki didn't: 1. read the X.509 Name Constraints field in its entirety, nor 2. check the certificate subject against the constraints correctly (1) is a simple fix, (2) requires reading the Common Name from the certificate. Closes #20, #134. --- src/name/verify.rs | 24 ++++++++++++++++++++---- tests/integration.rs | 19 +++++++++++++++++++ tests/wpt/ca.der | Bin 0 -> 16523 bytes tests/wpt/ee.der | Bin 0 -> 8384 bytes 4 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 tests/wpt/ca.der create mode 100644 tests/wpt/ee.der diff --git a/src/name/verify.rs b/src/name/verify.rs index 6082c19e..5b895e65 100644 --- a/src/name/verify.rs +++ b/src/name/verify.rs @@ -70,10 +70,7 @@ pub fn check_name_constraints( if !inner.peek(subtrees_tag.into()) { return Ok(None); } - let subtrees = der::nested(inner, subtrees_tag, Error::BadDer, |tagged| { - der::expect_tag_and_get_value(tagged, der::Tag::Sequence) - })?; - Ok(Some(subtrees)) + der::expect_tag_and_get_value(inner, subtrees_tag).map(Some) } let permitted_subtrees = parse_subtrees(input, der::Tag::ContextSpecificConstructed0)?; @@ -167,6 +164,10 @@ fn check_presented_id_conforms_to_constraints_in_subtree( dns_name::presented_id_matches_constraint(name, base).ok_or(Error::BadDer) } + (GeneralName::DirectoryName(name), GeneralName::DnsName(base)) => { + common_name(name).map(|cn| cn == base) + } + (GeneralName::DirectoryName(name), GeneralName::DirectoryName(base)) => Ok( presented_directory_name_matches_constraint(name, base, subtrees), ), @@ -326,3 +327,18 @@ fn general_name<'a>(input: &mut untrusted::Reader<'a>) -> Result }; Ok(name) } + +static COMMON_NAME: untrusted::Input = untrusted::Input::from(&[85, 4, 3]); + +fn common_name(input: untrusted::Input) -> Result { + let inner = &mut untrusted::Reader::new(input); + der::nested(inner, der::Tag::Set, Error::BadDer, |tagged| { + der::nested(tagged, der::Tag::Sequence, Error::BadDer, |tagged| { + let value = der::expect_tag_and_get_value(tagged, der::Tag::OID)?; + if value != COMMON_NAME { + return Err(Error::BadDer); + } + der::expect_tag_and_get_value(tagged, der::Tag::UTF8String) + }) + }) +} diff --git a/tests/integration.rs b/tests/integration.rs index 598641d3..f47402a1 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -53,6 +53,25 @@ pub fn netflix() { ); } +#[cfg(feature = "alloc")] +#[test] +pub fn wpt() { + let ee: &[u8] = include_bytes!("wpt/ee.der"); + let ca = include_bytes!("wpt/ca.der"); + + let anchors = vec![webpki::TrustAnchor::try_from_cert_der(ca).unwrap()]; + let anchors = webpki::TlsServerTrustAnchors(&anchors); + + #[allow(clippy::unreadable_literal)] // TODO: Make this clear. + let time = webpki::Time::from_seconds_since_unix_epoch(1619256684); + + let cert = webpki::EndEntityCert::try_from(ee).unwrap(); + assert_eq!( + Ok(()), + cert.verify_is_valid_tls_server_cert(ALL_SIGALGS, &anchors, &[], time) + ); +} + #[test] pub fn ed25519() { let ee: &[u8] = include_bytes!("ed25519/ee.der"); diff --git a/tests/wpt/ca.der b/tests/wpt/ca.der new file mode 100644 index 0000000000000000000000000000000000000000..f7d00160116c90b9ed17c2d9838a311570dad4d6 GIT binary patch literal 16523 zcmb_jdvKK170+fj;T1p#5FrUMgzzY?eEaR2%EN-dC>BL(TO3Pc32kX$LxPF6JoMVZ z@)V(T7)2y%6(~xJLv4$-rMA2V84BY}?HC(iB-0Gq>Oip-wCCInG|Qgf*$wv3y}$Fi z=bn4NbMG&4%!J1rGd{7^o9oHV^>%F?=u3uE|3@+;F2Aoj9b=+Vu6q`jq-3~u9sTh-T(srU`Sq?2I=a`R7YpR&+bOtY77{H?P~@{ODtEyz}cH zz1(`~;)8D|j;!0>^ow5SAHBcdvnL){F`#Mvlpl7lYH7=@T)C(2!o1sFFDm`ptF66v zPb={jPs@3re(m&QMRT`*uyn!5Ltj49R96t#Ryy;Qo|F2nY{@GxzBIk&q-O`Zt}}(<3f}~0+{2&<{fZAkKNk*G99iv+Na zBm1F{rQ9bY06LUoX*$UW;Cy1xS{xz~Dbd!C3w7c!f(S&Scs^2{C9?yq1)+`$kuaQT z48|6MD;I}I5Y~mm5X9)aD75Bqj3;HqBaI~ zVsI5AI6E-5Fw9#J&eVlbM`7L~aF+z(Ov6wo408~K5d`3T!Vqy`jRtXSFxN4d>j;b> z3av$9-l8}=a6Vy}Hy2t9!FmfrKT(K8;7o%sf&k2D9L5%a)`HNQD`ltH4CMY$?kX)9 z$DnQQ#0Jl5hOj1`4H_8HKvx698W_bOPen;W0DvWC5cS>2u0?6f%7!gU4VZ#b%b=jt z4l8ItwM-vXMVH!X1*Kz9L8&cJ&^Rh5n+UhDNij&X+C|if%}anJ(puF154@N{3RZn>0wV@LApLDx^&T2P`aEdXjrvO zPb`|Qx(f8vqUh3tf`U>Dub}iyp`i2#sGuP=W>=jrJqSX}c6Zwn=ssSO!+{x_Jx4=(WgYZ|Kkk(#PG!fP=}w3|R1 zCn;fP08;Qj8)$Ko5|kepX}KvQ(0V~ek|Nef79wam9ULK%p=JmNnjv!}l_DXR$JJ6c z*=Z$KX)01|k(5+Sf|yhmBxVhynv|T9n44*yBh0W=LvRgAlBbschdB#>Vf}E&NV{8>(HDypsi4WCp(icwMwfI8kZ_Gqcgemz?FYI>BD_<&PCSCJpGX@E#_>||_ zuAR}ZtUb}(qWR9QQm2?(J8MLarjW_`N?(D~J-_Ryd{16p7q7=LrH}J)AD<~*PCtym z)&s8I^cUT*wD3mE!vZ0}!@_$m4-1d&JS@CS^RV#DZu;}iORET$7M`7XSU6e`E6?K+ zC(q*&BX8veHo^>`Acc>)^()HDOuY3A;+I%>mX|nq9+wz+9+&ucYX|t1xObM9n0Fqh zi){L8(Kt2r;<&`Q+c<$8iEn3liEZa`iEHO^iD~C?iD&0=iDkF;rOGFHdQ=nVEAi_# z9%)aI9f@1Fex?4UOw{e1XkyvtT zADnvW07~-lKLrgVeh^Mv~c#mcbany_XxnL9?M!gw;3^0v*y z`C3xg8bBV{f)zl!_=*c8kOkM94byv-j9fn1u)OJ1u(a^mFuaxq&cyF#%kE~&?PksF zTET31T{Ap|$FbW%E~hj>wC33}x}D;4JIUr6KUWFje$ro1AXhAB16!5}iB7V(Do-qx zEZfI4WcqYF%imrDdusyP3dm`_2IkhdaFIDB*bhz@&{6!L6Z9Gx+a%MQkvC0W*3ZG# zCYj!hKIPneex~;S#nYz#lRf>d|J!dK!)~x!^wrzjk{?QYs`W@f9Z}|%I@%+a(ZdtPbU~BR08Pg}9ZS^hx)9!P} z-VNoy*nQsm-=1GOY)zBrwsk)ncX0OcZ~wAz=~thqzVz_igOh$R#(elt`IeVIIk50+ zr|)an{L=%AyRBWayRz+tf0rEH_fq|wU;X%-H}*d}byc{mux-dIQzwuAv`^$7U#^*O zsdZ-I^Jg}e|M}||Yw8cRZC*Mv@7|r~Dhghx&w2Z~FV!s>^NkTJhrPJu`e`%&u({X9 zsymB*clXJCEs?Po_jvEU?w;F^>^U@Q^v-4FpUsF?z1vX#{b9!%-M%07d27|gzrHi& G$^QarS!9*~ literal 0 HcmV?d00001 diff --git a/tests/wpt/ee.der b/tests/wpt/ee.der new file mode 100644 index 0000000000000000000000000000000000000000..7160db5aa8663bb6faf5bd1326faa302844d8188 GIT binary patch literal 8384 zcmb7Jdr(wW7~f@gQ9v{lkcSHjK5-WBy}NsN*GGwy)}#jFqfAi|L{Vf{AQ!6gbH8)o z96c*IN96XcDles%m#X8{A)LRrPfWqv1rPm|N5E17c+WmNn1lp2M~ zq3og{xv!T)sZ><=|GX?BZo|_p*N$(>$)Eb(hdaIu+&JaY&U^Kjv`f8LuemfjP-rT& zS2sU#mW4FcyqO+ZT=h&iv1Dm|`F@`n`R5LdN*J^3#@yoHr^R2i9Nky687u|aQ`ic_?KFh!OZRU{M1qaGYE%Et(9q}!1SXIB4 z3l}u|qlP}y|KL}5?5O6+u7-vg^So3_h4O1hgqm|iC^#R=qqdvci=(PYe|A;}s{_T| zm8Y+ri`&zvTo*fG?6-r9?rUdokrEuO4&j174y^4If2G%r4qKuF3cKAooD}G@r?7Y__4()159H4e+3+RW#;aNUMkk!V=dQ{aB zlxfY;F~nAzg{5t^>DgifTWn^FO>D7|CAQn`EV0!lvc&>J9Bj22SZk8p#0rSyJksY_ z{NyAnyVAN$eaa)teQ90bzZvjYfLvrHllTRoZvpFyKyE_*v-(EL1K`mEeF4agATBfT z#{lYL0dhU)2LY^WV0FG!N5I1a9t-F<5%3#WTZ`7U0J(_#0sUYG^)jM71AYUlBhWX2 zc+DU#3&^Jk>dOQF%_#psE&x9Tz;8kNATBdAKXlH3x|k3T$iD&P&B9#QYBK|UGsv@u z@&NoXg8b+~TmtaZ1o9(-KG%b|j6mNA;?;w7dEmbh$OX`EdgKqtry1l^1nZgrj|t?* zgz^CVH-h{KfX4v(#|ZdMKrVu~^k7{cOoQEOv01BgQAV$Jyv#}`&qd^8@^ihTw4UQ0_VGUyRA;TD| zw&W*=?danRIW4G7h?9m#Xh#Do#Hd*zMh70mM46n?haB3`M-O7O(I7^f8DccZLyR^x z#OQ+xG5Tmgj1ESK(cuCy8uTDW!!W>}9|5dGi0V!s4Pa-r(I5>mI!qx(gABl0h7;y_ zM841<2<%+LGQ?<5ffx-X5TgMfVlG>c zpa<;eyq1ZNE*Q{`mx+%qbkL55U5L?u1~Aut2N*r9$Z#5EdV&Uc%qg>vXt0HLG_*sE zE|36wezvj(HagE}fCqMT*2&a}25o3Zmr97y#SCIb8BTgAz;-gZrUwIPN0()Y(clX) zx_m>79;P5hk4F#_Wa6VoDq!c@o2~-4dnb22bUUcFbZKDQg}2(VKDZJ{&BA^PY_nvw zQ=26V)Vkb+({e;^rcRf!aF$NYNKR$EO-4L7M_WL`U0#EbTkTN^B%HDV{flhnM+B0B zi{OO{D1^2pkV2H?-V5>+<%YA%@2(?}dOn>;yYaNd=FwvwO02?1yNuoGJef#Icu{P9 z-AQ1r`br_W>9X_C&vQ|%$iEb$+o~>l!1#YfRPzCnVy1bnn3`?WXV~rYvKGu)D43HO zB^&6*>Gs33zBw8hLb=96zgNTTVZiArJmR zZpn80!48;*+_0&6xeBmX5oFT@Y-Bt`Ju0o!vOQSkc=dJA2`VDr+%oRj>n^*tZwn&v zv>S|6-PU8r=dyj_nrz!Rxpi47_H=#Zt5PsMY5$*c`p-Q5r(CHB$}