diff --git a/cmd/ooniprobe/internal/nettests/echcheck.go b/cmd/ooniprobe/internal/nettests/echcheck.go index 15a45de713..42bacedc6e 100644 --- a/cmd/ooniprobe/internal/nettests/echcheck.go +++ b/cmd/ooniprobe/internal/nettests/echcheck.go @@ -13,5 +13,9 @@ func (n ECHCheck) Run(ctl *Controller) error { } // providing an input containing an empty string causes the experiment // to recognize the empty string and use the default URL - return ctl.Run(builder, []model.ExperimentTarget{model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("")}) + return ctl.Run(builder, []model.ExperimentTarget{ + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://cloudflare-ech.com/cdn-cgi/trace"), + // Use ECH on a non-standard port. + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://min-ng.test.defo.ie:15443"), + }) } diff --git a/go.mod b/go.mod index 5fd069da38..208da29517 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,6 @@ module github.com/ooni/probe-cli/v3 -go 1.21.0 - -toolchain go1.22.3 +go 1.23.4 require ( filippo.io/age v1.2.1 diff --git a/internal/experiment/echcheck/generate.go b/internal/experiment/echcheck/generate.go index acf70d474b..a4cce2af60 100644 --- a/internal/experiment/echcheck/generate.go +++ b/internal/experiment/echcheck/generate.go @@ -4,91 +4,74 @@ package echcheck // ietf.org/archive/id/draft-ietf-tls-esni-14.html import ( - "fmt" + "io" + "github.com/cloudflare/circl/hpke" "golang.org/x/crypto/cryptobyte" - "io" ) -const clientHelloOuter uint8 = 0 - -// echExtension is the Encrypted Client Hello extension that is part of -// ClientHelloOuter as specified in: -// ietf.org/archive/id/draft-ietf-tls-esni-14.html#section-5 -type echExtension struct { - kdfID uint16 - aeadID uint16 - configID uint8 - enc []byte - payload []byte -} - -func (ech *echExtension) marshal() []byte { - var b cryptobyte.Builder - b.AddUint8(clientHelloOuter) - b.AddUint16(ech.kdfID) - b.AddUint16(ech.aeadID) - b.AddUint8(ech.configID) - b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { - b.AddBytes(ech.enc) - }) - b.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { - b.AddBytes(ech.payload) - }) - return b.BytesOrPanic() -} - -// generateGreaseExtension generates an ECH extension with random values as -// specified in ietf.org/archive/id/draft-ietf-tls-esni-14.html#section-6.2 -func generateGreaseExtension(rand io.Reader) ([]byte, error) { - // initialize HPKE suite parameters - kem := hpke.KEM(uint16(hpke.KEM_X25519_HKDF_SHA256)) - kdf := hpke.KDF(uint16(hpke.KDF_HKDF_SHA256)) - aead := hpke.AEAD(uint16(hpke.AEAD_AES128GCM)) - - if !kem.IsValid() || !kdf.IsValid() || !aead.IsValid() { - return nil, fmt.Errorf("required parameters not supported") +// ECH Config List per: +// https://www.ietf.org/archive/id/draft-ietf-tls-esni-22.html#name-encrypted-clienthello-confi +func generateGreaseyECHConfigList(rand io.Reader, publicName string) ([]byte, error) { + // Start ECHConfig + var c cryptobyte.Builder + version := uint16(0xfe0d) + c.AddUint16(version) + + // Start ECHConfigContents + var ecc cryptobyte.Builder + // Start HpkeKeyConfig + randConfigId := make([]byte, 1) + if _, err := io.ReadFull(rand, randConfigId); err != nil { + return nil, err } - - defaultHPKESuite := hpke.NewSuite(kem, kdf, aead) - - // generate a public key to place in 'enc' field + ecc.AddUint8(randConfigId[0]) + ecc.AddUint16(uint16(hpke.KEM_X25519_HKDF_SHA256)) + // Generate a public key + kem := hpke.KEM(uint16(hpke.KEM_X25519_HKDF_SHA256)) publicKey, _, err := kem.Scheme().GenerateKeyPair() - if err != nil { - return nil, fmt.Errorf("failed to generate key pair: %s", err) - } - - // initiate HPKE Sender - sender, err := defaultHPKESuite.NewSender(publicKey, nil) - if err != nil { - return nil, fmt.Errorf("failed to create sender: %s", err) - } - - // Set ECH Extension Fields - var ech echExtension - - ech.kdfID = uint16(kdf) - ech.aeadID = uint16(aead) - - randomByte := make([]byte, 1) - _, err = io.ReadFull(rand, randomByte) if err != nil { return nil, err } - ech.configID = randomByte[0] - - ech.enc, _, err = sender.Setup(rand) + publicKeyBytes, err := publicKey.MarshalBinary() if err != nil { return nil, err } + ecc.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(publicKeyBytes) + }) + // Start HpkeSymmetricCipherSuite + kdf := hpke.KDF(uint16(hpke.KDF_HKDF_SHA256)) + aead := hpke.AEAD(uint16(hpke.AEAD_AES128GCM)) + var cs cryptobyte.Builder + cs.AddUint16(uint16(kdf)) + cs.AddUint16(uint16(aead)) + ecc.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(cs.BytesOrPanic()) + }) + // End HpkeSymmetricCipherSuite + // End HpkeKeyConfig + maxNameLength := uint8(42) + ecc.AddUint8(maxNameLength) + publicNameBytes := []byte(publicName) + ecc.AddUint8LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(publicNameBytes) + }) + // Start ECHConfigExtension + var ece cryptobyte.Builder + ecc.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(ece.BytesOrPanic()) + }) + // End ECHConfigExtension + // End ECHConfigContents + c.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(ecc.BytesOrPanic()) + }) + // End ECHConfig + var l cryptobyte.Builder + l.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes(c.BytesOrPanic()) + }) - // TODO: compute this correctly as per https://www.ietf.org/archive/id/draft-ietf-tls-esni-14.html#name-recommended-padding-scheme - randomEncodedClientHelloInnerLen := 100 - cipherLen := int(aead.CipherLen(uint(randomEncodedClientHelloInnerLen))) - ech.payload = make([]byte, randomEncodedClientHelloInnerLen+cipherLen) - if _, err = io.ReadFull(rand, ech.payload); err != nil { - return nil, err - } - - return ech.marshal(), nil + return l.BytesOrPanic(), nil } diff --git a/internal/experiment/echcheck/generate_test.go b/internal/experiment/echcheck/generate_test.go new file mode 100644 index 0000000000..75f918b2b4 --- /dev/null +++ b/internal/experiment/echcheck/generate_test.go @@ -0,0 +1,17 @@ +package echcheck + +import ( + "crypto/rand" + "testing" +) + +func TestParseableGREASEConfigList(t *testing.T) { + // A GREASE extension that can't be parsed is invalid. + grease, err := generateGreaseyECHConfigList(rand.Reader, "example.com") + if err != nil { + t.Fatal(err) + } + if _, err := parseECHConfigList(grease); err != nil { + t.Fatal(err) + } +} diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index 3ecc648262..7e1e177670 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -2,125 +2,126 @@ package echcheck import ( "context" - "crypto/rand" "crypto/tls" - "net" + "crypto/x509" + "encoding/base64" + "fmt" + "net/url" "time" - "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - utls "gitlab.com/yawning/utls.git" ) -const echExtensionType uint16 = 0xfe0d - -func connectAndHandshake( - ctx context.Context, - startTime time.Time, - address string, sni string, outerSni string, - logger model.Logger) (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - - channel := make(chan model.ArchivalTLSOrQUICHandshakeResult) - - ol := logx.NewOperationLogger(logger, "echcheck: TCPConnect %s", address) - var dialer net.Dialer - conn, err := dialer.DialContext(ctx, "tcp", address) - ol.Stop(err) +// We can't see which outerservername go std lib selects, so we preemptively +// make sure it's unambiguous for a given ECH Config List. If the ecl is +// empty, return an empty string. +func getUnambiguousOuterServerName(ecl []byte) (string, error) { + if len(ecl) == 0 { + return "", nil + } + configs, err := parseECHConfigList(ecl) if err != nil { - return nil, netxlite.NewErrWrapper(netxlite.ClassifyGenericError, netxlite.ConnectOperation, err) + return "", fmt.Errorf("failed to parse ECH config: %w", err) } - - go func() { - var res *model.ArchivalTLSOrQUICHandshakeResult - if outerSni == "" { - res = handshake( - ctx, - conn, - startTime, - address, - sni, - logger, - ) - } else { - res = handshakeWithEch( - ctx, - conn, - startTime, - address, - outerSni, - logger, - ) - // We need to set this explicitly because otherwise it will get - // overridden with the outerSni in the case of ECH - res.ServerName = sni + outerServerName := string(configs[0].PublicName) + for _, ec := range configs { + if string(ec.PublicName) != outerServerName { + // It's perfectly valid to have multiple ECH configs with different + // `PublicName`s. But, since we can't see which one is selected by + // go's tls package, we can't accurately record OuterServerName. + return "", fmt.Errorf("ambigious OuterServerName for config") } - channel <- *res - }() - - return channel, nil + } + return outerServerName, nil } -func handshake(ctx context.Context, conn net.Conn, zeroTime time.Time, - address string, sni string, logger model.Logger) *model.ArchivalTLSOrQUICHandshakeResult { - return handshakeWithExtension(ctx, conn, zeroTime, address, sni, []utls.TLSExtension{}, logger) -} +func startHandshake(ctx context.Context, echConfigList []byte, isGrease bool, startTime time.Time, address string, target *url.URL, logger model.Logger, testOnlyRootCAs *x509.CertPool) (chan TestKeys, error) { -func handshakeWithEch(ctx context.Context, conn net.Conn, zeroTime time.Time, - address string, sni string, logger model.Logger) *model.ArchivalTLSOrQUICHandshakeResult { - payload, err := generateGreaseExtension(rand.Reader) - if err != nil { - panic("failed to generate grease ECH: " + err.Error()) - } + channel := make(chan TestKeys) - var utlsEchExtension utls.GenericExtension + tlsConfig := genEchTLSConfig(target.Hostname(), echConfigList, testOnlyRootCAs) - utlsEchExtension.Id = echExtensionType - utlsEchExtension.Data = payload + go func() { + tk := TestKeys{} + tk = handshake(ctx, isGrease, startTime, address, logger, tlsConfig, tk) + channel <- tk + }() - hs := handshakeWithExtension(ctx, conn, zeroTime, address, sni, []utls.TLSExtension{&utlsEchExtension}, logger) - hs.ECHConfig = "GREASE" - hs.OuterServerName = sni - return hs + return channel, nil } -func handshakeMaybePrintWithECH(doprint bool) string { - if doprint { - return "WithECH" +// Add to tk TestKeys all events as they occur. May call self recursively using retry_configs. +func handshake(ctx context.Context, isGrease bool, startTime time.Time, address string, logger model.Logger, tlsConfig *tls.Config, tk TestKeys) TestKeys { + var d string + if isGrease { + d = " (GREASE)" + } else if len(tlsConfig.EncryptedClientHelloConfigList) > 0 { + d = " (RealECH)" } - return "" -} - -func handshakeWithExtension(ctx context.Context, conn net.Conn, zeroTime time.Time, address string, sni string, - extensions []utls.TLSExtension, logger model.Logger) *model.ArchivalTLSOrQUICHandshakeResult { - tlsConfig := genTLSConfig(sni) - handshakerConstructor := newHandshakerWithExtensions(extensions) - tracedHandshaker := handshakerConstructor(log.Log, &utls.HelloFirefox_Auto) + ol1 := logx.NewOperationLogger(logger, "echcheck: TCPConnect %s", address) + trace := measurexlite.NewTrace(0, startTime) + dialer := trace.NewDialerWithoutResolver(logger) + conn, err := dialer.DialContext(ctx, "tcp", address) + ol1.Stop(err) + tk.TCPConnects = append(tk.TCPConnects, trace.TCPConnects()...) + tk.NetworkEvents = append(tk.NetworkEvents, trace.NetworkEvents()...) - ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshake%s", handshakeMaybePrintWithECH(len(extensions) > 0)) + ol2 := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) start := time.Now() - maybeTLSConn, err := tracedHandshaker.Handshake(ctx, conn, tlsConfig) + maybeTLSConn := tls.Client(conn, tlsConfig) + err = maybeTLSConn.HandshakeContext(ctx) finish := time.Now() - ol.Stop(err) + ol2.Stop(err) + + retryConfigs := []byte{} + if echErr, ok := err.(*tls.ECHRejectionError); ok && isGrease { + if len(echErr.RetryConfigList) > 0 { + retryConfigs = echErr.RetryConfigList + } + // We ignore this error in crafting our TLSOrQUICHandshakeResult + // since the *golang* error is expected and merely indicates we + // didn't get the ECH setup we wanted. It does NOT indicate that + // that the handshake itself was a failure. + // TODO: Can we *confirm* there wasn't a separate TLS failure? This might be ambiguous :-( + // TODO: Confirm above semantics with OONI team. + err = nil + } connState := netxlite.MaybeTLSConnectionState(maybeTLSConn) - return measurexlite.NewArchivalTLSOrQUICHandshakeResult(0, start.Sub(zeroTime), "tcp", address, tlsConfig, - connState, err, finish.Sub(zeroTime)) + hs := measurexlite.NewArchivalTLSOrQUICHandshakeResult(0, start.Sub(startTime), + "tcp", address, tlsConfig, connState, err, finish.Sub(startTime)) + if isGrease { + hs.ECHConfig = "GREASE" + } else { + hs.ECHConfig = base64.StdEncoding.EncodeToString(tlsConfig.EncryptedClientHelloConfigList) + } + osn, err := getUnambiguousOuterServerName(tlsConfig.EncryptedClientHelloConfigList) + if err != nil { + msg := fmt.Sprintf("can't determine OuterServerName: %s", err) + hs.SoError = &msg + } + hs.OuterServerName = osn + tk.TLSHandshakes = append(tk.TLSHandshakes, hs) + + if len(retryConfigs) > 0 { + tlsConfig.EncryptedClientHelloConfigList = retryConfigs + tk = handshake(ctx, false, startTime, address, logger, tlsConfig, tk) + } + + return tk } -// We are creating the pool just once because there is a performance penalty -// when creating it every time. See https://github.com/ooni/probe/issues/2413. -var certpool = netxlite.NewMozillaCertPool() - -// genTLSConfig generates tls.Config from a given SNI -func genTLSConfig(sni string) *tls.Config { - return &tls.Config{ // #nosec G402 - we need to use a large TLS versions range for measuring - RootCAs: certpool, - ServerName: sni, - NextProtos: []string{"h2", "http/1.1"}, - InsecureSkipVerify: true, // #nosec G402 - it's fine to skip verify in a nettest +func genEchTLSConfig(host string, echConfigList []byte, testOnlyRootCAs *x509.CertPool) *tls.Config { + c := &tls.Config{ServerName: host} + if len(echConfigList) > 0 { + c.EncryptedClientHelloConfigList = echConfigList + } + if testOnlyRootCAs != nil { + c.RootCAs = testOnlyRootCAs } + return c } diff --git a/internal/experiment/echcheck/handshake_test.go b/internal/experiment/echcheck/handshake_test.go index e7dc4bf4d7..f71859f0ab 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -2,8 +2,10 @@ package echcheck import ( "context" + "crypto/rand" + "crypto/tls" + "crypto/x509" "fmt" - "net" "net/http" "net/http/httptest" "net/url" @@ -13,38 +15,146 @@ import ( "github.com/ooni/probe-cli/v3/internal/model" ) -func TestHandshake(t *testing.T) { +type testServerConfig struct { + ts *httptest.Server + url *url.URL + tlsConfig *tls.Config +} + +func setupTest(t *testing.T) testServerConfig { ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprintln(w, "success") })) - defer ts.Close() parsed, err := url.Parse(ts.URL) if err != nil { t.Fatal(err) } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - var dialer net.Dialer - conn, err := dialer.DialContext(ctx, "tcp", parsed.Host) - if err != nil { - t.Fatal(err) + testPool := x509.NewCertPool() + testPool.AddCert(ts.Certificate()) + tlsConfig := &tls.Config{ + ServerName: parsed.Hostname(), + RootCAs: testPool, } - result := handshakeWithEch(ctx, conn, time.Now(), parsed.Host, "crypto.cloudflare.com", model.DiscardLogger) - if result == nil { - t.Fatal("expected result") + return testServerConfig{ + ts: ts, + url: parsed, + tlsConfig: tlsConfig, } +} - if result.SoError != nil { - t.Fatal("did not expect error, got: ", result.SoError) +func TestHandshake(t *testing.T) { + tests := []struct { + name string + sendGrease bool + useRetryConfigs bool + want func(*testing.T, testServerConfig, TestKeys) + }{ + { + name: "no ECH", + sendGrease: false, + useRetryConfigs: false, + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + if len(result.TCPConnects) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnects)) + } + if len(result.TLSHandshakes) != 1 { + t.Fatal("expected exactly one TLS handshake, got: ", len(result.TLSHandshakes)) + } + if result.TLSHandshakes[0].SoError != nil { + t.Fatal("did not expect soft error, got: ", *result.TLSHandshakes[0].SoError) + } + if result.TLSHandshakes[0].Failure != nil { + t.Fatal("did not expect error, got: ", *result.TLSHandshakes[0].Failure) + } + if result.TLSHandshakes[0].OuterServerName != "" { + t.Fatal("expected OuterServerName to be empty, got: ", result.TLSHandshakes[0].OuterServerName) + } + }, + }, + { + name: "fail to establish ECH handshake", + // We're using a GREASE ECHConfigList, but we'll handle it as if it's a genuine one (isGrease=False) + // Test server doesn't handle ECH yet, so it wouldn't send retry configs anyways. + sendGrease: true, + useRetryConfigs: false, + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + if len(result.TCPConnects) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnects)) + } + if len(result.TLSHandshakes) != 1 { + t.Fatal("expected exactly one TLS handshake, got: ", len(result.TLSHandshakes)) + } + if result.TLSHandshakes[0].ServerName != testConfig.url.Hostname() { + t.Fatal("expected ServerName to be set to ts.URL.Hostname(), got: ", result.TLSHandshakes[0].ServerName) + } + if result.TLSHandshakes[0].SoError != nil { + t.Fatal("did not expect soft error, got: ", *result.TLSHandshakes[0].SoError) + } + if result.TLSHandshakes[0].Failure == nil { + t.Fatal("no GREASE retry so expected Failure to be set") + } + if result.TLSHandshakes[0].OuterServerName != testConfig.url.Hostname() { + t.Fatal("expected OuterServerName to be testserver name, got: ", result.TLSHandshakes[0].OuterServerName) + } + }, + }, + { + name: "GREASEy ECH handshake", + sendGrease: true, + useRetryConfigs: true, + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + // Expect 1 TCP Connect and 1 TLS connect because test server does + // not provide retry configs to initiate a second set. Upon golang + // implementing retry_configs, this should become 2 and we should + // expressly check the 2nd one conforms to expectations. + if len(result.TCPConnects) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnects)) + } + if len(result.TLSHandshakes) != 1 { + t.Fatal("expected exactly one TLS handshake, got: ", len(result.TLSHandshakes)) + } + if result.TLSHandshakes[0].ECHConfig != "GREASE" { + t.Fatal("expected first ECHConfig to be string literal 'GREASE', got: ", result.TLSHandshakes[0].ECHConfig) + } + if result.TLSHandshakes[0].SoError != nil { + t.Fatal("did not expect soft error, got: ", *result.TLSHandshakes[0].SoError) + } + if result.TLSHandshakes[0].Failure != nil { + t.Fatal("did not expect failure, got: ", *result.TLSHandshakes[0].Failure) + } + if result.TLSHandshakes[0].OuterServerName != testConfig.url.Hostname() { + t.Fatal("expected OuterServerName to be testserver name, got: ", result.TLSHandshakes[0].OuterServerName) + } + }, + }, + // TODO: Add a test case with Real ECH once the server-side of crypto/tls supports it. } - if result.Failure != nil { - t.Fatal("did not expect error, got: ", *result.Failure) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testConfig := setupTest(t) + defer testConfig.ts.Close() - conn.Close() + ecl := []byte{} + if test.sendGrease { + grease, err := generateGreaseyECHConfigList(rand.Reader, testConfig.url.Hostname()) + if err != nil { + t.Fatal(err) + } + ecl = grease + } + testConfig.tlsConfig.EncryptedClientHelloConfigList = ecl + + ctx := context.Background() + ch, err := startHandshake(ctx, ecl, test.useRetryConfigs, time.Now(), testConfig.url.Host, testConfig.url, model.DiscardLogger, testConfig.tlsConfig.RootCAs) + if err != nil { + t.Fatal(err) + } + result := <-ch + test.want(t, testConfig, result) + }) + } } diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index 243c44c88f..833f5b5f2b 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -2,8 +2,10 @@ package echcheck import ( "context" + crand "crypto/rand" "errors" - "math/rand" + "fmt" + "math/rand/v2" "net" "net/url" @@ -15,7 +17,7 @@ import ( const ( testName = "echcheck" - testVersion = "0.2.0" + testVersion = "0.3.0" defaultURL = "https://cloudflare-ech.com/cdn-cgi/trace" ) @@ -29,6 +31,9 @@ var ( // TestKeys contains echcheck test keys. type TestKeys struct { + NetworkEvents []*model.ArchivalNetworkEvent `json:"network_events"` + Queries []*model.ArchivalDNSLookupResult `json:"queries"` + TCPConnects []*model.ArchivalTCPConnectResult `json:"tcp_connects"` TLSHandshakes []*model.ArchivalTLSOrQUICHandshakeResult `json:"tls_handshakes"` } @@ -52,6 +57,7 @@ func (m *Measurer) Run( ctx context.Context, args *model.ExperimentArgs, ) error { + if args.Measurement.Input == "" { args.Measurement.Input = defaultURL } @@ -63,33 +69,60 @@ func (m *Measurer) Run( return errInvalidInputScheme } - // 1. perform a DNSLookup - ol := logx.NewOperationLogger(args.Session.Logger(), "echcheck: DNSLookup[%s] %s", m.config.resolverURL(), parsed.Host) + // DNS Lookups for Address and HTTPS RR + ol := logx.NewOperationLogger(args.Session.Logger(), "echcheck: DNSLookups[%s] %s", m.config.resolverURL(), parsed.Host) trace := measurexlite.NewTrace(0, args.Measurement.MeasurementStartTimeSaved) resolver := trace.NewParallelDNSOverHTTPSResolver(args.Session.Logger(), m.config.resolverURL()) - addrs, err := resolver.LookupHost(ctx, parsed.Host) + // We dial the alias, even when there are hints in the HTTPS record. + addrs, addrsErr := resolver.LookupHost(ctx, parsed.Hostname()) + // Port prefixing per: + // https://www.rfc-editor.org/rfc/rfc9460.html#name-query-names-for-https-rrs + var dnsQueryHost = parsed.Hostname() + if parsed.Port() != "" && parsed.Port() != "443" { + dnsQueryHost = fmt.Sprintf("_%s._https.%s", parsed.Port(), parsed.Hostname()) + } + httpsRr, httpsErr := resolver.LookupHTTPS(ctx, dnsQueryHost) ol.Stop(err) + + if addrsErr != nil { + return addrsErr + } + if httpsErr != nil { + return httpsErr + } + realEchConfig := httpsRr.Ech + grease, err := generateGreaseyECHConfigList(crand.Reader, parsed.Hostname()) if err != nil { - return err + return fmt.Errorf("failed to generate GREASE ECH config: %w", err) } - runtimex.Assert(len(addrs) > 0, "expected at least one entry in addrs") - address := net.JoinHostPort(addrs[0], "443") - handshakes := []func() (chan model.ArchivalTLSOrQUICHandshakeResult, error){ - // handshake with ECH disabled and SNI coming from the URL - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, "", args.Session.Logger()) + runtimex.Assert(len(addrs) > 0, "expected at least one entry in addrs") + port := parsed.Port() + if port == "" { + port = "443" + } + address := net.JoinHostPort(addrs[0], port) + + handshakes := []func() (chan TestKeys, error){ + // Handshake with no ECH + func() (chan TestKeys, error) { + return startHandshake(ctx, []byte{}, false, + args.Measurement.MeasurementStartTimeSaved, address, + parsed, args.Session.Logger(), nil) }, - // handshake with ECH enabled and ClientHelloOuter SNI coming from the URL - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, parsed.Host, args.Session.Logger()) + + // Handshake with ECH GREASE + func() (chan TestKeys, error) { + return startHandshake(ctx, grease, true, + args.Measurement.MeasurementStartTimeSaved, address, + parsed, args.Session.Logger(), nil) }, - // handshake with ECH enabled and hardcoded different ClientHelloOuter SNI - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, "cloudflare.com", args.Session.Logger()) + + // Handshake with real ECH + func() (chan TestKeys, error) { + return startHandshake(ctx, realEchConfig, false, + args.Measurement.MeasurementStartTimeSaved, address, + parsed, args.Session.Logger(), nil) }, } @@ -99,8 +132,7 @@ func (m *Measurer) Run( handshakes[i], handshakes[j] = handshakes[j], handshakes[i] }) - var channels [3](chan model.ArchivalTLSOrQUICHandshakeResult) - var results [3](model.ArchivalTLSOrQUICHandshakeResult) + var channels [3](chan TestKeys) // Fire the handshakes in parallel // TODO: currently if one of the connects fails we fail the whole result @@ -113,14 +145,23 @@ func (m *Measurer) Run( } } + alltks := TestKeys{ + TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, + NetworkEvents: trace.NetworkEvents(), + Queries: trace.DNSLookupsFromRoundTrip(), + TCPConnects: []*model.ArchivalTCPConnectResult{}, + } + // Wait on each channel for the results to come in - for idx, ch := range channels { - results[idx] = <-ch + for _, ch := range channels { + tk := <-ch + alltks.TLSHandshakes = append(alltks.TLSHandshakes, tk.TLSHandshakes...) + alltks.NetworkEvents = append(alltks.NetworkEvents, tk.NetworkEvents...) + alltks.Queries = append(alltks.Queries, tk.Queries...) + alltks.TCPConnects = append(alltks.TCPConnects, tk.TCPConnects...) } - args.Measurement.TestKeys = TestKeys{TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{ - &results[0], &results[1], &results[2], - }} + args.Measurement.TestKeys = alltks return nil } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index c6fbe71ff2..a37e5e67ee 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -2,6 +2,8 @@ package echcheck import ( "context" + "net/url" + "strings" "testing" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -14,7 +16,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if measurer.ExperimentName() != "echcheck" { t.Fatal("unexpected name") } - if measurer.ExperimentVersion() != "0.2.0" { + if measurer.ExperimentVersion() != "0.3.0" { t.Fatal("unexpected version") } } @@ -114,12 +116,57 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // check results tk := msrmnt.TestKeys.(TestKeys) + foundA, foundAAAA, foundHTTPS := false, false, false + parsed, err := url.Parse(defaultURL) + if err != nil { + t.Fatal("bad default url:", err) + } + for _, q := range tk.Queries { + aboutHost := q.Hostname == parsed.Hostname() + if (q.Failure == nil) && aboutHost { + switch q.QueryType { + case "A": + foundA = true + case "AAAA": + foundAAAA = true + case "HTTPS": + foundHTTPS = true + default: + // nothing + } + } + } + if !foundA { + t.Fatal("No DNS type A roundtrip reported") + } + if !foundAAAA { + t.Fatal("No DNS type AAAA roundtrip reported") + } + if !foundHTTPS { + t.Fatal("No DNS type HTTPS roundtrip reported") + } + if len(tk.NetworkEvents) == 0 { + t.Fatal("no network events recorded") + } + // NoECH, GREASE, GREASE retry, RealECH + if len(tk.TLSHandshakes) != 4 { + t.Fatal("unexpected number of TLS handshakes", len(tk.TLSHandshakes)) + } + if len(tk.TCPConnects) != 4 { + t.Fatal("unexpected number of TCP connections", len(tk.TCPConnects)) + } for _, hs := range tk.TLSHandshakes { if hs.Failure != nil { if hs.ECHConfig == "GREASE" { - t.Fatal("unexpected exp failure:", hs.Failure) + // We expect that this either succeeds (i.e. with a non-ECH server) + // OR that it fails with an EchRejeECHRejectionError + if !strings.Contains(*hs.Failure, "tls: server rejected ECH") { + t.Fatal("unexpected exp (grease) failure:", *hs.Failure) + } + } else if len(hs.ECHConfig) > 0 { + t.Fatal("unexpected exp (ech) failure:", *hs.Failure) } else { - t.Fatal("unexpected ctrl failure:", hs.Failure) + t.Fatal("unexpected ctrl failure:", *hs.Failure) } } } diff --git a/internal/experiment/echcheck/utls.go b/internal/experiment/echcheck/utls.go deleted file mode 100644 index 50eb632377..0000000000 --- a/internal/experiment/echcheck/utls.go +++ /dev/null @@ -1,49 +0,0 @@ -package echcheck - -import ( - "context" - "crypto/tls" - "net" - - "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/runtimex" - utls "gitlab.com/yawning/utls.git" -) - -type tlsHandshakerWithExtensions struct { - extensions []utls.TLSExtension - dl model.DebugLogger - id *utls.ClientHelloID -} - -var _ model.TLSHandshaker = &tlsHandshakerWithExtensions{} - -// newHandshakerWithExtensions returns a NewHandshaker function for creating -// tlsHandshakerWithExtensions instances. -func newHandshakerWithExtensions(extensions []utls.TLSExtension) func(dl model.DebugLogger, id *utls.ClientHelloID) model.TLSHandshaker { - return func(dl model.DebugLogger, id *utls.ClientHelloID) model.TLSHandshaker { - return &tlsHandshakerWithExtensions{ - extensions: extensions, - dl: dl, - id: id, - } - } -} - -func (t *tlsHandshakerWithExtensions) Handshake( - ctx context.Context, tcpConn net.Conn, tlsConfig *tls.Config) (model.TLSConn, error) { - tlsConn, err := netxlite.NewUTLSConn(tcpConn, tlsConfig, t.id) - runtimex.Assert(err == nil, "unexpected error when creating UTLSConn") - - if t.extensions != nil && len(t.extensions) != 0 { - runtimex.Try0(tlsConn.BuildHandshakeState()) - tlsConn.Extensions = append(tlsConn.Extensions, t.extensions...) - } - - if err := tlsConn.Handshake(); err != nil { - return nil, err - } - - return tlsConn, nil -} diff --git a/internal/experiment/echcheck/utls_test.go b/internal/experiment/echcheck/utls_test.go deleted file mode 100644 index 806fe2896e..0000000000 --- a/internal/experiment/echcheck/utls_test.go +++ /dev/null @@ -1,41 +0,0 @@ -package echcheck - -import ( - "context" - "crypto/tls" - "errors" - "testing" - - "github.com/ooni/probe-cli/v3/internal/mocks" - "github.com/ooni/probe-cli/v3/internal/model" - utls "gitlab.com/yawning/utls.git" -) - -func TestTLSHandshakerWithExtension(t *testing.T) { - t.Run("when the TLS handshake fails", func(t *testing.T) { - thx := &tlsHandshakerWithExtensions{ - extensions: []utls.TLSExtension{}, - dl: model.DiscardLogger, - id: &utls.HelloChrome_70, - } - - expected := errors.New("mocked error") - tcpConn := &mocks.Conn{ - MockWrite: func(b []byte) (int, error) { - return 0, expected - }, - } - - tlsConfig := &tls.Config{ - InsecureSkipVerify: true, - } - - tlsConn, err := thx.Handshake(context.Background(), tcpConn, tlsConfig) - if !errors.Is(err, expected) { - t.Fatal(err) - } - if tlsConn != nil { - t.Fatal("expected nil tls conn") - } - }) -} diff --git a/internal/experiment/echcheck/verbatim.go b/internal/experiment/echcheck/verbatim.go new file mode 100644 index 0000000000..66fe96e056 --- /dev/null +++ b/internal/experiment/echcheck/verbatim.go @@ -0,0 +1,129 @@ +// This file contains verbatim copies of the code from go's tls package at 1.23.4. +// https://github.com/golang/go/blob/go1.23.4/src/crypto/tls/common.go +// https://github.com/golang/go/blob/go1.23.4/src/crypto/tls/ech.go +// This should be refreshed to newer implementations if the ECH RFC change the format. + +// This file is: +// Copyright 2024 The Go Authors. All rights reserved. +// Use of this code is governed by the BSD-style license found at: +// https://github.com/golang/go/blob/go1.23.4/LICENSE + +package echcheck + +import ( + "errors" + + "golang.org/x/crypto/cryptobyte" +) + +const extensionEncryptedClientHello uint16 = 0xfe0d + +type echCipher struct { + KDFID uint16 + AEADID uint16 +} + +type echExtension struct { + Type uint16 + Data []byte +} + +type echConfig struct { + raw []byte + + Version uint16 + Length uint16 + + ConfigID uint8 + KemID uint16 + PublicKey []byte + SymmetricCipherSuite []echCipher + + MaxNameLength uint8 + PublicName []byte + Extensions []echExtension +} + +var errMalformedECHConfig = errors.New("tls: malformed ECHConfigList") + +// parseECHConfigList parses a draft-ietf-tls-esni-18 ECHConfigList, returning a +// slice of parsed ECHConfigs, in the same order they were parsed, or an error +// if the list is malformed. +func parseECHConfigList(data []byte) ([]echConfig, error) { + s := cryptobyte.String(data) + // Skip the length prefix + var length uint16 + if !s.ReadUint16(&length) { + return nil, errMalformedECHConfig + } + if length != uint16(len(data)-2) { + return nil, errMalformedECHConfig + } + var configs []echConfig + for len(s) > 0 { + var ec echConfig + ec.raw = []byte(s) + if !s.ReadUint16(&ec.Version) { + return nil, errMalformedECHConfig + } + if !s.ReadUint16(&ec.Length) { + return nil, errMalformedECHConfig + } + if len(ec.raw) < int(ec.Length)+4 { + return nil, errMalformedECHConfig + } + ec.raw = ec.raw[:ec.Length+4] + if ec.Version != extensionEncryptedClientHello { + s.Skip(int(ec.Length)) + continue + } + if !s.ReadUint8(&ec.ConfigID) { + return nil, errMalformedECHConfig + } + if !s.ReadUint16(&ec.KemID) { + return nil, errMalformedECHConfig + } + if !s.ReadUint16LengthPrefixed((*cryptobyte.String)(&ec.PublicKey)) { + return nil, errMalformedECHConfig + } + var cipherSuites cryptobyte.String + if !s.ReadUint16LengthPrefixed(&cipherSuites) { + return nil, errMalformedECHConfig + } + for !cipherSuites.Empty() { + var c echCipher + if !cipherSuites.ReadUint16(&c.KDFID) { + return nil, errMalformedECHConfig + } + if !cipherSuites.ReadUint16(&c.AEADID) { + return nil, errMalformedECHConfig + } + ec.SymmetricCipherSuite = append(ec.SymmetricCipherSuite, c) + } + if !s.ReadUint8(&ec.MaxNameLength) { + return nil, errMalformedECHConfig + } + var publicName cryptobyte.String + if !s.ReadUint8LengthPrefixed(&publicName) { + return nil, errMalformedECHConfig + } + ec.PublicName = publicName + var extensions cryptobyte.String + if !s.ReadUint16LengthPrefixed(&extensions) { + return nil, errMalformedECHConfig + } + for !extensions.Empty() { + var e echExtension + if !extensions.ReadUint16(&e.Type) { + return nil, errMalformedECHConfig + } + if !extensions.ReadUint16LengthPrefixed((*cryptobyte.String)(&e.Data)) { + return nil, errMalformedECHConfig + } + ec.Extensions = append(ec.Extensions, e) + } + + configs = append(configs, ec) + } + return configs, nil +} diff --git a/internal/model/netx.go b/internal/model/netx.go index f6f4bc78ff..e4f3db4062 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -184,6 +184,10 @@ type HTTPSSvc struct { // IPv6 contains the IPv6 hints (which may be empty). IPv6 []string + + // Encrypted ClientHello config decoded from base64 to bytes + // (which may be empty). + Ech []byte } // MeasuringNetwork defines the constructors required for implementing OONI experiments. All diff --git a/internal/netxlite/dnsdecoder.go b/internal/netxlite/dnsdecoder.go index d00ef1ce26..3e5d9ed9a4 100644 --- a/internal/netxlite/dnsdecoder.go +++ b/internal/netxlite/dnsdecoder.go @@ -101,6 +101,7 @@ func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) { ALPN: []string{}, // ensure it's not nil IPv4: []string{}, // ensure it's not nil IPv6: []string{}, // ensure it's not nil + Ech: []byte{}, // ensure it's not nil } for _, answer := range r.msg.Answer { switch avalue := answer.(type) { @@ -117,11 +118,13 @@ func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) { for _, ip := range extv.Hint { out.IPv6 = append(out.IPv6, ip.String()) } + case *dns.SVCBECHConfig: + out.Ech = extv.ECH } } } } - if len(out.IPv4) <= 0 && len(out.IPv6) <= 0 { + if len(out.IPv4) <= 0 && len(out.IPv6) <= 0 && len(out.Ech) <= 0 { return nil, dnsDecoderWrapError(ErrOODNSNoAnswer) } return out, nil diff --git a/internal/netxlite/resolvercore.go b/internal/netxlite/resolvercore.go index 6a46beaa07..ece1e5856d 100644 --- a/internal/netxlite/resolvercore.go +++ b/internal/netxlite/resolvercore.go @@ -10,6 +10,7 @@ import ( "fmt" "net" "net/http" + "strconv" "strings" "time" @@ -234,13 +235,34 @@ func (r *resolverIDNA) LookupHost(ctx context.Context, hostname string) ([]strin return r.Resolver.LookupHost(ctx, host) } +// HTTPS queries may be for either an IDNA-encodable domain or a +// Port Prefix Named domain where the portion following the port +// and service components is an IDNA-encodable domain. func (r *resolverIDNA) LookupHTTPS( ctx context.Context, domain string) (*model.HTTPSSvc, error) { - host, err := idnax.ToASCII(domain) + + idnable := domain + portPrefix := "" + if strings.HasPrefix(domain, "_") { + components := strings.Split(domain, ".") + port, err := strconv.Atoi(strings.TrimPrefix(components[0], "_")) + if err != nil { + return nil, fmt.Errorf("invalid domain: %s", domain) + } + if len(components) >= 2 && components[1] == "_https" && port >= 0 && port <= 65535 { + idnable = strings.Join(components[2:], ".") + portPrefix = components[0] + "." + components[1] + "." + } else { + return nil, fmt.Errorf("invalid domain: %s", domain) + } + } + + ldh, err := idnax.ToASCII(idnable) if err != nil { return nil, err } - return r.Resolver.LookupHTTPS(ctx, host) + query := portPrefix + ldh + return r.Resolver.LookupHTTPS(ctx, query) } func (r *resolverIDNA) Network() string { diff --git a/internal/netxlite/resolvercore_test.go b/internal/netxlite/resolvercore_test.go index 43df93aa58..43892901f5 100644 --- a/internal/netxlite/resolvercore_test.go +++ b/internal/netxlite/resolvercore_test.go @@ -653,6 +653,74 @@ func TestResolverIDNA(t *testing.T) { t.Fatal("expected no response here") } }) + + t.Run("with underscore prefixed HTTPS SVCB Record", func(t *testing.T) { + expected := &model.HTTPSSvc{ + ALPN: []string{"h3"}, + IPv4: []string{"1.1.1.1"}, + IPv6: []string{}, + } + r := &resolverIDNA{ + Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + if domain != "_12345._https.example.org" { + return nil, errors.New("passed invalid domain") + } + return expected, nil + }, + }, + } + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "_12345._https.example.org") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expected, https); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("with underscore prefixed non-LDH HTTPS SVCB Record", func(t *testing.T) { + expected := &model.HTTPSSvc{ + ALPN: []string{"h3"}, + IPv4: []string{"1.1.1.1"}, + IPv6: []string{}, + } + r := &resolverIDNA{ + Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + if domain != "_12345._https.xn--d1acpjx3f.xn--p1ai" { + return nil, errors.New("passed invalid domain") + } + return expected, nil + }, + }, + } + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "_12345._https.яндекс.рф") + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(expected, https); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("with invalid underscore-prefixed domain", func(t *testing.T) { + r := &resolverIDNA{Resolver: &mocks.Resolver{ + MockLookupHTTPS: func(ctx context.Context, domain string) (*model.HTTPSSvc, error) { + return nil, errors.New("should not happen") + }, + }} + ctx := context.Background() + https, err := r.LookupHTTPS(ctx, "_asdf._https.foo.bar") + if err == nil || !strings.HasPrefix(err.Error(), "invalid domain:") { + t.Fatal("not the error we expected") + } + if https != nil { + t.Fatal("expected no response here") + } + }) }) t.Run("Network", func(t *testing.T) { diff --git a/internal/netxlite/resolverparallel.go b/internal/netxlite/resolverparallel.go index 129fe15e6d..787860f2db 100644 --- a/internal/netxlite/resolverparallel.go +++ b/internal/netxlite/resolverparallel.go @@ -79,11 +79,16 @@ func (r *ParallelResolver) LookupHost(ctx context.Context, hostname string) ([]s func (r *ParallelResolver) LookupHTTPS( ctx context.Context, hostname string) (*model.HTTPSSvc, error) { encoder := &DNSEncoderMiekg{} + trace := ContextTraceOrDefault(ctx) query := encoder.Encode(hostname, dns.TypeHTTPS, r.Txp.RequiresPadding()) + started := trace.TimeNow() response, err := r.Txp.RoundTrip(ctx, query) + finished := trace.TimeNow() if err != nil { + trace.OnDNSRoundTripForLookupHost(started, r, query, response, []string{}, err, finished) return nil, err } + trace.OnDNSRoundTripForLookupHost(started, r, query, response, []string{}, err, finished) return response.DecodeHTTPS() }