Skip to content

Commit

Permalink
std.crypto.tls: verify via Subject Alt Name
Browse files Browse the repository at this point in the history
Previously, the code only checked Common Name, leading to unable to
validate valid certificates which relied on the subject_alt_name
extension for host name verification.

This commit also adds rsa_pss_rsae_* back to the signature algorithms
list in the ClientHello.
  • Loading branch information
andrewrk committed Jan 2, 2023
1 parent 91a1302 commit c1c910b
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 25 deletions.
143 changes: 141 additions & 2 deletions lib/std/crypto/Certificate.zig
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,43 @@ pub const NamedCurve = enum {
});
};

pub const ExtensionId = enum {
subject_key_identifier,
key_usage,
private_key_usage_period,
subject_alt_name,
issuer_alt_name,
basic_constraints,
crl_number,
certificate_policies,
authority_key_identifier,

pub const map = std.ComptimeStringMap(ExtensionId, .{
.{ &[_]u8{ 0x55, 0x1D, 0x0E }, .subject_key_identifier },
.{ &[_]u8{ 0x55, 0x1D, 0x0F }, .key_usage },
.{ &[_]u8{ 0x55, 0x1D, 0x10 }, .private_key_usage_period },
.{ &[_]u8{ 0x55, 0x1D, 0x11 }, .subject_alt_name },
.{ &[_]u8{ 0x55, 0x1D, 0x12 }, .issuer_alt_name },
.{ &[_]u8{ 0x55, 0x1D, 0x13 }, .basic_constraints },
.{ &[_]u8{ 0x55, 0x1D, 0x14 }, .crl_number },
.{ &[_]u8{ 0x55, 0x1D, 0x20 }, .certificate_policies },
.{ &[_]u8{ 0x55, 0x1D, 0x23 }, .authority_key_identifier },
});
};

pub const GeneralNameTag = enum(u5) {
otherName = 0,
rfc822Name = 1,
dNSName = 2,
x400Address = 3,
directoryName = 4,
ediPartyName = 5,
uniformResourceIdentifier = 6,
iPAddress = 7,
registeredID = 8,
_,
};

