Skip to content

Conversation

fortuna
Copy link
Contributor

@fortuna fortuna commented Sep 2, 2025

Not working yet. For some reason I'm getting server rejections. Not sure what I'm doing wrong.

% go -C x run ./tools/fetch -v --ech-config=grease https://example.com
Sep  2 11:21:48.572 ERR HTTP request failed error="Get \"https://example.com\": tls: server rejected ECH"
exit status 1

Error source:
https://cs.opensource.google/search?q=%22server%20rejected%20ECH%22&ss=go%2Fgo

Note that with a valid ECH Config from a different site it does the handshake (though the certificate validation fails as expected):

% go -C x run ./tools/fetch -v --ech-config=AEb+DQBCqQAgACBlm7cfDx/gKuUAwRTe+Y9MExbIyuLpLcgTORIdi69uewAEAAEAAQATcHVibGljLnRlc3QuZGVmby5pZQAA https://example.com
Sep  2 11:22:24.446 ERR HTTP request failed error="Get \"https://example.com\": tls: failed to verify certificate: x509: certificate is valid for a248.e.akamai.net, *.akamaized.net, *.akamaized-staging.net, *.akamaihd.net, *.akamaihd-staging.net, not public.test.defo.ie"
exit status 1

@fortuna fortuna requested a review from Copilot September 2, 2025 15:18
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for ECH (Encrypted Client Hello) GREASE functionality to the fetch tool. The implementation allows testing ECH behavior by generating GREASE ECH configurations that can help identify middleboxes that reject ECH-enabled connections.

  • Implements ECH GREASE configuration generation using HPKE X25519 key pairs
  • Adds command-line support for ECH configuration in the fetch tool
  • Includes comprehensive test coverage for the ECH GREASE functionality

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
x/tools/fetch/main.go Adds ECH support with --ech-config flag and GREASE mode
x/ech/ech_grease.go Implements ECH GREASE configuration list generation
x/ech/ech_grease_test.go Provides test coverage for ECH GREASE functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// Can we make it work with a fake domain that validates the right domain?
echConfigBytes, err := ech.GenerateGreaseECHConfigList(rand.Reader, reqURL.Hostname())
if err != nil {
slog.Error("Failed to decode base64 ECH config", "error", err)
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The error message is misleading for the GREASE case since no base64 decoding is performed. Consider using a more accurate message like 'Failed to generate GREASE ECH config'.

Suggested change
slog.Error("Failed to decode base64 ECH config", "error", err)
slog.Error("Failed to generate GREASE ECH config", "error", err)

Copilot uses AI. Check for mistakes.

}

// uint8 maximum_name_length
b.AddUint8(uint8(42))
Copy link
Preview

Copilot AI Sep 2, 2025

Choose a reason for hiding this comment

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

The magic number 42 for maximum_name_length should be replaced with a named constant or documented inline comment explaining its purpose.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant