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

tls.Client: implement record padding #20558

Merged
merged 3 commits into from
Jul 21, 2024
Merged

Conversation

ianic
Copy link
Sponsor Contributor

@ianic ianic commented Jul 9, 2024

On decryption tls client should remove zero byte padding after the content type field. This padding is rarely used, the only site (from the list of top domains) that I found using it is tutanota.com.

From RFC:

All encrypted TLS records can be padded.
Padding is a string of zero-valued bytes appended to the ContentType
field before encryption.
the receiving implementation scans the field from the end toward the
beginning until it finds a non-zero octet. This non-zero octet is the content type of the message.

Currently we can't connect to that site:

$ zig run main.zig -- tutanota.com
error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/crypto/tls/Client.zig:476:45: 0x121fbed in init__anon_10331 (http_get_std)
                if (inner_ct != .handshake) return error.TlsUnexpectedMessage;
                                            ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1357:99: 0x1161f0b in connectTcp (http_get_std)
        conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed;
                                                                                                  ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1492:14: 0x11271e1 in connect (http_get_std)
    } orelse return client.connectTcp(host, port, protocol);
             ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1640:9: 0x111a24e in open (http_get_std)
        try client.connect(valid_uri.host.?.raw, uriPort(valid_uri, protocol), protocol);
        ^
/home/ianic/Code/tls.zig/example/http_get_std.zig:28:19: 0x1118f8c in main (http_get_std)
        var req = try client.open(.GET, uri, .{ .server_header_buffer = &server_header_buffer });
                  ^

using this example:

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();
    }
}

On decryption tls client should remove zero byte padding after the
content type field. This padding is rarely used, the only site (from the
list of top domains) that I found using it is `tutanota.com`.

From [RFC](https://datatracker.ietf.org/doc/html/rfc8446#section-5.4):
> All encrypted TLS records can be padded.
> Padding is a string of zero-valued bytes appended to the ContentType
field before encryption.
> the receiving implementation scans the field from the end toward the
beginning until it finds a non-zero octet. This non-zero octet is the
content type of the message.

Currently we can't connect to that site:
```
$ zig run main.zig -- tutanota.com
error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/crypto/tls/Client.zig:476:45: 0x121fbed in init__anon_10331 (http_get_std)
                if (inner_ct != .handshake) return error.TlsUnexpectedMessage;
                                            ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1357:99: 0x1161f0b in connectTcp (http_get_std)
        conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed;
                                                                                                  ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1492:14: 0x11271e1 in connect (http_get_std)
    } orelse return client.connectTcp(host, port, protocol);
             ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1640:9: 0x111a24e in open (http_get_std)
        try client.connect(valid_uri.host.?.raw, uriPort(valid_uri, protocol), protocol);
        ^
/home/ianic/Code/tls.zig/example/http_get_std.zig:28:19: 0x1118f8c in main (http_get_std)
        var req = try client.open(.GET, uri, .{ .server_header_buffer = &server_header_buffer });
                  ^
```
using this example:

```zig
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();
    }
}
```
For removing tls record padding.
@@ -468,7 468,7 @@ pub fn init(stream: anytype, ca_bundle: Certificate.Bundle, host: []const u8) In
read_seq = 1;
P.AEAD.decrypt(cleartext, ciphertext, auth_tag, record_header, nonce, p.server_handshake_key) catch
return error.TlsBadRecordMac;
break :c cleartext;
break :c @constCast(mem.trimRight(u8, cleartext, "\x00"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks unfortunate, is there an issue for std.mem changing constness?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed it looks bad, I had a discussion about this a while ago in the community discord but I don't believe there's an issue filed about it.

It'd be nice if these functions changed their return type based on the input's constness.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because its not ergonomic to do so and mutable slices coerce to const ones

@andrewrk andrewrk merged commit aa73bb6 into ziglang:master Jul 21, 2024
10 checks passed
@andrewrk
Copy link
Member

Great, thank you!

@andrewrk andrewrk added standard library This issue involves writing Zig code for the standard library. release notes This PR should be mentioned in the release notes. labels Jul 21, 2024
SammyJames pushed a commit to SammyJames/zig that referenced this pull request Aug 7, 2024
On decryption tls client should remove zero byte padding after the
content type field. This padding is rarely used, the only site (from the
list of top domains) that I found using it is `tutanota.com`.

From [RFC](https://datatracker.ietf.org/doc/html/rfc8446#section-5.4):
> All encrypted TLS records can be padded.
> Padding is a string of zero-valued bytes appended to the ContentType
field before encryption.
> the receiving implementation scans the field from the end toward the
beginning until it finds a non-zero octet. This non-zero octet is the
content type of the message.

Currently we can't connect to that site:
```
$ zig run main.zig -- tutanota.com
error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/crypto/tls/Client.zig:476:45: 0x121fbed in init__anon_10331 (http_get_std)
                if (inner_ct != .handshake) return error.TlsUnexpectedMessage;
                                            ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1357:99: 0x1161f0b in connectTcp (http_get_std)
        conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed;
                                                                                                  ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1492:14: 0x11271e1 in connect (http_get_std)
    } orelse return client.connectTcp(host, port, protocol);
             ^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.208 854e86c56/lib/std/http/Client.zig:1640:9: 0x111a24e in open (http_get_std)
        try client.connect(valid_uri.host.?.raw, uriPort(valid_uri, protocol), protocol);
        ^
/home/ianic/Code/tls.zig/example/http_get_std.zig:28:19: 0x1118f8c in main (http_get_std)
        var req = try client.open(.GET, uri, .{ .server_header_buffer = &server_header_buffer });
                  ^
```
using this example:

```zig
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();
    }
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes This PR should be mentioned in the release notes. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants