Skip to content

feat: add opt-in AIA certificate chain fetching (tlsAIAFetch) - #6137

Merged
joanlopez merged 32 commits into
masterfrom
vt/tls_fix_v2
Sep 8, 2026
Merged

joanlopez merged 32 commits into
masterfrom
vt/tls_fix_v2

Conversation

@vtorosyan

@vtorosyan vtorosyan commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Original PR and review comments are here: #5942

What?

Go's crypto/tls does not fetch missing intermediate certificates via the Authority Information Access (AIA) extension; browsers do. Servers that send only a leaf certificate and rely on AIA to fill in the chain cause k6 TLS handshakes to fail with "x509: certificate signed by unknown authority" even when the chain is technically valid.

This commit adds a new tlsAIAFetch option (env: K6_TLS_AIA_FETCH) that enables AIA chain fetching on a per-test basis. When enabled, k6 intercepts failed chain verification, follows IssuingCertificateURL fields (BFS, up to 5 hops), retries verification with any fetched intermediates, and then restores hostname verification — exactly what browsers do.

Implementation details:

  • lib/netext/aia.go: core WrapTLSConfigForAIAFetching function; uses a separate http.Client (not measured as test traffic) with a 10s timeout; process-level sync.Map cache so repeated handshakes pay the HTTP cost at most once per AIA URL; circular-reference protection via seen map
  • lib/options.go: new TLSAIAFetch null.Bool option (default off)
  • internal/js/runner.go: opt-in wiring — wraps the per-VU tls.Config when tlsAIAFetch: true; no-op when insecureSkipTLSVerify is also true
  • internal/js/modules/k6/grpc/client.go: fix buildTLSConfig to copy VerifyPeerCertificate and VerifyConnection callbacks from the parent config, so AIA verification is not silently bypassed for gRPC connections that supply explicit TLS options (cacerts/cert/key)

Tests:

  • lib/netext/aia_test.go: 10 parallel unit tests covering happy path, complete chain passthrough, disabled/plain-Go regression, InsecureSkip- Verify passthrough, unreachable URL, fetch timeout, HTTP error, malformed cert, circular AIA references, and hostname-mismatch enforcement
  • internal/js/runner_aia_test.go: full VU integration test (Source and Archive runners) verifying the enabled/disabled toggle end-to-end

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

Fixes #5941

vtorosyan and others added 5 commits July 9, 2026 18:21
Go's crypto/tls does not fetch missing intermediate certificates via the
Authority Information Access (AIA) extension; browsers do. Servers that
send only a leaf certificate and rely on AIA to fill in the chain cause
k6 TLS handshakes to fail with "x509: certificate signed by unknown
authority" even when the chain is technically valid.

This commit adds a new `tlsAIAFetch` option (env: K6_TLS_AIA_FETCH) that
enables AIA chain fetching on a per-test basis. When enabled, k6 intercepts
failed chain verification, follows IssuingCertificateURL fields (BFS, up to
5 hops), retries verification with any fetched intermediates, and then
restores hostname verification — exactly what browsers do.

Implementation details:
- lib/netext/aia.go: core WrapTLSConfigForAIAFetching function; uses a
  separate http.Client (not measured as test traffic) with a 10s timeout;
  process-level sync.Map cache so repeated handshakes pay the HTTP cost
  at most once per AIA URL; circular-reference protection via seen map
- lib/options.go: new TLSAIAFetch null.Bool option (default off)
- internal/js/runner.go: opt-in wiring — wraps the per-VU tls.Config
  when tlsAIAFetch: true; no-op when insecureSkipTLSVerify is also true
- internal/js/modules/k6/grpc/client.go: fix buildTLSConfig to copy
  VerifyPeerCertificate and VerifyConnection callbacks from the parent
  config, so AIA verification is not silently bypassed for gRPC
  connections that supply explicit TLS options (cacerts/cert/key)

