-
-
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
test std lib TLS implementation against many real world servers #14172
Comments
www.wikipedia.org uses DigiCert, Telegram uses GoDaddy. Might be useful to try to cover a wide range of root certificates? |
Updated example https://github.com/rofrol/http-client-zig-example or here: const std = @import("std");
const http = std.http;
pub fn main() !void {
var arena_instance = std.heap.ArenaAllocator.init(std.heap.page_allocator);
const arena = arena_instance.allocator();
const gpa = arena;
const args = try std.process.argsAlloc(arena);
var client: http.Client = .{
.allocator = gpa,
};
defer client.deinit();
const url = try std.Uri.parse(args[1]);
var req = try client.request(url, .{}, .{});
defer req.deinit();
var bw = std.io.bufferedWriter(std.io.getStdOut().writer());
const w = bw.writer();
var total: usize = 0;
var buf: [500000]u8 = undefined;
while (true) {
const amt = try req.readAll(&buf);
total += amt;
if (amt == 0) break;
std.debug.print("got {d} bytes (total {d})\n", .{ amt, total });
try w.writeAll(buf[0..amt]);
}
try bw.flush();
std.debug.print("{s}", .{buf[0..total]});
} slash is needed i.e. Tested on macos m1 in vm ubuntu 22.04 using multipass https://shwjason.medium.com/how-a-simple-script-helped-help-made-me-over-1000-month-a759a604e4b3 |
@rofrol, nice update. Now, I just tried this same link and got this error with the version: ./httpClient https://onet.pl/
error: TlsCertificateNotVerified
/home/kassane/.local/lib/zig/std/crypto/tls/Client.zig:493:49: 0x26a9af in init__anon_5905 (httpClient)
.certificate => return error.TlsCertificateNotVerified,
^
/home/kassane/.local/lib/zig/std/mem.zig:0:13: 0x277225 in request (httpClient)
/home/kassane/.local/lib/zig/std/http/Client.zig:882:23: 0x27722d in request (httpClient)
.connection = try client.connect(host, port, protocol),
^
/home/kassane/.local/lib/zig/std/process.zig:0:0: 0x2807ce in main (httpClient) My another suggestion link to test: https://test.openquantumsafe.org/ |
Some sites are not supported yet I have updated above example to use master. Works on macos m1.
zig built with instructions from https://github.com/ziglang/zig/wiki/Building-Zig-From-Source First I have run
in zig-bootstrap (took 5 hours I think). Then I have downloaded zig sources and compiled zig:
|
Might be a good idea to include common code repository sites into that list, given the Package Manager MVP can't retrieve tarballs from sourcehut, and probably others. |
I have found these two lists: And json for moz.com https://github.com/Kikobeats/top-sites/blob/master/top-sites.json |
I think something wrong with |
@star-tek-mb does that require editing the source of zig std? Can it be done by providing options to std.http.Client? |
@trgwii Only by editing zig std for now (you can edit source of your current zig installation). You can just reorder, remove other cipher suites in zig/lib/std/crypto/tls/Client.zig Line 1299 in 885d696
Leave AES_128_GCM_SHA256 and CHACHA20_POLY1305_SHA256 . Those are working ones.
|
I found a new kind of error: CertificatePublicKeyInvalid for nodejs.org, which is valid in browsers. I managed to fix it by editing my local std, but I'm putting this here as an issue rather than a PR, because I don't know the implications of bumping these memory limits, nor what the proper limits should be. Here's the repr, including the changes to stdlib I made that fixed it for me locally: const std = @import("std");
comptime {
// This program currently errors with error: CertificatePublicKeyInvalid (L466)
// Tested with:
// 0.11.0-dev.1586+2d017f379 (x86_64-windows)
// 0.11.0-dev.1580+a5b34a61a (x86_64-linux)
// To make this program work on both:
// Inside this function
_ = std.crypto.tls.Client.init;
// 1. change L538
//-var rsa_mem_buf: [512 * 32]u8 = undefined;
//+var rsa_mem_buf: [512 * 64]u8 = undefined;
// 2. change L360
//-var main_cert_pub_key_buf: [300]u8 = undefined;
//+var main_cert_pub_key_buf: [600]u8 = undefined;
}
pub fn main() !void {
var gpa = std.heap.GeneralPurposeAllocator(.{}){};
defer std.debug.assert(!gpa.deinit());
const allocator = gpa.allocator();
var client = std.http.Client{ .allocator = allocator };
defer client.deinit();
const uri = try std.Uri.parse("https://nodejs.org/en/");
var request = try client.request(uri, .{}, .{});
defer request.deinit();
} |
I used #14172 (comment) to test (0.11.0-dev.1638+7199d7c77): |
@fubark try doubling things even further, that has worked for me a couple of times. |
It seems that this is also a problem for the module system, I guess that makes sense, just want to report it for future generations. If I add a URL from sourcehut, it seems to be the same, for example: Then during
Just that one single line is printed and the execution exits. |
Fixed in #15031 (might fix other issues reported here by the way) |
There's a CERT using 4096-bit RSA here.
2 * 512 * 32 looks indeed necessary for a 4096-bit modulus.
512 bytes for the 4096 bit modulus + a couple bytes for the DER encoding. That makes sense. Bumping these values look reasonable to me. The large memory requirements for |
I am encountering My environment:
I have local changes to support 4096-bit modules so that sites such as https://nodejs.org works. And my test program (modified from #14172 (comment) to support recent changes): const std = @import("std");
pub fn main() !void {
var gpa = std.heap.GeneralPurposeAllocator(.{}){};
// defer std.debug.assert(!gpa.deinit());
const allocator = gpa.allocator();
var client = std.http.Client{ .allocator = allocator };
defer client.deinit();
const uri = try std.Uri.parse("https://httpbin.org/ip/");
var headers = std.http.Headers{ .allocator = allocator };
defer headers.deinit();
var request = try client.request(.GET, uri, headers, .{});
defer request.deinit();
} |
|
Eventually, everybody's gonna move to TLS 1.3. I'm not sure that it's worth investing time in a TLS 1.2 implementation today, especially since TLS 1.2 has a lot of footguns. |
This is not at all true. There are no plans to deprecate TLS 1.2 in the IETF or among browsers, although various key exchange methods, ciphers, and signature types have been deprecated.
This might be true. However, most of the footguns can be left out if you only implement ciphers that align with those that are included in 1.3, e.g. the ECDHE + {ECDSA,RSA} + {AES_GCM, ChaChaPoly} subset. Otherwise, it's extremely unlikely you'd be able to connect to any top website that isn't hosted on a CDN, or to any embedded device. "We haven't gotten to it and it's not that important" is a perfectly fine prioritization decision, but "eventually everyone will be on TLS 1.3" is not a good justification. If y'all are willing to take it, I'd be happy to work on 1.2 support. |
I fully agree, TLS 1.2 must be supported on the long run. zig version I bumped into the following error with TLS 1.2: On top of TLS 1.3, i bumped into same error but different root cause: Most probably because I am using the following URI with IP instead of FQDN: Cheers, |
Can you please stop spamming this thread with the same comment? It's a good comment, just leave it alone. |
no worries, I have just removed it. I dont like my hours of work being referred to as "spam". I am working on my own TLS implementation, so I was interested in this same question. in the future I would recommend unsubscribing to threads that are too noisy for your taste. good luck, I like the Zig project and hope the best for it. |
@1268 Just to offer a different interpretation, I can't see what you posted (nor how many times you posted it) but Andrew said your comment was a good comment so your work is welcome and appreciated, and one copy of your comment was enough ("just leave it alone"). |
unsubscribing from this thread. He said what he said. Andrew is an undeniably brilliant programmer, better than I will ever be. but words matter. good luck again to Andrew and the project. I hope the best for Zig, its a good language. |
I'll reproduce the comment here:
using this input: https://github.com/Kikobeats/top-sites/blob/master/top-sites.json here is some Go code: package main
import (
"crypto/tls"
"encoding/json"
"fmt"
"net/http"
"net/url"
"os"
"time"
)
func main() {
text, err := os.ReadFile("top-sites.json")
if err != nil {
panic(err)
}
var sites []struct {
Root_Domain string `json:"rootDomain"`
}
json.Unmarshal(text, &sites)
http.DefaultClient.Timeout = 9*time.Second
http.DefaultTransport = &http.Transport{
TLSClientConfig: &tls.Config{MaxVersion: tls.VersionTLS12},
}
req := new(http.Request)
req.Header = make(http.Header)
req.URL = new(url.URL)
req.URL.Scheme = "https"
req.ProtoMajor = 1
req.ProtoMinor = 1
req.Header["User-Agent"] = []string{"curl/7.84.0"}
for i, site := range sites {
req.URL.Host = site.Root_Domain
res, err := http.DefaultClient.Do(req)
if err != nil {
fmt.Print("FAIL ", err, " ")
} else {
if err := res.Body.Close(); err != nil {
panic(err)
}
if res.StatusCode != http.StatusOK {
fmt.Print("FAIL ", res.Status, " ")
}
}
fmt.Println(site.Root_Domain, len(sites)-i)
time.Sleep(99 * time.Millisecond)
}
} from my judgment, every single server that accepted a request worked with TLS 1.2 or below. however if you switch to TLS 1.3:
you get many failures, including at least the ones below. so to me, TLS 1.2 should have first class support, and TLS 1.3 should be the afterthought. not the reverse.
|
FYI it's not possible to connect to huggingface.com from windows. macOS works fine. will update here with more info when I figure out what's the problem... (Fails with
Ok, I forgot that browsers come with their own list of certificates, so this is easily fixed in user-space. |
I got into this again, this time because of TLS v1.2. Is anyone working on this? Maybe I could give it a try? |
In the meantime you can use zig-curl jiacai2050/zig-curl#4 |
Until these websites support TLS 1.3, you may try zig-tls12, which only fails on a few corner cases and some certificate errors. |
I am also having issues with TLS Robert@Roberts-MacBook-Pro-2 ~/z/zigscheme (main)> zig fetch https://ccrma.stanford.edu/software/s7/s7.tar.gz
error: unable to connect to server: TlsInitializationFailed |
that's a known issue. that server is TLS 1.2 only:
for some reason Zig decided to implement TLS 1.3 only, or possible start with TLS 1.3 and add 1.2 later. so if they refuse 1.2, you're just gonna have to build the Zig tool yourself using a third party TLS package, or if they decide to add 1.2, you'll just need to wait until that happens. |
Just ran into this issue with invisible-mirror.net with zig fetch and got |
which uses TLS 1.2... |
Could zig optionally use kTLS on linux? |
"Currently only the symmetric encryption is handled in the kernel. After the TLS handshake is complete, we have all the parameters required to move the data-path to the kernel". However, most of the complexity of TLS lies in handshaking (many signature and key exchange algorithms are required, plus you need to securely manage the whole process), which is probably why both zig and linux are reluctant to expand its implementation. |
Makes sense, the issues here indeed are about the handshaking part |
https://github.com/chromium/badssl.com might be useful for testing, since it's not supposed to change its TLS config there are some unintended expired certificates on some of the subdomains, but it feels a lot more stable to check against domains like these: might even be able to spin up a local server for testing |
I made an alternate tls 1.3 and tls 1.2 Zig protocol library. While testing with the top 500 domains and then comparing with std lib I found a few problems in the std lib implementation. All domains below are tls 1.3 capable. 1. handshake message can be fragmented into multiple tls recordsError: error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls.zig:551:32: 0x123e4d7 in sub (http_get_std)
if (end > d.their_end) return error.TlsDecodeError;
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls/Client.zig:485:31: 0x12114c8 in init__anon_9815 (http_get_std)
var hsd = try ctd.sub(handshake_len); Current implementation requires one handshake message per wrapped (encrypted) tls record.
For example whatsapp.com after initial sever hello sends 3 wrapped record:
Those 3 tls wrapped records contain following handshake messages:
Certificate spans all 3 wrapped tls records. And the other two are also in last wrapped record. Correct implementation should decrypt wrapped record to cleartext buffer, if the handshake message is larger than cleartext buffer it should decrypt another record append it to the cleartext and check cleartext length again until it has full handshake message. Reference: https://datatracker.ietf.org/doc/html/rfc8446#appendix-C.3 It is also allowed that single tls record contains multiple handshake messages. 2. certificate longer then handshake buffertls.Client is in init using handshake_buffer of 8000 bytes. When parsing messages with long certificates it fails: error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls.zig:470:37: 0x123df6f in readAtLeast__anon_10807 (http_get_std)
if (request_amt > dest.len) return error.TlsRecordOverflow;
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls/Client.zig:436:9: 0x120f331 in init__anon_9815 (http_get_std)
try d.readAtLeast(stream, record_len);
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/http/Client.zig:1357:99: 0x115477b in connectTcp (http_get_std)
conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed;
^ For example googleblog.com sends 10932 bytes of handshake certificate message. Domains:
3. certificate chain not continuousDomain:
Error: error: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/Certificate.zig:258:13: 0x125c388 in verify (http_get_std)
return error.CertificateIssuerMismatch;
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls/Client.zig:547:37: 0x1212b6f in init__anon_9815 (http_get_std)
try prev_cert.verify(subject, now_sec); If we look at certificates sent by terra.com.br:
Certificate 0 and 2 are valid chain, 1 is not part of the chain and should be ignored. One possible fix is to change line 547 to: prev_cert.verify(subject, now_sec) catch |err| switch (err) {
error.CertificateIssuerMismatch => {
try certs_decoder.ensure(2);
certs_decoder.skip(2);
continue;
},
else => return err,
}; 4. rsa_pkcs1_sha384 required in client hello signature algorithms extensionerror: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls.zig:201:9: 0x125024e in toError (http_get_std)
return switch (alert) {
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls/Client.zig:253:17: 0x120b7d2 in init__anon_9815 (http_get_std)
try desc.toError(); alert is: TlsAlertHandshakeFailure If I add .rsa_pkcs1_sha384 to the list of supported signature algorithms, without implementing that in other places, handshake succeeds. zig/lib/std/crypto/tls/Client.zig Line 167 in f7d72ce
Domain:
5. secp384r1 named group required in client hello supported groups extensionerror: TlsInitializationFailed
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls.zig:472:39: 0x123dfeb in readAtLeast__anon_10807 (http_get_std)
if (actual_amt < request_amt) return error.TlsConnectionTruncated;
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls.zig:480:9: 0x123e2d4 in readAtLeastOurAmt__anon_10806 (http_get_std)
try readAtLeast(d, stream, our_amt);
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/crypto/tls/Client.zig:238:9: 0x120b3f5 in init__anon_9815 (http_get_std)
try d.readAtLeastOurAmt(stream, tls.record_header_len);
^
/usr/local/zig/zig-linux-x86_64-0.14.0-dev.66+1fdf13a14/lib/std/http/Client.zig:1357:99: 0x115476b in connectTcp (http_get_std)
conn.data.tls_client.* = std.crypto.tls.Client.init(stream, client.ca_bundle, host) catch return error.TlsInitializationFailed; zig/lib/std/crypto/tls/Client.zig Line 174 in f7d72ce
Domain:
|
I consider the criteria for this to be met by #21872 (comment) Any issues with specific sites can be now tracked separately. |
Extracted from #13980.
Here is a small test program:
This issue tracks real world web servers that cannot be accessed. Here's one for starters:
This issue can be closed when, idk, the "top 100 most popular websites" or some list like that can all be retrieved successfully.
May require working on the http client as well to support chunked encoding and things like this.
The text was updated successfully, but these errors were encountered: