From 5fabad5d75bd46d90aa5e5759fcd2d3bf86e23cd Mon Sep 17 00:00:00 2001 From: John Hess Date: Wed, 11 Dec 2024 16:53:33 -0600 Subject: [PATCH 01/10] implement real Encrypted ClientHello probe --- go.mod | 2 +- internal/experiment/echcheck/echconfig.go | 30 +++++ internal/experiment/echcheck/generate.go | 11 +- internal/experiment/echcheck/handshake.go | 86 ++++++++---- .../experiment/echcheck/handshake_test.go | 2 +- internal/experiment/echcheck/measure.go | 53 ++++++-- internal/experiment/echcheck/measure_test.go | 6 +- internal/experiment/echcheck/verbatim.go | 126 ++++++++++++++++++ internal/model/netx.go | 4 + internal/netxlite/dnsdecoder.go | 5 +- 10 files changed, 273 insertions(+), 52 deletions(-) create mode 100644 internal/experiment/echcheck/echconfig.go create mode 100644 internal/experiment/echcheck/verbatim.go diff --git a/go.mod b/go.mod index 5a797b194e..1538dba8ee 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ooni/probe-cli/v3 -go 1.21.0 +go 1.23.4 toolchain go1.22.2 diff --git a/internal/experiment/echcheck/echconfig.go b/internal/experiment/echcheck/echconfig.go new file mode 100644 index 0000000000..8b10dc0e91 --- /dev/null +++ b/internal/experiment/echcheck/echconfig.go @@ -0,0 +1,30 @@ +package echcheck + +import ( + "encoding/base64" +) + +const extensionEncryptedClientHello uint16 = 0xfe0d + +type echConfigList struct { + Configs []echConfig + raw []byte +} + +// The raw data of the ECHConfigList as it would appear in the DNS record. +func (ecl echConfigList) Base64() string { + return base64.StdEncoding.EncodeToString(ecl.raw) +} + +// Uses go's own tls package's parsing implementation and returns +// the result along with the raw data in an echConfigList. +func parseRawEchConfig(data []byte) (echConfigList, error) { + configs, err := parseECHConfigList(data) + if err != nil { + return echConfigList{}, err + } + return echConfigList{ + raw: data, + Configs: configs, + }, nil +} diff --git a/internal/experiment/echcheck/generate.go b/internal/experiment/echcheck/generate.go index acf70d474b..9d4521d8a2 100644 --- a/internal/experiment/echcheck/generate.go +++ b/internal/experiment/echcheck/generate.go @@ -5,17 +5,18 @@ package echcheck 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 +// echTLSExtension 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 { +type echTLSExtension struct { kdfID uint16 aeadID uint16 configID uint8 @@ -23,7 +24,7 @@ type echExtension struct { payload []byte } -func (ech *echExtension) marshal() []byte { +func (ech *echTLSExtension) marshal() []byte { var b cryptobyte.Builder b.AddUint8(clientHelloOuter) b.AddUint16(ech.kdfID) @@ -65,7 +66,7 @@ func generateGreaseExtension(rand io.Reader) ([]byte, error) { } // Set ECH Extension Fields - var ech echExtension + var ech echTLSExtension ech.kdfID = uint16(kdf) ech.aeadID = uint16(aead) diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index 3ecc648262..cbe5e72e71 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -17,11 +17,21 @@ import ( 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) { +type EchMode int + +const ( + NoECH EchMode = iota + GreaseECH + RealECH +) + +// Connect to `host` at `address` and attempt a TLS handshake. When using real ECH, `ecl` must +// contain the ECHConfigList to use; in other modes, `ecl` is ignored. If the ECH config provides +// a different OuterServerName than `host`, it will be recorded in the result and used per go's +// tls package's behavior. +// Returns a channel that will contain the archival result of the handshake. +func connectAndHandshake(ctx context.Context, mode EchMode, ecl echConfigList, startTime time.Time, + address string, host string, logger model.Logger) (chan model.ArchivalTLSOrQUICHandshakeResult, error) { channel := make(chan model.ArchivalTLSOrQUICHandshakeResult) @@ -35,27 +45,13 @@ func connectAndHandshake( 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 + switch mode { + case NoECH: + res = handshakeWithoutEch(ctx, conn, startTime, address, host, logger) + case GreaseECH: + res = handshakeWithGreaseyEch(ctx, conn, startTime, address, host, logger) + case RealECH: + res = handshakeWithRealEch(ctx, conn, startTime, address, host, ecl, logger) } channel <- *res }() @@ -63,12 +59,12 @@ func connectAndHandshake( return channel, nil } -func handshake(ctx context.Context, conn net.Conn, zeroTime time.Time, +func handshakeWithoutEch(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 handshakeWithEch(ctx context.Context, conn net.Conn, zeroTime time.Time, +func handshakeWithGreaseyEch(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 { @@ -88,11 +84,33 @@ func handshakeWithEch(ctx context.Context, conn net.Conn, zeroTime time.Time, func handshakeMaybePrintWithECH(doprint bool) string { if doprint { - return "WithECH" + return "WithGreaseECH" } return "" } +// ECHConfigList must be valid, non-empty, and to specify only one unique PublicName, +// i.e. OuterServerName across all configs. Host is the service to connect to, i.e. +// the inner SNI. +func handshakeWithRealEch(ctx context.Context, conn net.Conn, zeroTime time.Time, + address string, host string, ecl echConfigList, logger model.Logger) *model.ArchivalTLSOrQUICHandshakeResult { + + tlsConfig := genEchTLSConfig(host, ecl) + + ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshakeWithRealECH") + start := time.Now() + maybeTLSConn, err := tls.Dial("tcp", address, tlsConfig) + finish := time.Now() + ol.Stop(err) + + connState := netxlite.MaybeTLSConnectionState(maybeTLSConn) + hs := measurexlite.NewArchivalTLSOrQUICHandshakeResult(0, start.Sub(zeroTime), "tcp", address, tlsConfig, + connState, err, finish.Sub(zeroTime)) + hs.ECHConfig = ecl.Base64() + hs.OuterServerName = string(ecl.Configs[0].PublicName) + return hs +} + 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) @@ -124,3 +142,13 @@ func genTLSConfig(sni string) *tls.Config { InsecureSkipVerify: true, // #nosec G402 - it's fine to skip verify in a nettest } } + +func genEchTLSConfig(host string, ecl echConfigList) *tls.Config { + return &tls.Config{ + EncryptedClientHelloConfigList: ecl.raw, + // This will be used as the inner SNI and we will validate + // we get a certificate for this name. The outer SNI will + // be set based on the ECH config. + ServerName: host, + } +} diff --git a/internal/experiment/echcheck/handshake_test.go b/internal/experiment/echcheck/handshake_test.go index e7dc4bf4d7..bb9d3537a0 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -33,7 +33,7 @@ func TestHandshake(t *testing.T) { t.Fatal(err) } - result := handshakeWithEch(ctx, conn, time.Now(), parsed.Host, "crypto.cloudflare.com", model.DiscardLogger) + result := handshakeWithGreaseyEch(ctx, conn, time.Now(), parsed.Host, "crypto.cloudflare.com", model.DiscardLogger) if result == nil { t.Fatal("expected result") } diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index 243c44c88f..9958671eb5 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -3,7 +3,8 @@ package echcheck import ( "context" "errors" - "math/rand" + "fmt" + "math/rand/v2" "net" "net/url" @@ -15,7 +16,7 @@ import ( const ( testName = "echcheck" - testVersion = "0.2.0" + testVersion = "0.3.0" defaultURL = "https://cloudflare-ech.com/cdn-cgi/trace" ) @@ -52,6 +53,7 @@ func (m *Measurer) Run( ctx context.Context, args *model.ExperimentArgs, ) error { + if args.Measurement.Input == "" { args.Measurement.Input = defaultURL } @@ -67,29 +69,54 @@ func (m *Measurer) Run( ol := logx.NewOperationLogger(args.Session.Logger(), "echcheck: DNSLookup[%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.Host) + httpsRr, httpsErr := resolver.LookupHTTPS(ctx, parsed.Host) ol.Stop(err) + if addrsErr != nil { + return addrsErr + } + if httpsErr != nil { + return httpsErr + } + rawEchConfig := httpsRr.Ech + ecl, err := parseRawEchConfig(rawEchConfig) if err != nil { - return err + return fmt.Errorf("failed to parse ECH config: %w", err) } + if len(ecl.Configs) == 0 { + return fmt.Errorf("no ECH configs for %s", parsed.Host) + } + outerServerName := string(ecl.Configs[0].PublicName) + for _, ec := range ecl.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 %s", parsed.Host) + } + } + 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 + // Handshake with no ECH func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, "", args.Session.Logger()) + return connectAndHandshake(ctx, NoECH, echConfigList{}, args.Measurement.MeasurementStartTimeSaved, + address, parsed.Host, args.Session.Logger()) }, - // handshake with ECH enabled and ClientHelloOuter SNI coming from the URL + + // Handshake with ECH GREASE func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, parsed.Host, args.Session.Logger()) + return connectAndHandshake(ctx, GreaseECH, echConfigList{}, args.Measurement.MeasurementStartTimeSaved, + address, parsed.Host, args.Session.Logger()) }, - // handshake with ECH enabled and hardcoded different ClientHelloOuter SNI + + // Use real ECH func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, "cloudflare.com", args.Session.Logger()) + return connectAndHandshake(ctx, RealECH, ecl, args.Measurement.MeasurementStartTimeSaved, + address, parsed.Host, args.Session.Logger()) }, } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index c6fbe71ff2..fada5f71bb 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -14,7 +14,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") } } @@ -117,7 +117,9 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { for _, hs := range tk.TLSHandshakes { if hs.Failure != nil { if hs.ECHConfig == "GREASE" { - t.Fatal("unexpected exp failure:", hs.Failure) + 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) } diff --git a/internal/experiment/echcheck/verbatim.go b/internal/experiment/echcheck/verbatim.go new file mode 100644 index 0000000000..54beedab40 --- /dev/null +++ b/internal/experiment/echcheck/verbatim.go @@ -0,0 +1,126 @@ +// 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/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" +) + +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 From f17cfa8e7c31a8ca3ebc2bee73028cdc14dbee67 Mon Sep 17 00:00:00 2001 From: John Hess Date: Mon, 13 Jan 2025 14:06:10 -0600 Subject: [PATCH 02/10] use stdlib instead of utls; probe multiple ports; use retry_configs when available; tests --- cmd/ooniprobe/internal/nettests/echcheck.go | 6 +- go.mod | 2 - go.sum | 2 - internal/experiment/echcheck/echconfig.go | 30 ---- internal/experiment/echcheck/generate.go | 128 +++++++-------- internal/experiment/echcheck/generate_test.go | 17 ++ internal/experiment/echcheck/handshake.go | 146 +++++------------- .../experiment/echcheck/handshake_test.go | 122 ++++++++++++--- internal/experiment/echcheck/measure.go | 55 ++++--- internal/experiment/echcheck/measure_test.go | 6 +- internal/experiment/echcheck/utls.go | 49 ------ internal/experiment/echcheck/utls_test.go | 41 ----- internal/experiment/echcheck/verbatim.go | 3 + internal/netxlite/resolvercore.go | 26 +++- internal/netxlite/resolvercore_test.go | 68 ++++++++ 15 files changed, 356 insertions(+), 345 deletions(-) delete mode 100644 internal/experiment/echcheck/echconfig.go create mode 100644 internal/experiment/echcheck/generate_test.go delete mode 100644 internal/experiment/echcheck/utls.go delete mode 100644 internal/experiment/echcheck/utls_test.go diff --git a/cmd/ooniprobe/internal/nettests/echcheck.go b/cmd/ooniprobe/internal/nettests/echcheck.go index 15a45de713..b36c273e7d 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"), + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://cloudflare-ech.com:443"), + model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://min-ng.test.defo.ie:15443"), + }) } diff --git a/go.mod b/go.mod index 1538dba8ee..4946149729 100644 --- a/go.mod +++ b/go.mod @@ -2,8 +2,6 @@ module github.com/ooni/probe-cli/v3 go 1.23.4 -toolchain go1.22.2 - require ( filippo.io/age v1.2.0 github.com/AlecAivazis/survey/v2 v2.3.7 diff --git a/go.sum b/go.sum index 880ccc19fb..3862988514 100644 --- a/go.sum +++ b/go.sum @@ -406,8 +406,6 @@ github.com/ooni/oocrypto v0.7.0 h1:FBwabkaDoroWcw7cqnxD065IZv/cvWiww729T2xWDOw= github.com/ooni/oocrypto v0.7.0/go.mod h1:kzRkZ1AGWFWLJW+ncw94c9ulpEI0oyDGDj59tGKpFuc= github.com/ooni/oohttp v0.8.0 h1:D256SKWc8FFN5WvNpG0ImomtXP3deAiRmxoN2GfKUeQ= github.com/ooni/oohttp v0.8.0/go.mod h1:6KnSv/hwqZFegFugPEHUGFghmby/9LavhA3BtCE+RQ4= -github.com/ooni/probe-assets v0.24.0 h1:9y6bF9PyXrPBHu/RmyRZY8JOXHC6W2ZNRC7kaPcuHHk= -github.com/ooni/probe-assets v0.24.0/go.mod h1:m0k2FFzcLfFm7dhgyYkLCUR3R0CoRPr0jcjctDS2+gU= github.com/ooni/probe-assets v0.25.0 h1:W/zqKRjkRkTYKHURhiFIuflh+Trm1WaPUWSfVU/y2VA= github.com/ooni/probe-assets v0.25.0/go.mod h1:m0k2FFzcLfFm7dhgyYkLCUR3R0CoRPr0jcjctDS2+gU= github.com/oschwald/geoip2-golang v1.9.0 h1:uvD3O6fXAXs+usU+UGExshpdP13GAqp4GBrzN7IgKZc= diff --git a/internal/experiment/echcheck/echconfig.go b/internal/experiment/echcheck/echconfig.go deleted file mode 100644 index 8b10dc0e91..0000000000 --- a/internal/experiment/echcheck/echconfig.go +++ /dev/null @@ -1,30 +0,0 @@ -package echcheck - -import ( - "encoding/base64" -) - -const extensionEncryptedClientHello uint16 = 0xfe0d - -type echConfigList struct { - Configs []echConfig - raw []byte -} - -// The raw data of the ECHConfigList as it would appear in the DNS record. -func (ecl echConfigList) Base64() string { - return base64.StdEncoding.EncodeToString(ecl.raw) -} - -// Uses go's own tls package's parsing implementation and returns -// the result along with the raw data in an echConfigList. -func parseRawEchConfig(data []byte) (echConfigList, error) { - configs, err := parseECHConfigList(data) - if err != nil { - return echConfigList{}, err - } - return echConfigList{ - raw: data, - Configs: configs, - }, nil -} diff --git a/internal/experiment/echcheck/generate.go b/internal/experiment/echcheck/generate.go index 9d4521d8a2..a4cce2af60 100644 --- a/internal/experiment/echcheck/generate.go +++ b/internal/experiment/echcheck/generate.go @@ -4,92 +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" ) -const clientHelloOuter uint8 = 0 - -// echTLSExtension 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 echTLSExtension struct { - kdfID uint16 - aeadID uint16 - configID uint8 - enc []byte - payload []byte -} - -func (ech *echTLSExtension) 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 echTLSExtension - - 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 cbe5e72e71..c2fce8ce42 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -2,36 +2,19 @@ package echcheck import ( "context" - "crypto/rand" "crypto/tls" + "encoding/base64" "net" + "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 - -type EchMode int - -const ( - NoECH EchMode = iota - GreaseECH - RealECH -) - -// Connect to `host` at `address` and attempt a TLS handshake. When using real ECH, `ecl` must -// contain the ECHConfigList to use; in other modes, `ecl` is ignored. If the ECH config provides -// a different OuterServerName than `host`, it will be recorded in the result and used per go's -// tls package's behavior. -// Returns a channel that will contain the archival result of the handshake. -func connectAndHandshake(ctx context.Context, mode EchMode, ecl echConfigList, startTime time.Time, - address string, host string, logger model.Logger) (chan model.ArchivalTLSOrQUICHandshakeResult, error) { +func connectAndHandshake(ctx context.Context, echConfigList []byte, isGrease bool, startTime time.Time, address string, target *url.URL, outerServerName string, logger model.Logger) (chan model.ArchivalTLSOrQUICHandshakeResult, error) { channel := make(chan model.ArchivalTLSOrQUICHandshakeResult) @@ -43,109 +26,62 @@ func connectAndHandshake(ctx context.Context, mode EchMode, ecl echConfigList, s return nil, netxlite.NewErrWrapper(netxlite.ClassifyGenericError, netxlite.ConnectOperation, err) } + tlsConfig := genEchTLSConfig(target.Hostname(), echConfigList) + go func() { - var res *model.ArchivalTLSOrQUICHandshakeResult - switch mode { - case NoECH: - res = handshakeWithoutEch(ctx, conn, startTime, address, host, logger) - case GreaseECH: - res = handshakeWithGreaseyEch(ctx, conn, startTime, address, host, logger) - case RealECH: - res = handshakeWithRealEch(ctx, conn, startTime, address, host, ecl, logger) - } - channel <- *res + hs := handshake(ctx, conn, echConfigList, isGrease, startTime, address, logger, tlsConfig) + hs.OuterServerName = outerServerName + channel <- *hs }() return channel, nil } -func handshakeWithoutEch(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 handshakeWithGreaseyEch(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()) +func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGrease bool, startTime time.Time, address string, logger model.Logger, tlsConfig *tls.Config) *model.ArchivalTLSOrQUICHandshakeResult { + var d string + if isGrease { + d = " (GREASE)" + } else if len(echConfigList) > 0 { + d = " (RealECH)" } + ol := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) + start := time.Now() - var utlsEchExtension utls.GenericExtension - - utlsEchExtension.Id = echExtensionType - utlsEchExtension.Data = payload - - hs := handshakeWithExtension(ctx, conn, zeroTime, address, sni, []utls.TLSExtension{&utlsEchExtension}, logger) - hs.ECHConfig = "GREASE" - hs.OuterServerName = sni - return hs -} + maybeTLSConn := tls.Client(conn, tlsConfig) + err := maybeTLSConn.HandshakeContext(ctx) -func handshakeMaybePrintWithECH(doprint bool) string { - if doprint { - return "WithGreaseECH" + if echErr, ok := err.(*tls.ECHRejectionError); ok && isGrease { + if len(echErr.RetryConfigList) > 0 { + tlsConfig.EncryptedClientHelloConfigList = echErr.RetryConfigList + maybeTLSConn, err = tls.Dial("tcp", address, tlsConfig) + } } - return "" -} - -// ECHConfigList must be valid, non-empty, and to specify only one unique PublicName, -// i.e. OuterServerName across all configs. Host is the service to connect to, i.e. -// the inner SNI. -func handshakeWithRealEch(ctx context.Context, conn net.Conn, zeroTime time.Time, - address string, host string, ecl echConfigList, logger model.Logger) *model.ArchivalTLSOrQUICHandshakeResult { - - tlsConfig := genEchTLSConfig(host, ecl) - - ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshakeWithRealECH") - start := time.Now() - maybeTLSConn, err := tls.Dial("tcp", address, tlsConfig) finish := time.Now() ol.Stop(err) - connState := netxlite.MaybeTLSConnectionState(maybeTLSConn) - hs := measurexlite.NewArchivalTLSOrQUICHandshakeResult(0, start.Sub(zeroTime), "tcp", address, tlsConfig, - connState, err, finish.Sub(zeroTime)) - hs.ECHConfig = ecl.Base64() - hs.OuterServerName = string(ecl.Configs[0].PublicName) + var connState tls.ConnectionState + // If there's been an error, processing maybeTLSConn can panic. + if err != nil { + connState = tls.ConnectionState{} + } else { + connState = netxlite.MaybeTLSConnectionState(maybeTLSConn) + } + 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(echConfigList) + } return hs } -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) - - ol := logx.NewOperationLogger(logger, "echcheck: TLSHandshake%s", handshakeMaybePrintWithECH(len(extensions) > 0)) - start := time.Now() - maybeTLSConn, err := tracedHandshaker.Handshake(ctx, conn, tlsConfig) - finish := time.Now() - ol.Stop(err) - - connState := netxlite.MaybeTLSConnectionState(maybeTLSConn) - return measurexlite.NewArchivalTLSOrQUICHandshakeResult(0, start.Sub(zeroTime), "tcp", address, tlsConfig, - connState, err, finish.Sub(zeroTime)) -} - -// 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) *tls.Config { + if len(echConfigList) == 0 { + return &tls.Config{ServerName: host} } -} - -func genEchTLSConfig(host string, ecl echConfigList) *tls.Config { return &tls.Config{ - EncryptedClientHelloConfigList: ecl.raw, + EncryptedClientHelloConfigList: echConfigList, // This will be used as the inner SNI and we will validate // we get a certificate for this name. The outer SNI will // be set based on the ECH config. diff --git a/internal/experiment/echcheck/handshake_test.go b/internal/experiment/echcheck/handshake_test.go index bb9d3537a0..9209b16749 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -2,49 +2,135 @@ package echcheck import ( "context" + "crypto/rand" + "crypto/tls" + "crypto/x509" "fmt" "net" "net/http" "net/http/httptest" "net/url" + "strings" "testing" "time" "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 := handshakeWithGreaseyEch(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, *model.ArchivalTLSOrQUICHandshakeResult) + }{ + { + name: "no ECH", + sendGrease: false, + useRetryConfigs: false, + want: func(t *testing.T, testConfig testServerConfig, result *model.ArchivalTLSOrQUICHandshakeResult) { + if result.SoError != nil { + t.Fatal("did not expect error, got: ", result.SoError) + } + if result.Failure != nil { + t.Fatal("did not expect error, got: ", *result.Failure) + } + if result.OuterServerName != "" { + t.Fatal("expected OuterServerName to be empty, got: ", result.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 *model.ArchivalTLSOrQUICHandshakeResult) { + if result.ServerName != testConfig.url.Hostname() { + t.Fatal("expected ServerName to be set to ts.URL.Hostname(), got: ", result.ServerName) + } + + if result.SoError != nil { + t.Fatal("did not expect error, got: ", result.SoError) + } - if result.Failure != nil { - t.Fatal("did not expect error, got: ", *result.Failure) + if result.Failure == nil || !strings.Contains(*result.Failure, "tls: server rejected ECH") { + t.Fatal("server should have rejected ECH: ", *result.Failure) + } + }, + }, + { + name: "GREASEy ECH handshake", + sendGrease: true, + useRetryConfigs: true, + want: func(t *testing.T, testConfig testServerConfig, result *model.ArchivalTLSOrQUICHandshakeResult) { + if result.ECHConfig != "GREASE" { + t.Fatal("expected ECHConfig to be string literal 'GREASE', got: ", result.ECHConfig) + } + if result.SoError != nil { + t.Fatal("did not expect error, got: ", result.SoError) + } + if result.Failure == nil || !strings.Contains(*result.Failure, "tls: server rejected ECH") { + t.Fatal("expected Connection to fail because test server doesn't handle ECH yet") + } + }, + }, + // TODO: Add a test case with Real ECH once the server-side of crypto/tls supports it. } - conn.Close() + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testConfig := setupTest(t) + defer testConfig.ts.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() + conn, err := net.Dial("tcp", testConfig.url.Host) + if err != nil { + t.Fatal(err) + } + result := handshake(ctx, conn, ecl, test.useRetryConfigs, time.Now(), testConfig.url.Host, model.DiscardLogger, testConfig.tlsConfig) + test.want(t, testConfig, result) + }) + } } diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index 9958671eb5..581f35d6ed 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -2,6 +2,7 @@ package echcheck import ( "context" + crand "crypto/rand" "errors" "fmt" "math/rand/v2" @@ -65,30 +66,35 @@ 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()) // We dial the alias, even when there are hints in the HTTPS record. - addrs, addrsErr := resolver.LookupHost(ctx, parsed.Host) - httpsRr, httpsErr := resolver.LookupHTTPS(ctx, parsed.Host) + 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 } - rawEchConfig := httpsRr.Ech - ecl, err := parseRawEchConfig(rawEchConfig) + realEchConfig := httpsRr.Ech + configs, err := parseECHConfigList(realEchConfig) if err != nil { return fmt.Errorf("failed to parse ECH config: %w", err) } - if len(ecl.Configs) == 0 { - return fmt.Errorf("no ECH configs for %s", parsed.Host) - } - outerServerName := string(ecl.Configs[0].PublicName) - for _, ec := range ecl.Configs { + // outerServerName is Populated in results when ECH is used. + 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 @@ -96,27 +102,38 @@ func (m *Measurer) Run( return fmt.Errorf("ambigious OuterServerName for %s", parsed.Host) } } + grease, err := generateGreaseyECHConfigList(crand.Reader, parsed.Hostname()) + if err != nil { + 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") + port := parsed.Port() + if port == "" { + port = "443" + } + address := net.JoinHostPort(addrs[0], port) handshakes := []func() (chan model.ArchivalTLSOrQUICHandshakeResult, error){ // Handshake with no ECH func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, NoECH, echConfigList{}, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, args.Session.Logger()) + return connectAndHandshake(ctx, []byte{}, false, + args.Measurement.MeasurementStartTimeSaved, + address, parsed, "", args.Session.Logger()) }, // Handshake with ECH GREASE func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, GreaseECH, echConfigList{}, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, args.Session.Logger()) + return connectAndHandshake(ctx, grease, true, + args.Measurement.MeasurementStartTimeSaved, + address, parsed, outerServerName, args.Session.Logger()) }, - // Use real ECH + // Handshake with real ECH func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { - return connectAndHandshake(ctx, RealECH, ecl, args.Measurement.MeasurementStartTimeSaved, - address, parsed.Host, args.Session.Logger()) + return connectAndHandshake(ctx, realEchConfig, false, + args.Measurement.MeasurementStartTimeSaved, + address, parsed, outerServerName, args.Session.Logger()) }, } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index fada5f71bb..178fc0cd1f 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -117,11 +117,11 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { for _, hs := range tk.TLSHandshakes { if hs.Failure != nil { if hs.ECHConfig == "GREASE" { - t.Fatal("unexpected exp (grease) failure:", hs.Failure) + t.Fatal("unexpected exp (grease) failure:", *hs.Failure) } else if len(hs.ECHConfig) > 0 { - t.Fatal("unexpected exp (ech) failure:", hs.Failure) + 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 index 54beedab40..66fe96e056 100644 --- a/internal/experiment/echcheck/verbatim.go +++ b/internal/experiment/echcheck/verbatim.go @@ -1,4 +1,5 @@ // 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. @@ -15,6 +16,8 @@ import ( "golang.org/x/crypto/cryptobyte" ) +const extensionEncryptedClientHello uint16 = 0xfe0d + type echCipher struct { KDFID uint16 AEADID uint16 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) { From 7f6104f25acb7bcc59ef1f174f4eb5f4d7222057 Mon Sep 17 00:00:00 2001 From: John Hess Date: Fri, 24 Jan 2025 20:33:58 +0100 Subject: [PATCH 03/10] capture ArchicalTCPConnectResult(s) --- internal/experiment/echcheck/handshake.go | 42 +++++++----- .../experiment/echcheck/handshake_test.go | 67 ++++++++++++------- internal/experiment/echcheck/measure.go | 45 ++++++++----- 3 files changed, 93 insertions(+), 61 deletions(-) diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index c2fce8ce42..9da1331f60 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -3,6 +3,7 @@ package echcheck import ( "context" "crypto/tls" + "crypto/x509" "encoding/base64" "net" "net/url" @@ -14,30 +15,33 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -func connectAndHandshake(ctx context.Context, echConfigList []byte, isGrease bool, startTime time.Time, address string, target *url.URL, outerServerName string, logger model.Logger) (chan model.ArchivalTLSOrQUICHandshakeResult, error) { +func connectAndHandshake(ctx context.Context, echConfigList []byte, isGrease bool, startTime time.Time, address string, target *url.URL, outerServerName string, logger model.Logger, testOnlyRootCAs *x509.CertPool) (chan TestKeys, error) { - channel := make(chan model.ArchivalTLSOrQUICHandshakeResult) + channel := make(chan TestKeys) ol := logx.NewOperationLogger(logger, "echcheck: TCPConnect %s", address) - var dialer net.Dialer + trace := measurexlite.NewTrace(0, startTime) + dialer := trace.NewDialerWithoutResolver(logger) conn, err := dialer.DialContext(ctx, "tcp", address) ol.Stop(err) if err != nil { return nil, netxlite.NewErrWrapper(netxlite.ClassifyGenericError, netxlite.ConnectOperation, err) } - tlsConfig := genEchTLSConfig(target.Hostname(), echConfigList) + tlsConfig := genEchTLSConfig(target.Hostname(), echConfigList, testOnlyRootCAs) go func() { - hs := handshake(ctx, conn, echConfigList, isGrease, startTime, address, logger, tlsConfig) - hs.OuterServerName = outerServerName - channel <- *hs + tk := handshake(ctx, conn, echConfigList, isGrease, startTime, address, logger, tlsConfig) + tk.TLSHandshakes[0].OuterServerName = outerServerName + tcpcs := trace.TCPConnects() + tk.TCPConnect = append(tk.TCPConnect, tcpcs...) + channel <- tk }() return channel, nil } -func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGrease bool, startTime time.Time, address string, logger model.Logger, tlsConfig *tls.Config) *model.ArchivalTLSOrQUICHandshakeResult { +func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGrease bool, startTime time.Time, address string, logger model.Logger, tlsConfig *tls.Config) TestKeys { var d string if isGrease { d = " (GREASE)" @@ -53,6 +57,7 @@ func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGreas if echErr, ok := err.(*tls.ECHRejectionError); ok && isGrease { if len(echErr.RetryConfigList) > 0 { tlsConfig.EncryptedClientHelloConfigList = echErr.RetryConfigList + // TODO: trace this TCP connection maybeTLSConn, err = tls.Dial("tcp", address, tlsConfig) } } @@ -73,18 +78,19 @@ func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGreas } else { hs.ECHConfig = base64.StdEncoding.EncodeToString(echConfigList) } - return hs + tk := TestKeys{ + TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{hs}, + } + return tk } -func genEchTLSConfig(host string, echConfigList []byte) *tls.Config { - if len(echConfigList) == 0 { - return &tls.Config{ServerName: host} +func genEchTLSConfig(host string, echConfigList []byte, testOnlyRootCAs *x509.CertPool) *tls.Config { + c := &tls.Config{ServerName: host} + if len(echConfigList) > 0 { + c.EncryptedClientHelloConfigList = echConfigList } - return &tls.Config{ - EncryptedClientHelloConfigList: echConfigList, - // This will be used as the inner SNI and we will validate - // we get a certificate for this name. The outer SNI will - // be set based on the ECH config. - ServerName: host, + 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 9209b16749..ad0fa39496 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -6,7 +6,6 @@ import ( "crypto/tls" "crypto/x509" "fmt" - "net" "net/http" "net/http/httptest" "net/url" @@ -52,21 +51,27 @@ func TestHandshake(t *testing.T) { name string sendGrease bool useRetryConfigs bool - want func(*testing.T, testServerConfig, *model.ArchivalTLSOrQUICHandshakeResult) + want func(*testing.T, testServerConfig, TestKeys) }{ { name: "no ECH", sendGrease: false, useRetryConfigs: false, - want: func(t *testing.T, testConfig testServerConfig, result *model.ArchivalTLSOrQUICHandshakeResult) { - if result.SoError != nil { - t.Fatal("did not expect error, got: ", result.SoError) + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + if len(result.TCPConnect) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) } - if result.Failure != nil { - t.Fatal("did not expect error, got: ", *result.Failure) + if len(result.TLSHandshakes) != 1 { + t.Fatal("expected exactly one TLS handshake, got: ", len(result.TLSHandshakes)) } - if result.OuterServerName != "" { - t.Fatal("expected OuterServerName to be empty, got: ", result.OuterServerName) + if result.TLSHandshakes[0].SoError != nil { + t.Fatal("did not expect 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) } }, }, @@ -76,17 +81,21 @@ func TestHandshake(t *testing.T) { // 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 *model.ArchivalTLSOrQUICHandshakeResult) { - if result.ServerName != testConfig.url.Hostname() { - t.Fatal("expected ServerName to be set to ts.URL.Hostname(), got: ", result.ServerName) + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + if len(result.TCPConnect) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) } - - if result.SoError != nil { - t.Fatal("did not expect error, got: ", result.SoError) + if len(result.TLSHandshakes) != 1 { + t.Fatal("expected exactly one TLS handshake, got: ", len(result.TLSHandshakes)) } - - if result.Failure == nil || !strings.Contains(*result.Failure, "tls: server rejected ECH") { - t.Fatal("server should have rejected ECH: ", *result.Failure) + 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 error, got: ", result.TLSHandshakes[0].SoError) + } + if result.TLSHandshakes[0].Failure == nil || !strings.Contains(*result.TLSHandshakes[0].Failure, "tls: server rejected ECH") { + t.Fatal("server should have rejected ECH: ", *result.TLSHandshakes[0].Failure) } }, }, @@ -94,14 +103,20 @@ func TestHandshake(t *testing.T) { name: "GREASEy ECH handshake", sendGrease: true, useRetryConfigs: true, - want: func(t *testing.T, testConfig testServerConfig, result *model.ArchivalTLSOrQUICHandshakeResult) { - if result.ECHConfig != "GREASE" { - t.Fatal("expected ECHConfig to be string literal 'GREASE', got: ", result.ECHConfig) + want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { + if len(result.TCPConnect) != 1 { + t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) + } + 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 ECHConfig to be string literal 'GREASE', got: ", result.TLSHandshakes[0].ECHConfig) } - if result.SoError != nil { - t.Fatal("did not expect error, got: ", result.SoError) + if result.TLSHandshakes[0].SoError != nil { + t.Fatal("did not expect error, got: ", result.TLSHandshakes[0].SoError) } - if result.Failure == nil || !strings.Contains(*result.Failure, "tls: server rejected ECH") { + if result.TLSHandshakes[0].Failure == nil || !strings.Contains(*result.TLSHandshakes[0].Failure, "tls: server rejected ECH") { t.Fatal("expected Connection to fail because test server doesn't handle ECH yet") } }, @@ -125,11 +140,11 @@ func TestHandshake(t *testing.T) { } ctx := context.Background() - conn, err := net.Dial("tcp", testConfig.url.Host) + ch, err := connectAndHandshake(ctx, ecl, test.useRetryConfigs, time.Now(), testConfig.url.Host, testConfig.url, "", model.DiscardLogger, testConfig.tlsConfig.RootCAs) if err != nil { t.Fatal(err) } - result := handshake(ctx, conn, ecl, test.useRetryConfigs, time.Now(), testConfig.url.Host, model.DiscardLogger, testConfig.tlsConfig) + result := <-ch test.want(t, testConfig, result) }) } diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index 581f35d6ed..e8bc552e87 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -31,6 +31,9 @@ var ( // TestKeys contains echcheck test keys. type TestKeys struct { + NetworkEvents []*model.ArchivalNetworkEvent `json:"network_events"` + Queries []*model.ArchivalDNSLookupResult `json:"queries"` + TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connects"` TLSHandshakes []*model.ArchivalTLSOrQUICHandshakeResult `json:"tls_handshakes"` } @@ -114,26 +117,26 @@ func (m *Measurer) Run( } address := net.JoinHostPort(addrs[0], port) - handshakes := []func() (chan model.ArchivalTLSOrQUICHandshakeResult, error){ + handshakes := []func() (chan TestKeys, error){ // Handshake with no ECH - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { + func() (chan TestKeys, error) { return connectAndHandshake(ctx, []byte{}, false, - args.Measurement.MeasurementStartTimeSaved, - address, parsed, "", args.Session.Logger()) + args.Measurement.MeasurementStartTimeSaved, address, + parsed, "", args.Session.Logger(), nil) }, // Handshake with ECH GREASE - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { + func() (chan TestKeys, error) { return connectAndHandshake(ctx, grease, true, - args.Measurement.MeasurementStartTimeSaved, - address, parsed, outerServerName, args.Session.Logger()) + args.Measurement.MeasurementStartTimeSaved, address, + parsed, outerServerName, args.Session.Logger(), nil) }, // Handshake with real ECH - func() (chan model.ArchivalTLSOrQUICHandshakeResult, error) { + func() (chan TestKeys, error) { return connectAndHandshake(ctx, realEchConfig, false, - args.Measurement.MeasurementStartTimeSaved, - address, parsed, outerServerName, args.Session.Logger()) + args.Measurement.MeasurementStartTimeSaved, address, + parsed, outerServerName, args.Session.Logger(), nil) }, } @@ -143,8 +146,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 @@ -157,14 +159,23 @@ func (m *Measurer) Run( } } + alltks := TestKeys{ + TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, + NetworkEvents: []*model.ArchivalNetworkEvent{}, + Queries: []*model.ArchivalDNSLookupResult{}, + TCPConnect: []*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.TCPConnect = append(alltks.TCPConnect, tk.TCPConnect...) } - args.Measurement.TestKeys = TestKeys{TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{ - &results[0], &results[1], &results[2], - }} + args.Measurement.TestKeys = alltks return nil } From 46442c67bf63e83e3d8338d18778a3b827b60379 Mon Sep 17 00:00:00 2001 From: John Hess Date: Fri, 24 Jan 2025 23:36:12 +0100 Subject: [PATCH 04/10] capture additional TLS connections; refactor --- internal/experiment/echcheck/handshake.go | 101 ++++++++++++------ .../experiment/echcheck/handshake_test.go | 43 +++++--- internal/experiment/echcheck/measure.go | 32 ++---- internal/experiment/echcheck/measure_test.go | 14 ++- 4 files changed, 114 insertions(+), 76 deletions(-) diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index 9da1331f60..864c6e8563 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -5,7 +5,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/base64" - "net" + "fmt" "net/url" "time" @@ -15,72 +15,103 @@ import ( "github.com/ooni/probe-cli/v3/internal/netxlite" ) -func connectAndHandshake(ctx context.Context, echConfigList []byte, isGrease bool, startTime time.Time, address string, target *url.URL, outerServerName string, logger model.Logger, testOnlyRootCAs *x509.CertPool) (chan TestKeys, error) { - - channel := make(chan TestKeys) - - ol := logx.NewOperationLogger(logger, "echcheck: TCPConnect %s", address) - trace := measurexlite.NewTrace(0, startTime) - dialer := trace.NewDialerWithoutResolver(logger) - 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) } + 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") + } + } + return outerServerName, nil +} + +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) { + + channel := make(chan TestKeys) tlsConfig := genEchTLSConfig(target.Hostname(), echConfigList, testOnlyRootCAs) go func() { - tk := handshake(ctx, conn, echConfigList, isGrease, startTime, address, logger, tlsConfig) - tk.TLSHandshakes[0].OuterServerName = outerServerName - tcpcs := trace.TCPConnects() - tk.TCPConnect = append(tk.TCPConnect, tcpcs...) + tk := TestKeys{} + tk = handshake(ctx, isGrease, startTime, address, logger, tlsConfig, tk) channel <- tk }() return channel, nil } -func handshake(ctx context.Context, conn net.Conn, echConfigList []byte, isGrease bool, startTime time.Time, address string, logger model.Logger, tlsConfig *tls.Config) TestKeys { +// 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(echConfigList) > 0 { + } else if len(tlsConfig.EncryptedClientHelloConfigList) > 0 { d = " (RealECH)" } - ol := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) - start := time.Now() + 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) + newTcpcs := trace.TCPConnects() + tk.TCPConnects = append(tk.TCPConnects, newTcpcs...) + + ol2 := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) + start := time.Now() maybeTLSConn := tls.Client(conn, tlsConfig) - err := maybeTLSConn.HandshakeContext(ctx) + err = maybeTLSConn.HandshakeContext(ctx) + finish := time.Now() + ol2.Stop(err) + retryConfigs := []byte{} if echErr, ok := err.(*tls.ECHRejectionError); ok && isGrease { if len(echErr.RetryConfigList) > 0 { - tlsConfig.EncryptedClientHelloConfigList = echErr.RetryConfigList - // TODO: trace this TCP connection - maybeTLSConn, err = tls.Dial("tcp", address, tlsConfig) + 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 } - finish := time.Now() - ol.Stop(err) - var connState tls.ConnectionState - // If there's been an error, processing maybeTLSConn can panic. - if err != nil { - connState = tls.ConnectionState{} - } else { - connState = netxlite.MaybeTLSConnectionState(maybeTLSConn) - } + connState := netxlite.MaybeTLSConnectionState(maybeTLSConn) 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(echConfigList) + hs.ECHConfig = base64.StdEncoding.EncodeToString(tlsConfig.EncryptedClientHelloConfigList) } - tk := TestKeys{ - TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{hs}, + 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 } diff --git a/internal/experiment/echcheck/handshake_test.go b/internal/experiment/echcheck/handshake_test.go index ad0fa39496..f71859f0ab 100644 --- a/internal/experiment/echcheck/handshake_test.go +++ b/internal/experiment/echcheck/handshake_test.go @@ -9,7 +9,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "strings" "testing" "time" @@ -58,14 +57,14 @@ func TestHandshake(t *testing.T) { sendGrease: false, useRetryConfigs: false, want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { - if len(result.TCPConnect) != 1 { - t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) + 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 error, got: ", result.TLSHandshakes[0].SoError) + 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) @@ -82,8 +81,8 @@ func TestHandshake(t *testing.T) { sendGrease: true, useRetryConfigs: false, want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { - if len(result.TCPConnect) != 1 { - t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) + 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)) @@ -92,10 +91,13 @@ func TestHandshake(t *testing.T) { 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 error, got: ", result.TLSHandshakes[0].SoError) + t.Fatal("did not expect soft error, got: ", *result.TLSHandshakes[0].SoError) } - if result.TLSHandshakes[0].Failure == nil || !strings.Contains(*result.TLSHandshakes[0].Failure, "tls: server rejected ECH") { - t.Fatal("server should have rejected ECH: ", *result.TLSHandshakes[0].Failure) + 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) } }, }, @@ -104,20 +106,27 @@ func TestHandshake(t *testing.T) { sendGrease: true, useRetryConfigs: true, want: func(t *testing.T, testConfig testServerConfig, result TestKeys) { - if len(result.TCPConnect) != 1 { - t.Fatal("expected exactly one TCPConnect, got: ", len(result.TCPConnect)) + // 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 ECHConfig to be string literal 'GREASE', got: ", result.TLSHandshakes[0].ECHConfig) + 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 error, got: ", result.TLSHandshakes[0].SoError) + 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].Failure == nil || !strings.Contains(*result.TLSHandshakes[0].Failure, "tls: server rejected ECH") { - t.Fatal("expected Connection to fail because test server doesn't handle ECH yet") + if result.TLSHandshakes[0].OuterServerName != testConfig.url.Hostname() { + t.Fatal("expected OuterServerName to be testserver name, got: ", result.TLSHandshakes[0].OuterServerName) } }, }, @@ -136,11 +145,11 @@ func TestHandshake(t *testing.T) { t.Fatal(err) } ecl = grease - testConfig.tlsConfig.EncryptedClientHelloConfigList = ecl } + testConfig.tlsConfig.EncryptedClientHelloConfigList = ecl ctx := context.Background() - ch, err := connectAndHandshake(ctx, ecl, test.useRetryConfigs, time.Now(), testConfig.url.Host, testConfig.url, "", model.DiscardLogger, testConfig.tlsConfig.RootCAs) + 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) } diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index e8bc552e87..c877e67d06 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -33,7 +33,7 @@ var ( type TestKeys struct { NetworkEvents []*model.ArchivalNetworkEvent `json:"network_events"` Queries []*model.ArchivalDNSLookupResult `json:"queries"` - TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connects"` + TCPConnects []*model.ArchivalTCPConnectResult `json:"tcp_connects"` TLSHandshakes []*model.ArchivalTLSOrQUICHandshakeResult `json:"tls_handshakes"` } @@ -91,20 +91,6 @@ func (m *Measurer) Run( return httpsErr } realEchConfig := httpsRr.Ech - configs, err := parseECHConfigList(realEchConfig) - if err != nil { - return fmt.Errorf("failed to parse ECH config: %w", err) - } - // outerServerName is Populated in results when ECH is used. - 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 %s", parsed.Host) - } - } grease, err := generateGreaseyECHConfigList(crand.Reader, parsed.Hostname()) if err != nil { return fmt.Errorf("failed to generate GREASE ECH config: %w", err) @@ -120,23 +106,23 @@ func (m *Measurer) Run( handshakes := []func() (chan TestKeys, error){ // Handshake with no ECH func() (chan TestKeys, error) { - return connectAndHandshake(ctx, []byte{}, false, + return startHandshake(ctx, []byte{}, false, args.Measurement.MeasurementStartTimeSaved, address, - parsed, "", args.Session.Logger(), nil) + parsed, args.Session.Logger(), nil) }, // Handshake with ECH GREASE func() (chan TestKeys, error) { - return connectAndHandshake(ctx, grease, true, + return startHandshake(ctx, grease, true, args.Measurement.MeasurementStartTimeSaved, address, - parsed, outerServerName, args.Session.Logger(), nil) + parsed, args.Session.Logger(), nil) }, // Handshake with real ECH func() (chan TestKeys, error) { - return connectAndHandshake(ctx, realEchConfig, false, + return startHandshake(ctx, realEchConfig, false, args.Measurement.MeasurementStartTimeSaved, address, - parsed, outerServerName, args.Session.Logger(), nil) + parsed, args.Session.Logger(), nil) }, } @@ -163,7 +149,7 @@ func (m *Measurer) Run( TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, NetworkEvents: []*model.ArchivalNetworkEvent{}, Queries: []*model.ArchivalDNSLookupResult{}, - TCPConnect: []*model.ArchivalTCPConnectResult{}, + TCPConnects: []*model.ArchivalTCPConnectResult{}, } // Wait on each channel for the results to come in @@ -172,7 +158,7 @@ func (m *Measurer) Run( alltks.TLSHandshakes = append(alltks.TLSHandshakes, tk.TLSHandshakes...) alltks.NetworkEvents = append(alltks.NetworkEvents, tk.NetworkEvents...) alltks.Queries = append(alltks.Queries, tk.Queries...) - alltks.TCPConnect = append(alltks.TCPConnect, tk.TCPConnect...) + alltks.TCPConnects = append(alltks.TCPConnects, tk.TCPConnects...) } args.Measurement.TestKeys = alltks diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 178fc0cd1f..47a8aada4e 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -2,6 +2,7 @@ package echcheck import ( "context" + "strings" "testing" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -114,10 +115,21 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // check results tk := msrmnt.TestKeys.(TestKeys) + // 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 (grease) 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 { From 599a449ef242336f18ab019f2e27106bb36da8e1 Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 09:12:00 -0500 Subject: [PATCH 05/10] capture A/AAAA DNS queries before handshake attempts --- internal/experiment/echcheck/measure.go | 2 +- internal/experiment/echcheck/measure_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index c877e67d06..cfe3c936a7 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -148,7 +148,7 @@ func (m *Measurer) Run( alltks := TestKeys{ TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, NetworkEvents: []*model.ArchivalNetworkEvent{}, - Queries: []*model.ArchivalDNSLookupResult{}, + Queries: trace.DNSLookupsFromRoundTrip(), TCPConnects: []*model.ArchivalTCPConnectResult{}, } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 47a8aada4e..4a869fc655 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -115,6 +115,10 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // check results tk := msrmnt.TestKeys.(TestKeys) + if len(tk.Queries) != 3 { + // TODO Check that expected DNS Queries are included + t.Fatal("unexpected number of DNS Queries recorded", len(tk.Queries)) + } // NoECH, GREASE, GREASE retry, RealECH if len(tk.TLSHandshakes) != 4 { t.Fatal("unexpected number of TLS handshakes", len(tk.TLSHandshakes)) From 3b12900461cc8e64446bd3907bdbc85dc2f768b3 Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 09:59:25 -0500 Subject: [PATCH 06/10] capture net events during tcp connection --- internal/experiment/echcheck/handshake.go | 4 ++-- internal/experiment/echcheck/measure_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/experiment/echcheck/handshake.go b/internal/experiment/echcheck/handshake.go index 864c6e8563..7e1e177670 100644 --- a/internal/experiment/echcheck/handshake.go +++ b/internal/experiment/echcheck/handshake.go @@ -67,8 +67,8 @@ func handshake(ctx context.Context, isGrease bool, startTime time.Time, address dialer := trace.NewDialerWithoutResolver(logger) conn, err := dialer.DialContext(ctx, "tcp", address) ol1.Stop(err) - newTcpcs := trace.TCPConnects() - tk.TCPConnects = append(tk.TCPConnects, newTcpcs...) + tk.TCPConnects = append(tk.TCPConnects, trace.TCPConnects()...) + tk.NetworkEvents = append(tk.NetworkEvents, trace.NetworkEvents()...) ol2 := logx.NewOperationLogger(logger, "echcheck: DialTLS%s", d) start := time.Now() diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 4a869fc655..bf43ea0108 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -119,6 +119,9 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // TODO Check that expected DNS Queries are included t.Fatal("unexpected number of DNS Queries recorded", len(tk.Queries)) } + if len(tk.NetworkEvents) != 4 { + t.Fatal("unexpected number of Network events recorded", len(tk.NetworkEvents)) + } // NoECH, GREASE, GREASE retry, RealECH if len(tk.TLSHandshakes) != 4 { t.Fatal("unexpected number of TLS handshakes", len(tk.TLSHandshakes)) From 19e6ee8b12792e9cd06ea003acf1d1bb2b6ed6bd Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 11:33:06 -0500 Subject: [PATCH 07/10] remove superfluous test --- cmd/ooniprobe/internal/nettests/echcheck.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ooniprobe/internal/nettests/echcheck.go b/cmd/ooniprobe/internal/nettests/echcheck.go index b36c273e7d..42bacedc6e 100644 --- a/cmd/ooniprobe/internal/nettests/echcheck.go +++ b/cmd/ooniprobe/internal/nettests/echcheck.go @@ -15,7 +15,7 @@ func (n ECHCheck) Run(ctl *Controller) error { // to recognize the empty string and use the default URL return ctl.Run(builder, []model.ExperimentTarget{ model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://cloudflare-ech.com/cdn-cgi/trace"), - model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://cloudflare-ech.com:443"), + // Use ECH on a non-standard port. model.NewOOAPIURLInfoWithDefaultCategoryAndCountry("https://min-ng.test.defo.ie:15443"), }) } From 799620c87587fdaae626a7efcc8a460083dc074a Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 11:48:56 -0500 Subject: [PATCH 08/10] capture net events during dns lookups --- internal/experiment/echcheck/measure.go | 2 +- internal/experiment/echcheck/measure_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/experiment/echcheck/measure.go b/internal/experiment/echcheck/measure.go index cfe3c936a7..833f5b5f2b 100644 --- a/internal/experiment/echcheck/measure.go +++ b/internal/experiment/echcheck/measure.go @@ -147,7 +147,7 @@ func (m *Measurer) Run( alltks := TestKeys{ TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, - NetworkEvents: []*model.ArchivalNetworkEvent{}, + NetworkEvents: trace.NetworkEvents(), Queries: trace.DNSLookupsFromRoundTrip(), TCPConnects: []*model.ArchivalTCPConnectResult{}, } diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index bf43ea0108..36f0f23f5b 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -119,8 +119,8 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // TODO Check that expected DNS Queries are included t.Fatal("unexpected number of DNS Queries recorded", len(tk.Queries)) } - if len(tk.NetworkEvents) != 4 { - t.Fatal("unexpected number of Network events recorded", len(tk.NetworkEvents)) + if len(tk.NetworkEvents) == 0 { + t.Fatal("no network events recorded") } // NoECH, GREASE, GREASE retry, RealECH if len(tk.TLSHandshakes) != 4 { From ce019905539bf9b396c302d8e2231c5f23310439 Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 13:15:20 -0500 Subject: [PATCH 09/10] trace LookupHTTPS DNS roundtrip --- internal/netxlite/resolverparallel.go | 5 +++++ 1 file changed, 5 insertions(+) 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() } From 4b4cd145befe54e1dc8a9f5fc497259e31e5ace5 Mon Sep 17 00:00:00 2001 From: John Hess Date: Thu, 13 Feb 2025 13:46:03 -0500 Subject: [PATCH 10/10] test for specific dns queries --- internal/experiment/echcheck/measure_test.go | 32 ++++++++++++++++++-- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/internal/experiment/echcheck/measure_test.go b/internal/experiment/echcheck/measure_test.go index 36f0f23f5b..a37e5e67ee 100644 --- a/internal/experiment/echcheck/measure_test.go +++ b/internal/experiment/echcheck/measure_test.go @@ -2,6 +2,7 @@ package echcheck import ( "context" + "net/url" "strings" "testing" @@ -115,9 +116,34 @@ func TestMeasurementSuccessRealWorld(t *testing.T) { // check results tk := msrmnt.TestKeys.(TestKeys) - if len(tk.Queries) != 3 { - // TODO Check that expected DNS Queries are included - t.Fatal("unexpected number of DNS Queries recorded", len(tk.Queries)) + 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")