pub const Parsed = struct {
certificate: Certificate,
issuer_slice: Slice,
Expand All @@ -91,6 +128,7 @@ pub const Parsed = struct {
pub_key_algo: PubKeyAlgo,
pub_key_slice: Slice,
message_slice: Slice,
subject_alt_name_slice: Slice,
validity: Validity,

pub const PubKeyAlgo = union(AlgorithmCategory) {
Expand Down Expand Up @@ -137,6 +175,10 @@ pub const Parsed = struct {
return p.slice(p.message_slice);
}

pub fn subjectAltName(p: Parsed) []const u8 {
return p.slice(p.subject_alt_name_slice);
}

pub const VerifyError = error{
CertificateIssuerMismatch,
CertificateNotYetValid,
Expand All @@ -152,8 +194,10 @@ pub const Parsed = struct {
CertificateSignatureNamedCurveUnsupported,
};

/// This function checks the time validity for the subject only. Checking
/// the issuer's time validity is out of scope.
/// This function verifies:
/// * That the subject's issuer is indeed the provided issuer.
/// * The time validity of the subject.
/// * The signature.
pub fn verify(parsed_subject: Parsed, parsed_issuer: Parsed) VerifyError!void {
// Check that the subject's issuer name matches the issuer's
// subject name.
Expand Down Expand Up @@ -194,6 +238,62 @@ pub const Parsed = struct {
),
}
}

pub const VerifyHostNameError = error{
CertificateHostMismatch,
CertificateFieldHasInvalidLength,
};

pub fn verifyHostName(parsed_subject: Parsed, host_name: []const u8) VerifyHostNameError!void {
// If the Subject Alternative Names extension is present, this is
// what to check. Otherwise, only the common name is checked.
const subject_alt_name = parsed_subject.subjectAltName();
if (subject_alt_name.len == 0) {
if (checkHostName(host_name, parsed_subject.commonName())) {
return;
} else {
return error.CertificateHostMismatch;
}
}

const general_names = try der.Element.parse(subject_alt_name, 0);
var name_i = general_names.slice.start;
while (name_i < general_names.slice.end) {
const general_name = try der.Element.parse(subject_alt_name, name_i);
name_i = general_name.slice.end;
switch (@intToEnum(GeneralNameTag, @enumToInt(general_name.identifier.tag))) {
.dNSName => {
const dns_name = subject_alt_name[general_name.slice.start..general_name.slice.end];
if (checkHostName(host_name, dns_name)) return;
},
else => {},
}
}

return error.CertificateHostMismatch;
}

fn checkHostName(host_name: []const u8, dns_name: []const u8) bool {
if (mem.eql(u8, dns_name, host_name)) {
return true; // exact match
}

if (mem.startsWith(u8, dns_name, "*.")) {
// wildcard certificate, matches any subdomain
// TODO: I think wildcards are not supposed to match any prefix but
// only match exactly one subdomain.
if (mem.endsWith(u8, host_name, dns_name[1..])) {
// The host_name has a subdomain, but the important part matches.
return true;
}
if (mem.eql(u8, dns_name[2..], host_name)) {
// The host_name has no subdomain and matches exactly.
return true;
}
}

return false;
}
};

pub fn parse(cert: Certificate) !Parsed {
Expand Down Expand Up @@ -268,6 +368,39 @@ pub fn parse(cert: Certificate) !Parsed {
const sig_elem = try der.Element.parse(cert_bytes, sig_algo.slice.end);
const signature = try parseBitString(cert, sig_elem);

// Extensions
var subject_alt_name_slice = der.Element.Slice.empty;
ext: {
if (pub_key_info.slice.end >= tbs_certificate.slice.end)
break :ext;

const outer_extensions = try der.Element.parse(cert_bytes, pub_key_info.slice.end);
if (outer_extensions.identifier.tag != .bitstring)
break :ext;

const extensions = try der.Element.parse(cert_bytes, outer_extensions.slice.start);

var ext_i = extensions.slice.start;
while (ext_i < extensions.slice.end) {
const extension = try der.Element.parse(cert_bytes, ext_i);
ext_i = extension.slice.end;
const oid_elem = try der.Element.parse(cert_bytes, extension.slice.start);
const ext_id = parseExtensionId(cert_bytes, oid_elem) catch |err| switch (err) {
error.CertificateHasUnrecognizedObjectId => continue,
else => |e| return e,
};
const critical_elem = try der.Element.parse(cert_bytes, oid_elem.slice.end);
const ext_bytes_elem = if (critical_elem.identifier.tag != .boolean)
critical_elem
else
try der.Element.parse(cert_bytes, critical_elem.slice.end);
switch (ext_id) {
.subject_alt_name => subject_alt_name_slice = ext_bytes_elem.slice,
else => continue,
}
}
}

return .{
.certificate = cert,
.common_name_slice = common_name,
Expand All @@ -282,6 +415,7 @@ pub fn parse(cert: Certificate) !Parsed {
.not_before = not_before_utc,
.not_after = not_after_utc,
},
.subject_alt_name_slice = subject_alt_name_slice,
};
}

Expand Down Expand Up @@ -444,6 +578,10 @@ pub fn parseNamedCurve(bytes: []const u8, element: der.Element) !NamedCurve {
return parseEnum(NamedCurve, bytes, element);
}

pub fn parseExtensionId(bytes: []const u8, element: der.Element) !ExtensionId {
return parseEnum(ExtensionId, bytes, element);
}

fn parseEnum(comptime E: type, bytes: []const u8, element: der.Element) !E {
if (element.identifier.tag != .object_identifier)
return error.CertificateFieldHasWrongDataType;
Expand Down Expand Up @@ -604,6 +742,7 @@ pub const der = struct {
boolean = 1,
integer = 2,
bitstring = 3,
octetstring = 4,
null = 5,
object_identifier = 6,
sequence = 16,
Expand Down
27 changes: 4 additions & 23 deletions lib/std/crypto/tls/Client.zig
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ pub fn init(stream: anytype, ca_bundle: Certificate.Bundle, host: []const u8) !C
.ecdsa_secp256r1_sha256,
.ecdsa_secp384r1_sha384,
.ecdsa_secp521r1_sha512,
.rsa_pss_rsae_sha256,
.rsa_pss_rsae_sha384,
.rsa_pss_rsae_sha512,
.rsa_pkcs1_sha256,
.rsa_pkcs1_sha384,
.rsa_pkcs1_sha512,
Expand Down Expand Up @@ -444,9 +447,7 @@ pub fn init(stream: anytype, ca_bundle: Certificate.Bundle, host: []const u8) !C
const subject = try subject_cert.parse();
if (cert_index == 0) {
// Verify the host on the first certificate.
if (!hostMatchesCommonName(host, subject.commonName())) {
return error.TlsCertificateHostMismatch;
}
try subject.verifyHostName(host);

// Keep track of the public key for the
// certificate_verify message later.
Expand Down Expand Up @@ -1162,26 +1163,6 @@ fn straddleByte(s1: []const u8, s2: []const u8, index: usize) u8 {
}
}

fn hostMatchesCommonName(host: []const u8, common_name: []const u8) bool {
if (mem.eql(u8, common_name, host)) {
return true; // exact match
}

if (mem.startsWith(u8, common_name, "*.")) {
// wildcard certificate, matches any subdomain
if (mem.endsWith(u8, host, common_name[1..])) {
// The host has a subdomain, but the important part matches.
return true;
}
if (mem.eql(u8, common_name[2..], host)) {
// The host has no subdomain and matches exactly.
return true;
}
}

return false;
}

const builtin = @import("builtin");
const native_endian = builtin.cpu.arch.endian();

Expand Down

0 comments on commit c1c910b

Please sign in to comment.