Tests:
- lib/netext/aia_test.go: 10 parallel unit tests covering happy path,
  complete chain passthrough, disabled/plain-Go regression, InsecureSkip-
  Verify passthrough, unreachable URL, fetch timeout, HTTP error, malformed
  cert, circular AIA references, and hostname-mismatch enforcement
- internal/js/runner_aia_test.go: full VU integration test (Source and
  Archive runners) verifying the enabled/disabled toggle end-to-end

Fixes #5941

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- aia.go: split var block so gochecknoglobals nolint directives are
  per-variable (golangci-lint v2 requires inline directives); extract
  buildVerifyPeerFn, verifyWithAIA, and buildVerifyConnFn to bring
  WrapTLSConfigForAIAFetching within the gocognit limit; fix makezero
  by using append instead of make+copy for the BFS queue; wrap the
  fetchAIAIntermediates signature to stay within the lll limit; remove
  unused //nolint:gosec on direct assignment (G402 only fires on struct
  literals); add //nolint:gosec to httpClient.Do for G704 (SSRF);
  add //nolint:contextcheck to runner.go call site (AIA fetches
  intentionally use context.Background inside the TLS callback)
- aia_test.go: use closeBody helper for error-path tests (bodyclose);
  _, _ = fmt.Fprintln for errcheck; change nullLogger return type from
  *logrus.Logger to logrus.FieldLogger (forbidigo); add //nolint:noctx
  to net.Listen call; remove nolint directives that are unused because
  gosec/forcetypeassert are excluded for test files
- runner_aia_test.go: remove unused //nolint:forcetypeassert
- kill_other.go: remove obsolete // +build constraint (modernize)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without this, tlsAIAFetch set in a script's options block would be
silently dropped when k6 merges option sources (script + CLI flags +
env vars), causing AIA fetching to never activate despite being
configured.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 9, 2026 16:30 — with GitHub Actions Inactive
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 9, 2026 16:33 — with GitHub Actions Inactive
@vtorosyan

Copy link
Copy Markdown
Contributor Author

I ran again the end-to-end test chain in https://github.com/grafana/sm-k6-tls-test to ensure nothing changed against the previous PR, results are clean.

⏺ Full results against vt/tls_fix_v2:

  ┌────────────────────────────────────────────────────────┬─────────────────────────┐
  │                        Scenario                        │         Result          │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ aia-chain (server sends only leaf, requires AIA fetch) │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ complete-chain (full chain, AIA not needed)            │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ self-signed (should fail without skip)                 │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ self-signed/skip (with insecureSkipTLSVerify)          │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ insecure-skip                                          │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ wrong-hostname (should fail without skip)              │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ wrong-hostname/skip                                    │ PASS                    │
  ├────────────────────────────────────────────────────────┼─────────────────────────┤
  │ badssl-aia (real-world, incomplete-chain.badssl.com)   │ PASS (from earlier run) │

@vtorosyan
vtorosyan marked this pull request as ready for review July 10, 2026 11:42
@vtorosyan
vtorosyan requested a review from a team as a code owner July 10, 2026 11:42
@vtorosyan
vtorosyan requested review from joanlopez and szkiba and removed request for a team July 10, 2026 11:42
@vtorosyan
vtorosyan requested a review from mstoykov July 10, 2026 11:43
@szkiba

szkiba commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

@vtorosyan for me it seems ok, but to be sure I asked claude for review. Here is the output:

Blocker — gRPC ignores user-supplied CA when tlsAIAFetch is enabled

File: internal/js/modules/k6/grpc/client.go:172

buildTLSConfig now copies VerifyPeerCertificate / VerifyConnection from the
parent (VU) config:

tlsCfg := &tls.Config{
    ...
    RootCAs:               cp, // user-supplied cacerts
    VerifyPeerCertificate: parentConfig.VerifyPeerCertificate,
    VerifyConnection:      parentConfig.VerifyConnection,
}

When tlsAIAFetch: true, the parent config is the AIA-wrapped config, so its
VerifyPeerCertificate is the closure built in lib/netext/aia.go. That closure
reads its captured config's RootCAs, not the gRPC config's:

// lib/netext/aia.go:126
roots := cfg.RootCAs   // cfg = the wrapped VU config, NOT grpc's tlsCfg

So the cp pool that gRPC builds from the user's cacerts
(grpc/client.go:154-159) is never consulted during verification. Chain
verification runs against the wrapped VU config's RootCAs (typically nil → system
pool).

Failure scenario: A gRPC test against a server whose chain roots in a private
CA, supplied via cacerts, with options.tlsAIAFetch = true. Verification uses
the system pool instead of the provided CA and the handshake fails with
x509: certificate signed by unknown authority — even though the correct CA was
given.

Why it matters: silent, and it makes two supported features mutually
incompatible. A user enabling AIA globally silently breaks every gRPC call that
relies on a custom CA.

Suggested fix: the AIA callback must verify against the config it is attached
to, not a config captured at wrap time. Options:

  • Re-wrap the gRPC tlsCfg with WrapTLSConfigForAIAFetching after setting
    RootCAs: cp (and don't copy the parent callbacks), so the closure captures
    the gRPC config. Cleanest.
  • Or don't copy VerifyPeerCertificate/VerifyConnection in buildTLSConfig
    and re-apply AIA wrapping explicitly when the parent had it enabled.

Add a gRPC-path test covering cacerts + tlsAIAFetch: true.


Should fix — PKCS#7 (.p7c) AIA responses are silently rejected

File: lib/netext/aia.go:287

fetchCertFromAIAURL only handles raw DER and PEM:

if cert, parseErr := x509.ParseCertificate(body); parseErr == nil {
    return cert, nil
}
block, _ := pem.Decode(body)
if block == nil {
    return nil, errors.New("AIA response is neither valid DER nor PEM")
}

Many public CAs serve the issuing certificate as PKCS#7 / CMS
(application/pkcs7-mime, .p7c). Those responses parse as neither DER nor PEM,
so the intermediate is dropped and the chain stays incomplete — the exact case
this feature exists to fix.

Failure scenario: Server's IssuingCertificateURL points to a .p7c file.
Fetch succeeds (HTTP 200), parse fails, intermediate silently skipped, handshake
still fails with unknown authority. The user sees no clear reason why AIA
"didn't work".

Suggested fix: add a PKCS#7 fallback after the DER/PEM attempts (extract the
certificate(s) from the CMS bundle and add them). If PKCS#7 support is
intentionally out of scope, document the limitation prominently and log at Warn
when a response looks like PKCS#7 so users can diagnose it.

When the VU tls.Config is AIA-wrapped and a grpc.connect() call supplies
its own cacerts, buildTLSConfig previously copied the parent's
VerifyPeerCertificate/VerifyConnection callbacks. Those callbacks
captured the VU config's RootCAs, so the gRPC-supplied cp was silently
ignored and the handshake failed with "x509: certificate signed by
unknown authority" even when the correct CA was provided.

Instead, re-wrap the gRPC-local tls.Config with WrapTLSConfigForAIAFetching
when AIA is enabled at the VU level. The new closure captures the gRPC
config, so its RootCAs (cp) is used for chain verification.

Adds a targeted test that reproduces the failure and verifies the fix.
Some CAs (notably Sectigo and others) serve intermediate certificates
as PKCS#7/CMS bundles rather than raw DER or PEM. k6 does not currently
parse that format, so the fetched intermediate is silently dropped and
the chain remains incomplete — the user sees an "unknown authority"
handshake failure with no clue that PKCS#7 was the cause.

Detect PKCS#7 responses via the Content-Type header and by scanning
for the ASN.1 signedData OID, and emit a Warn log pointing at the URL
so users can diagnose the situation. Full PKCS#7 parser support is
tracked as a follow-up.
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 14, 2026 15:26 — with GitHub Actions Inactive
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 14, 2026 15:28 — with GitHub Actions Inactive
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 14, 2026 15:36 — with GitHub Actions Inactive
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing July 14, 2026 15:39 — with GitHub Actions Inactive
@vtorosyan

Copy link
Copy Markdown
Contributor Author

@szkiba thank you for the feedback! I have pushed some changes:

  • buildTLSConfig no longer copies the parent's VerifyPeerCertificate/VerifyConnection; instead, if AIA is enabled at the VU level, we re-wrap the gRPC-local tls.Config so the closure captures gRPC's RootCAs (cp). Added a targeted test that reproduces the failure and now verifies the fix.

  • PKCS#7: added a Warn log that detects PKCS#7 responses via Content-Type or the signedData OID, so users can diagnose "AIA didn't work" for those chains. Full parser support tracked as follow-up here AIA: add PKCS#7 parser support for intermediate fetching #6146

Regression check: I also ran the existing TLS scenarios in my sm-k6-tls-test suite against the fixed binary - all still pass, confirming the HTTP AIA path is not regressed.

szkiba
szkiba previously approved these changes Jul 21, 2026

@szkiba szkiba left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread internal/js/modules/k6/grpc/client.go Outdated
Comment thread internal/js/runner.go Outdated
Comment thread lib/netext/aia.go Outdated
Comment thread internal/js/modules/k6/grpc/client.go Outdated
}
// Re-wrap so the verification closure reads this config's RootCAs, not the VU's.
if aiaEnabled {
tlsCfg = netext.WrapTLSConfigForAIAFetching(tlsCfg, logger, nil)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here if the AIA is enabled we would've already wrtapped it and set the wrapper and InsecureSkipVeriryf.

Above we do copy the latter but not the former but this wrapper funciton checks the latter and doesn't do the wrapping. So this makes gRPC completely insecure.

Please do add a test that checks taht if gRPC certificates do not match and the AIA is enabled it will fail

@joanlopez joanlopez Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I concur, it's what I tried to explain here: https://github.com/grafana/k6/pull/6137/changes#r3621521110

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks both, really good catch!

buildTLSConfig now takes insecureSkipVerify from state.Options.InsecureSkipTLSVerify.Bool (the user's actual intent) instead of parentConfig.InsecureSkipVerify (which the AIA wrapper had flipped to true internally, causing the re-wrap to short-circuit and leave verification off). See 9c0391b

Also added TestBuildTLSConfig_AIARejectsMismatchedCACert covering the scenario you described. I made sure to run the test before and it failed against the pre-fix code and passes now.

Comment thread lib/netext/aia.go Outdated
Comment thread lib/netext/aia.go Outdated
Comment thread lib/netext/aia.go Outdated
Comment thread lib/netext/aia.go Outdated
Comment thread lib/netext/aia_test.go Outdated
Comment thread internal/js/modules/k6/grpc/client.go Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that, when aiaEnabled is on, the VU's tls.Config reaches here with InsecureSkipVerify already set to true by the wrapper, so buildTLSConfig copies that into the fresh config, and the re-wrap sees it and early-returns.

Can you walk through what VerifyPeerCertificate ends up being on the returned config for a gRPC connection that passes cacerts, please?

Comment thread internal/js/runner.go
Comment on lines +588 to +597
// Rebuild the AIA fetcher against the new options — Blacklist / BlockedHostnames /
// Hosts / Resolver may have changed and the fetcher's dialer captures them.
runnerDialer := &netext.Dialer{
Dialer: r.BaseDialer,
Resolver: r.Resolver,
Blacklist: opts.BlacklistIPs,
BlockedHostnames: opts.BlockedHostnames.Trie,
Hosts: opts.Hosts.Trie,
}
r.aiaFetcher = netext.NewAIAFetcher(runnerDialer.DialContext)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess it's not critical, but this allocation could be gated behind:

if tlsAIAFetch := opts.TLSAIAFetch; tlsAIAFetch.Valid && tlsAIAFetch.Bool {
  ...
}

joanlopez
joanlopez previously approved these changes Aug 13, 2026

@joanlopez joanlopez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM overall, one thing I want to double check though.

In buildVerifyConnFn, we skip verification when cs.ServerName == "", but looks like Go strips that to "" for IP-literal server names, as SNI can't carry IPs.

So for any bare-IP target, this just skips verification entirely.

Go doesn't seem to do that though, it verifies against the raw config.ServerName, and VerifyHostname does check IP literals against the cert's IP SANs, so it's a real check we'd be bypassing here.

Is skipping verification for IP targets on purpose, or should we be checking against the raw ServerName instead of cs.ServerName?
cc/ @vtorosyan @mstoykov

@oleiade

oleiade commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

triage: waiting on @vtorosyan ideas on @joanlopez 's comment above before we move on with merging 🙇🏻

@vtorosyan

Copy link
Copy Markdown
Contributor Author

@joanlopez nice catch - I spent some time trying to assess the impact and the best way to move forward. So indeed, returning simply nil is passing the verification which translates to any valid CA-signed cert passing. This is actually a security hole, so we can't leave it like that.

I have tried to properly fix it but it is getting messy... so I am proposing instead just to fail-close for empty ServerName + AIA. In practice the impact is that a user who wants tlsAIAFetch + a bare-IP target will get an error. So they have to either use a hostname, or disable tlsAIAFetch, or set insecureSkipTLSVerify.

I think the impact of this is going to be small, Public CAs generally don't issue certs for IP addresses afaik. So this should be more of a corner case that affects a narrow slice of users.

If you give me your +1 / -1, I will:

  1. Commit a change to fail-close
  2. Update docs to mention that bare-IP is not supported.

@szkiba
szkiba self-requested a review August 25, 2026 07:49
@joanlopez

Copy link
Copy Markdown
Contributor

@vtorosyan I'm happy with your trade-off, I think it's a good balance 👍🏻 As you said, this will most likely only affect a narrow slice of users, and if that's not the case, we will hear about that, and we can always rework it.

Happy to go with this, let me know once you're done with the actions you mentioned. Thanks! 🙇🏻 🙌🏻

szkiba
szkiba previously approved these changes Aug 27, 2026
For IP-literal targets Go strips the ServerName from ConnectionState (SNI
can't carry IPs), so the AIA verify callback previously returned nil,
accepting any valid CA-signed chain. Return an error instead so the
handshake fails; users hitting bare-IP targets must disable tlsAIAFetch
or set insecureSkipTLSVerify. Test suite updates dial via "localhost" so
existing paths keep exercising real hostname verification, plus a new
test covers the IP-literal rejection path.
@vtorosyan
vtorosyan dismissed stale reviews from szkiba, joanlopez, and mstoykov via a6dfed7 August 27, 2026 15:51
@vtorosyan

Copy link
Copy Markdown
Contributor Author

@joanlopez a6dfed7 - thank you!

The fail-close change in the AIA verify callback now rejects IP-literal
targets. TestVUIntegrationAIAFetching dialed 127.0.0.1 with a matching
IP SAN, which used to work but now hits the closed path. Switch the
leaf's SAN and the dial address to "localhost" so SNI carries a
ServerName the wrapper can verify.
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing August 27, 2026 16:10 — with GitHub Actions Inactive
@vtorosyan
vtorosyan temporarily deployed to azure-trusted-signing August 27, 2026 16:13 — with GitHub Actions Inactive
@mstoykov
mstoykov requested review from joanlopez and szkiba September 8, 2026 09:13
@joanlopez
joanlopez merged commit cdd7355 into master Sep 8, 2026
68 of 69 checks passed
@joanlopez
joanlopez deleted the vt/tls_fix_v2 branch September 8, 2026 09:46
inancgumus added a commit to grafana/k6-DefinitelyTyped that referenced this pull request Sep 10, 2026
Clarify when certificate fetching helps and its hostname requirement.

Related: grafana/k6#6137

This branch was previously deployed

1 inactive deployment
azure-trusted-signing — e5909cd0 Deployed Aug 27, 2026 by vtorosyan via sign-packages #10497
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.

k6 fails TLS verification for certificates requiring AIA chain fetching

5 participants