-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Conversation
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")); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Great, thank you! |
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(); } } ```
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(); } } ```
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:
Currently we can't connect to that site:
using this example: