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

crypto.Certificate: case insensitive host name check #20545

Merged
merged 1 commit into from
Jul 9, 2024

Conversation

ianic
Copy link
Contributor

@ianic ianic commented Jul 8, 2024

Comparing host name with dns name from certificate should be case insensitive.

I found a few domains (from the cloudflare list of top domains) for which tls.Client fails to connect. Error is:

error: TlsInitializationFailed
/home/ianic/Code/zig/lib/std/crypto/Certificate.zig:336:9: 0x126cddf in verifyHostName (http_get_std)
        return error.CertificateHostMismatch;
        ^
/home/ianic/Code/zig/lib/std/crypto/tls/Client.zig:534:37: 0x1220f7f in init__anon_10280 (http_get_std)
                                    try subject.verifyHostName(host);
                                    ^
/home/ianic/Code/zig/lib/std/http/Client.zig:1357:99: 0x1161f2b in connectTcp (http_get_std)
        conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed;
                                                                                                  ^
/home/ianic/Code/zig/lib/std/http/Client.zig:1492:14: 0x1127201 in connect (http_get_std)
    } orelse return client.connectTcp(host, port, protocol);
             ^
/home/ianic/Code/zig/lib/std/http/Client.zig:1640:9: 0x1119dbe in open (http_get_std)
        try client.connect(valid_uri.host.?.raw, uriPort(valid_uri, protocol), protocol);

In its certificate this domains have dns names which are not strictly lower case. This is what checkHostName (called from verifyHostName) is comparing:

host_name dns_name
ey.com EY.COM
truist.com Truist.com
wscampanhas.bradesco WSCAMPANHAS.BRADESCO
dell.com Dell.com

From RFC2818:

Matching is performed using the matching rules specified by [RFC2459].

From RFC2459:

When comparing URIs, conforming implementations
MUST compare the scheme and host without regard to case, but assume
the remainder of the scheme-specific-part is case sensitive.

Domain certificate can be inspected with something like:

$ openssl s_client -connect dell.com:443 2>&1  | openssl x509 -noout -text | grep "DNS:"
                **DNS:Dell.com**

Testing with:

const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    const args = try std.process.argsAlloc(allocator);
    defer std.process.argsFree(allocator, args);

    if (args.len > 1) {
        const domain = args[1];

        var client: std.http.Client = .{ .allocator = allocator };
        defer client.deinit();

        // Add https:// prefix if needed
        const url = brk: {
            const scheme = "https://";
            if (domain.len >= scheme.len and std.mem.eql(u8, domain[0..scheme.len], scheme))
                break :brk domain;

            var url_buf: [128]u8 = undefined;
            break :brk try std.fmt.bufPrint(&url_buf, "https://{s}", .{domain});
        };

        const uri = try std.Uri.parse(url);
        var server_header_buffer: [16 * 1024]u8 = undefined;
        var req = try client.open(.GET, uri, .{ .server_header_buffer = &server_header_buffer });
        defer req.deinit();

        try req.send();
        try req.wait();
    }
}

$ zig run main.zig -- truist.com

This makes comparing host name with dns name from certificate case
insensitive.

I found a few domains (from the
[cloudflare](https://radar.cloudflare.com/domains) list of top domains)
for which tls.Client fails to connect. Error is:

```zig
error: TlsInitializationFailed
Code/zig/lib/std/crypto/Certificate.zig:336:9: 0x1177b1f in verifyHostName (http_get_std)
        return error.CertificateHostMismatch;
Code/zig/lib/std/crypto/tls23/handshake_client.zig:461:25: 0x11752bd in parseServerCertificate (http_get_std)
                        try subject.verifyHostName(opt.host);
```
In its certificate this domains have host names which are not strictly
lower case. This is what checkHostName is comparing:

 |host_name            |  dns_name                |
 |------------------------------------------------|
 |ey.com               | EY.COM                   |
 |truist.com           | Truist.com               |
 |wscampanhas.bradesco | WSCAMPANHAS.BRADESCO     |
 |dell.com             | Dell.com                 |

From
[RFC2818](https://datatracker.ietf.org/doc/html/rfc2818#section-2.4):
>  Matching is performed using the matching rules specified by
   [RFC2459].
From [RFC2459](https://datatracker.ietf.org/doc/html/rfc2459#section-4.2.1.7):
> When comparing URIs, conforming implementations
> MUST compare the scheme and host without regard to case, but assume
> the remainder of the scheme-specific-part is case sensitive.

Testing with:

```
const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    const args = try std.process.argsAlloc(allocator);
    defer std.process.argsFree(allocator, args);

    if (args.len > 1) {
        const domain = args[1];

        var client: std.http.Client = .{ .allocator = allocator };
        defer client.deinit();

        // Add https:// prefix if needed
        const url = brk: {
            const scheme = "https://";
            if (domain.len >= scheme.len and std.mem.eql(u8, domain[0..scheme.len], scheme))
                break :brk domain;

            var url_buf: [128]u8 = undefined;
            break :brk try std.fmt.bufPrint(&url_buf, "https://{s}", .{domain});
        };

        const uri = try std.Uri.parse(url);
        var server_header_buffer: [16 * 1024]u8 = undefined;
        var req = try client.open(.GET, uri, .{ .server_header_buffer = &server_header_buffer });
        defer req.deinit();

        try req.send();
        try req.wait();
    }
}
```
`$ zig run example/main.zig -- truist.com `
@andrewrk andrewrk merged commit c1e7eb7 into ziglang:master Jul 9, 2024
10 checks passed
@andrewrk
Copy link
Member

andrewrk commented Jul 9, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants