Skip to content

ECH: cleanups#21532

Closed
bagder wants to merge 1 commit into
masterfrom
bagder/setopt-ech
Closed

ECH: cleanups#21532
bagder wants to merge 1 commit into
masterfrom
bagder/setopt-ech

Conversation

@bagder

@bagder bagder commented May 7, 2026

Copy link
Copy Markdown
Member
  • passing an unknown string to CURLOPT_ECH now returns error

    To properly allow applications to spot if they pass in a typo or
    something to libcurl.

  • CURLECH_DISABLE is now a plain zero internally, not a dedicated bit which
    simplifies checks for when ECH is enabled

  • Dropped the CURLECH_CLA_CFG bit, and just check STRING_ECH_CONFIG

  • Turn grease/enable/hard into three different numerical values, no bitmask
    needed

  • Convert the struct field 'tls_ech' from an int to a byte.

@github-actions github-actions Bot added the tests label May 7, 2026
@bagder bagder requested a review from Copilot May 7, 2026 21:55
@bagder bagder marked this pull request as ready for review May 7, 2026 21:56

Copilot AI left a comment

Copy link
Copy Markdown

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 tightens CURLOPT_ECH option parsing in curl_easy_setopt() so applications can reliably detect typos/unknown strings by receiving an error return, rather than silently accepting an unrecognized value.

Changes:

  • Refactors CURLOPT_ECH parsing into a dedicated helper (setopt_ech()) and returns CURLE_BAD_FUNCTION_ARGUMENT for unknown strings.
  • Updates the lib1521 option-error allowlist generator to treat CURLOPT_ECH as an option that may return CURLE_BAD_FUNCTION_ARGUMENT for invalid strings.

Reviewed changes

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

File Description
tests/libtest/mk-lib1521.pl Adds CURLOPT_ECH to the set of string options allowed to return CURLE_BAD_FUNCTION_ARGUMENT for unrecognized values.
lib/setopt.c Introduces setopt_ech() and makes CURLOPT_ECH return an error for unknown strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/setopt.c Outdated
Comment thread lib/setopt.c Outdated
Comment thread lib/setopt.c Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

Comment thread lib/setopt.c Outdated
Comment thread lib/setopt.c
Comment thread lib/setopt.c Outdated
- passing an unknown string to CURLOPT_ECH now returns error

  To properly allow applications to spot if they pass in a typo or
  something to libcurl.

- CURLECH_DISABLE is now a plain zero internally, not a dedicated bit which
  simplifies checks for when ECH is enabled

- Dropped the CURLECH_CLA_CFG bit, and just check STRING_ECH_CONFIG

- Turn grease/enable/hard into three different numerical values, no bitmask
  needed

- Convert the struct field 'tls_ech' from an int to a byte.
@bagder bagder force-pushed the bagder/setopt-ech branch from 6232208 to 3c18ead Compare May 8, 2026 09:17
@bagder bagder changed the title setopt: passing an unknown string to CURLOPT_ECH now returns error ECH: cleanups May 8, 2026
@bagder

bagder commented May 8, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode

augmentcode Bot commented May 8, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR refactors libcurl’s ECH (Encrypted Client Hello) option handling to be stricter on invalid input and simpler internally.

Changes:

  • Centralizes CURLOPT_ECH parsing in a new setopt_ech() helper and returns an error for unknown strings
  • Simplifies internal ECH state by replacing bitmasks with enum-like numeric values (with CURLECH_DISABLE as 0)
  • Removes CURLECH_CLA_CFG and instead uses stored string presence (e.g. STRING_ECH_CONFIG) to infer CLI config
  • Converts tls_ech storage from int to uint8_t and updates OpenSSL/rustls/wolfSSL backends accordingly
  • Updates the lib1521 generator allowlist so CURLOPT_ECH may return CURLE_BAD_FUNCTION_ARGUMENT for invalid strings

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/setopt.c
Comment thread lib/vtls/vtls.h
@bagder bagder closed this in b174b8b May 8, 2026
@bagder bagder deleted the bagder/setopt-ech branch May 8, 2026 14:44
outcast36 pushed a commit to greearb/curl that referenced this pull request Jun 3, 2026
- passing an unknown string to CURLOPT_ECH now returns error

  To properly allow applications to spot if they pass in a typo or
  something to libcurl.

- CURLECH_DISABLE is now a plain zero internally, not a dedicated bit which
  simplifies checks for when ECH is enabled

- Dropped the CURLECH_CLA_CFG bit, and just check STRING_ECH_CONFIG

- Turn grease/enable/hard into three different numerical values, no bitmask
  needed

- Convert the struct field 'tls_ech' from an int to a byte.

Closes curl#21532
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants