Skip to content

Add HTTPS RR CNAME consistency check to Happy Eyeballs#1

Open
mxinden-bot wants to merge 1 commit into
mainfrom
claude/loving-rubin-GZixw
Open

Add HTTPS RR CNAME consistency check to Happy Eyeballs#1
mxinden-bot wants to merge 1 commit into
mainfrom
claude/loving-rubin-GZixw

Conversation

@mxinden-bot

Copy link
Copy Markdown
Owner

Implements HTTPS Resource Record (RR) CNAME consistency checking in the Happy Eyeballs state machine to prevent connection failures when dual-CDN steering causes mismatches between HTTPS record targets and A/AAAA resolution results.

Summary

When a domain uses dual-CDN steering, the HTTPS record's TargetName may point to one CDN while the A/AAAA CNAME chain steers to another. This can cause ECH handshake failures. This change adds optional filtering to drop HTTPS records whose TargetName is inconsistent with the canonical name from A/AAAA resolution, preferring the origin's plain addresses instead.

Key Changes

  • DnsResult enum: Extended Aaaa and A variants to include an optional canonical name (Option<TargetName>) reported by the resolver
  • Target name matching: Added target_names_match() function for case-insensitive DNS name comparison with trailing dot normalization (RFC 4343)
  • NetworkConfig: Added check_https_rr_cname boolean field to control whether HTTPS records are filtered based on CNAME consistency
  • HTTPS record filtering: Implemented logic to drop HTTPS records whose TargetName doesn't match the canonical name from A/AAAA resolution
  • FFI updates: Updated C++ glue layer to accept and pass through the canonical name from DNS responses and the new configuration flag
  • Test updates: Modified test to enable the new CNAME checking behavior

Implementation Details

  • The canonical name is only consulted for origin queries (not HTTPS record queries)
  • Filtering only applies when the resolver reports a canonical name; if none is available, no filtering occurs
  • DNS name comparison is case-insensitive and handles optional trailing dots to accommodate different resolver implementations
  • Mirrors Firefox's network.dns.https_rr.check_record_with_cname preference behavior

https://claude.ai/code/session_01Quk5LdGKEiv13yyQRrUwWh

ip_preference: IpPreference,
resolution_delay_ms: u32,
connection_attempt_delay_ms: u32,
check_https_rr_cname: bool,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

no need for the option. always enable. see happy-eyeballs review.

Comment on lines +233 to +241
//
// Happy Eyeballs v3 does not fall back to the origin once any ECH-bearing
// record is in play, so this origin-fallback case is pinned to the legacy
// path.
add_task(async function test_https_rr_with_matched_cname_1() {
Services.prefs.setBoolPref("network.http.happy_eyeballs_enabled", false);
registerCleanupFunction(() => {
Services.prefs.clearUserPref("network.http.happy_eyeballs_enabled");
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ah, I thought once a cname mismatch is detected, it does fallback. Please explain.

@mxinden-bot mxinden-bot force-pushed the claude/loving-rubin-GZixw branch 4 times, most recently from 560c6fb to df61a33 Compare June 1, 2026 16:38
Ignore an HTTPS record whose TargetName is inconsistent with the canonical
name produced by the origin's A/AAAA resolution, preferring the origin's
plain A/AAAA addresses (the CNAME). This avoids breaking the ECH handshake
under dual-CDN steering, matching the legacy CheckRecordIsUsableWithCname.

The logic lives in the happy-eyeballs state machine (vendored at v0.7.0),
where the check runs unconditionally. The canonical name rides on
DnsResult::A / DnsResult::Aaaa and is threaded through the FFI from
OnARecord / OnAAAARecord via nsIDNSAddrRecord::GetCanonicalName; the A/AAAA
queries always request RESOLVE_CANONICAL_NAME.

CheckRecordIsUsableWithCname remains as a legacy path used only when
network.http.happy_eyeballs_enabled is false. Re-enable
test_trr_https_rr_with_cname.js for HE3, pinning to the legacy path the
subtests that exercise legacy-only behavior: the check_record_with_cname=false
case (HE3 always runs the check) and the ECH origin-fallback case (HE3 does
not fall back to the origin once an ECH-bearing record is in play).

Differential Revision: https://phabricator.services.mozilla.com/D303605
@mxinden-bot mxinden-bot force-pushed the claude/loving-rubin-GZixw branch from df61a33 to 9fd6d26 Compare June 2, 2026 09:56
mxinden-bot pushed a commit that referenced this pull request Jun 2, 2026
Upstream commit: https://webrtc.googlesource.com/src/+/983b4f025fb568e404eba87500edfd381d241fce
    [M149] Validate CGImage dimensions in MouseCursorMonitorMac

    Original change's description:
    > Validate CGImage dimensions in MouseCursorMonitorMac
    >
    > This adds missing height checks alongside the existing width checks
    > when processing and scaling the cursor image.
    >
    > Fixed: chromium:513268100
    > Change-Id: Ida1e58334a9d3bda429819e4af51b33afbd0a95d
    > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/472700
    > Reviewed-by: Alexander Cooper <alcooper@chromium.org>
    > Auto-Submit: Johannes Kron <kron@webrtc.org>
    > Commit-Queue: Alexander Cooper <alcooper@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#47715}

    (cherry picked from commit b343cd4e280980f26b4feb80684f57d5b4324e12)

    Bug: chromium:514928856,chromium:513268100
    Change-Id: Ida1e58334a9d3bda429819e4af51b33afbd0a95d
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/473981
    Auto-Submit: Chrome Cherry Picker <chrome-cherry-picker@chops-service-accounts.iam.gserviceaccount.com>
    Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
    Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
    Cr-Commit-Position: refs/branch-heads/7827@{#1}
    Cr-Branched-From: d606bc991592fbf4dcbe85e9d05db5e501a5ad42-refs/heads/main@{#47595}
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.

3 participants