Skip to content

🐛 fall back to system trust store when no custom TLS CA is set - #331

Merged
maxlerebourg merged 5 commits into
mainfrom
fix/327-tls-system-ca
Jun 23, 2026
Merged

🐛 fall back to system trust store when no custom TLS CA is set#331
maxlerebourg merged 5 commits into
mainfrom
fix/327-tls-system-ca

Conversation

@mathieuHa

Copy link
Copy Markdown
Collaborator

Closes #327.

Problem

Setting crowdsecLapiScheme=https against a LAPI exposed behind a reverse proxy with a publicly trusted certificate (e.g. Let's Encrypt) currently forces the operator to pick one of two suboptimal options:

  • set crowdsecLapiTLSInsecureVerify=true (skip verification — defeats the point), or
  • pin crowdsecLapiTLSCertificateAuthority to a CA bundle (extra config nobody should need for a public CA).

The expected behavior — "use the system trust store" — wasn't reachable.

Root cause

Two contributing bugs in pkg/configuration/configuration.go:

  1. validateParamsTLS returned an error when CrowdsecLapiTLSCertificateAuthority was empty.
  2. getTLSConfig always initialized tls.Config.RootCAs = x509.NewCertPool() (an empty pool), which silently disables the stdlib's fall-back to x509.SystemCertPool(). So even if validation didn't fire, verification of a Let's Encrypt cert would still fail.

The Go idiom for "use the system trust store" is to leave tls.Config.RootCAs as nil. The traefik image (alpine + ca-certificates) ships a CA bundle that already trusts the public roots.

Fix

  • validateParamsTLS: drop the empty-CA error; still rejects garbage PEM.
  • getTLSConfig: only allocate RootCAs when a custom CA is provided; otherwise leave it nil.
  • Same change applies symmetrically to the AppSec path (helper is shared).

Tests

Test_validateParamsTLS and Test_GetTLSConfigCrowdsec were scaffolds (// TODO: Add test cases.) — now filled in.

Test_validateParamsTLS covers:

  • Empty CA → accepted
  • Valid PEM → accepted
  • Garbage CA → rejected

Test_GetTLSConfigCrowdsec covers:

  • HTTP scheme → empty tls.Config
  • HTTPS + no CA → RootCAs == nil (system trust store)
  • HTTPS + custom CA → RootCAs populated
  • HTTPS + insecure verify → InsecureSkipVerify == true
  • HTTPS + garbage CA → error

Docs

README.md — replaced the "you need to either accept any certificates or add the CA" wording with three explicit options (system trust store / custom CA / skip verification), for both the LAPI and the AppSec sections. Per-field descriptions of Crowdsec{Lapi,Appsec}TlsCertificateAuthority now mention the system trust store as the default fallback.

Compatibility

No behavior change for existing setups: configs that explicitly set a custom CA or insecure verify continue to work identically. The new behavior is only reachable by omitting both — which was rejected before.

@mathieuHa
mathieuHa requested a review from maxlerebourg May 30, 2026 17:05
Comment thread pkg/configuration/configuration_test.go Outdated
Comment thread pkg/configuration/configuration_test.go Outdated
@mathieuHa
mathieuHa force-pushed the fix/327-tls-system-ca branch from 993a415 to a2a21ae Compare June 6, 2026 10:04
mathieuHa added a commit that referenced this pull request Jun 6, 2026
Address review on #331:
- the self-signed validPEM block was duplicated in two test funcs; declare it
  once at package level and drop both local copies.
- rename cfgGarbage -> cfgInvalidCA (and its test case) for a descriptive name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
maxlerebourg
maxlerebourg previously approved these changes Jun 14, 2026
mathieuHa and others added 5 commits June 15, 2026 18:05
Closes #327.

Until now, configuring `crowdsecLapiScheme=https` forced the operator
to either provide `crowdsecLapiTLSCertificateAuthority` (a custom CA)
or set `crowdsecLapiTLSInsecureVerify=true` — there was no way to rely
on the host's system trust store, which is the expected setup when the
LAPI sits behind a reverse proxy with a publicly trusted (e.g. Let's
Encrypt) certificate.

Two contributing bugs:
  - `validateParamsTLS` rejected an empty CA up-front.
  - `getTLSConfig` always allocated an empty `tls.Config.RootCAs`,
    which silently disabled the standard library's fall-back to
    `x509.SystemCertPool()`.

Fix: drop the validation error for the empty-CA case and only allocate
`RootCAs` when a custom CA is actually provided. Same change applies
symmetrically to the AppSec path since the helper is shared.

Add unit tests covering the four meaningful states (HTTP, HTTPS with
system CA, HTTPS with custom CA, HTTPS with insecure verify) plus the
malformed-PEM rejection. README updated to document the system trust
store as an explicit option for both LAPI and AppSec HTTPS.
Address review on #331:
- the self-signed validPEM block was duplicated in two test funcs; declare it
  once at package level and drop both local copies.
- rename cfgGarbage -> cfgInvalidCA (and its test case) for a descriptive name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tore)

CI regression coverage for this PR: with no custom CA configured, the bouncer
must fall back to the OS/system trust store for an HTTPS LAPI.

In the binary suite the "system trust store" is whatever Go's
x509.SystemCertPool() reads, which honours SSL_CERT_FILE on the Traefik process.
The scenario mints a throwaway CA, serves the mock LAPI over HTTPS with a cert
signed by it, and runs the stack twice:
  - positive: SSL_CERT_FILE = our CA       -> LAPI trusted    -> 200
  - negative: SSL_CERT_FILE = empty bundle  -> not trusted     -> 403 (fail-closed)
The negative run proves the patch still VERIFIES (not an insecure skip).

- mocklapi: optional --lapi-tls-cert/--lapi-tls-key to serve the LAPI over TLS.
- common.sh: opt-in LAPI_TLS_CERT/KEY (HTTPS mock) and TRAEFIK_SSL_CERT_FILE
  (inject SSL_CERT_FILE into Traefik); both default-empty, other scenarios
  unaffected.
- adds openssl as a scenario-only dependency.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@maxlerebourg
maxlerebourg merged commit f4dcd93 into main Jun 23, 2026
5 checks passed
@maxlerebourg
maxlerebourg deleted the fix/327-tls-system-ca branch July 5, 2026 17:39
renovate Bot added a commit to sdwilsh/ansible-playbooks that referenced this pull request Jul 29, 2026
…plugin to v1.7.0

##### [\`v1.7.0\`](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/releases/tag/v1.7.0)

#### Deprecated

`BanHTMLFilePath` become `BanFilePath`
`CaptchaHTMLFilePath` become `CaptchaFilePath`

#### What's Changed

- 🍱 Header.Add -> Set, log Info -> Debug, version bump by [@maxlerebourg](https://github.com/maxlerebourg) in [#324](maxlerebourg/crowdsec-bouncer-traefik-plugin#324)
- ✨️ Do not block middleware startup due to stream intialization by [@maxlerebourg](https://github.com/maxlerebourg) in [#281](maxlerebourg/crowdsec-bouncer-traefik-plugin#281)
- 📝 docs: fix Mermaid diagrams not rendering (lowercase destroy keyword) by [@mathieuHa](https://github.com/mathieuHa) in [#335](maxlerebourg/crowdsec-bouncer-traefik-plugin#335)
- ✨ tests: CI end-to-end suite — Traefik binary + mock LAPI by [@mathieuHa](https://github.com/mathieuHa) in [#329](maxlerebourg/crowdsec-bouncer-traefik-plugin#329)
- ⬆️ Bump actions/upload-artifact from 4 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#336](maxlerebourg/crowdsec-bouncer-traefik-plugin#336)
- ⬆️ Bump actions/checkout from 6 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#341](maxlerebourg/crowdsec-bouncer-traefik-plugin#341)
- 🐛 fall back to system trust store when no custom TLS CA is set by [@mathieuHa](https://github.com/mathieuHa) in [#331](maxlerebourg/crowdsec-bouncer-traefik-plugin#331)
- Add parameter to configure the ban page Content-Type response header by [@highpingblorg](https://github.com/highpingblorg) in [#325](maxlerebourg/crowdsec-bouncer-traefik-plugin#325)
- ⬆️ Bump actions/cache from 5 to 6 by [@dependabot](https://github.com/dependabot)\[bot] in [#344](maxlerebourg/crowdsec-bouncer-traefik-plugin#344)
- Consider 502, 503 and 504 as unavaible for appsec by [@dani](https://github.com/dani) in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- 🐛 appsec: do not buffer unreadable (gRPC/HTTP2) request bodies by [@mathieuHa](https://github.com/mathieuHa) in [#332](maxlerebourg/crowdsec-bouncer-traefik-plugin#332)
- 🤖 chore: adopt Renovate for automated dependency updates by [@mathieuHa](https://github.com/mathieuHa) in [#330](maxlerebourg/crowdsec-bouncer-traefik-plugin#330)
- ⬆️ renovate: Update all (major) by [@maxlerebourg](https://github.com/maxlerebourg) in [#345](maxlerebourg/crowdsec-bouncer-traefik-plugin#345)
- 🐛 ci(renovate): fix OOM crash and stop using Mend's default gitAuthor by [@mathieuHa](https://github.com/mathieuHa) in [#347](maxlerebourg/crowdsec-bouncer-traefik-plugin#347)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#349](maxlerebourg/crowdsec-bouncer-traefik-plugin#349)
- Do not consider body unreadable when it's http.NoBody by [@dani](https://github.com/dani) in [#352](maxlerebourg/crowdsec-bouncer-traefik-plugin#352)
- feat: Allow cache reading from replicas by [@samtoxie](https://github.com/samtoxie) in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)
- ✨ Renovate update version.go by [@maxlerebourg](https://github.com/maxlerebourg) in [#360](maxlerebourg/crowdsec-bouncer-traefik-plugin#360)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#350](maxlerebourg/crowdsec-bouncer-traefik-plugin#350)

#### New Contributors

- [@dani](https://github.com/dani) made their first contribution in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- [@samtoxie](https://github.com/samtoxie) made their first contribution in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)

**Full Changelog**: <maxlerebourg/crowdsec-bouncer-traefik-plugin@v1.6.0...v1.7.0>
renovate Bot added a commit to sdwilsh/ansible-playbooks that referenced this pull request Jul 29, 2026
…plugin to v1.7.0

##### [\`v1.7.0\`](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/releases/tag/v1.7.0)

#### Deprecated

`BanHTMLFilePath` become `BanFilePath`
`CaptchaHTMLFilePath` become `CaptchaFilePath`

#### What's Changed

- 🍱 Header.Add -> Set, log Info -> Debug, version bump by [@maxlerebourg](https://github.com/maxlerebourg) in [#324](maxlerebourg/crowdsec-bouncer-traefik-plugin#324)
- ✨️ Do not block middleware startup due to stream intialization by [@maxlerebourg](https://github.com/maxlerebourg) in [#281](maxlerebourg/crowdsec-bouncer-traefik-plugin#281)
- 📝 docs: fix Mermaid diagrams not rendering (lowercase destroy keyword) by [@mathieuHa](https://github.com/mathieuHa) in [#335](maxlerebourg/crowdsec-bouncer-traefik-plugin#335)
- ✨ tests: CI end-to-end suite — Traefik binary + mock LAPI by [@mathieuHa](https://github.com/mathieuHa) in [#329](maxlerebourg/crowdsec-bouncer-traefik-plugin#329)
- ⬆️ Bump actions/upload-artifact from 4 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#336](maxlerebourg/crowdsec-bouncer-traefik-plugin#336)
- ⬆️ Bump actions/checkout from 6 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#341](maxlerebourg/crowdsec-bouncer-traefik-plugin#341)
- 🐛 fall back to system trust store when no custom TLS CA is set by [@mathieuHa](https://github.com/mathieuHa) in [#331](maxlerebourg/crowdsec-bouncer-traefik-plugin#331)
- Add parameter to configure the ban page Content-Type response header by [@highpingblorg](https://github.com/highpingblorg) in [#325](maxlerebourg/crowdsec-bouncer-traefik-plugin#325)
- ⬆️ Bump actions/cache from 5 to 6 by [@dependabot](https://github.com/dependabot)\[bot] in [#344](maxlerebourg/crowdsec-bouncer-traefik-plugin#344)
- Consider 502, 503 and 504 as unavaible for appsec by [@dani](https://github.com/dani) in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- 🐛 appsec: do not buffer unreadable (gRPC/HTTP2) request bodies by [@mathieuHa](https://github.com/mathieuHa) in [#332](maxlerebourg/crowdsec-bouncer-traefik-plugin#332)
- 🤖 chore: adopt Renovate for automated dependency updates by [@mathieuHa](https://github.com/mathieuHa) in [#330](maxlerebourg/crowdsec-bouncer-traefik-plugin#330)
- ⬆️ renovate: Update all (major) by [@maxlerebourg](https://github.com/maxlerebourg) in [#345](maxlerebourg/crowdsec-bouncer-traefik-plugin#345)
- 🐛 ci(renovate): fix OOM crash and stop using Mend's default gitAuthor by [@mathieuHa](https://github.com/mathieuHa) in [#347](maxlerebourg/crowdsec-bouncer-traefik-plugin#347)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#349](maxlerebourg/crowdsec-bouncer-traefik-plugin#349)
- Do not consider body unreadable when it's http.NoBody by [@dani](https://github.com/dani) in [#352](maxlerebourg/crowdsec-bouncer-traefik-plugin#352)
- feat: Allow cache reading from replicas by [@samtoxie](https://github.com/samtoxie) in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)
- ✨ Renovate update version.go by [@maxlerebourg](https://github.com/maxlerebourg) in [#360](maxlerebourg/crowdsec-bouncer-traefik-plugin#360)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#350](maxlerebourg/crowdsec-bouncer-traefik-plugin#350)

#### New Contributors

- [@dani](https://github.com/dani) made their first contribution in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- [@samtoxie](https://github.com/samtoxie) made their first contribution in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)

**Full Changelog**: <maxlerebourg/crowdsec-bouncer-traefik-plugin@v1.6.0...v1.7.0>
renovate Bot added a commit to sdwilsh/ansible-playbooks that referenced this pull request Jul 29, 2026
…plugin to v1.7.0

##### [\`v1.7.0\`](https://github.com/maxlerebourg/crowdsec-bouncer-traefik-plugin/releases/tag/v1.7.0)

#### Deprecated

`BanHTMLFilePath` become `BanFilePath`
`CaptchaHTMLFilePath` become `CaptchaFilePath`

#### What's Changed

- 🍱 Header.Add -> Set, log Info -> Debug, version bump by [@maxlerebourg](https://github.com/maxlerebourg) in [#324](maxlerebourg/crowdsec-bouncer-traefik-plugin#324)
- ✨️ Do not block middleware startup due to stream intialization by [@maxlerebourg](https://github.com/maxlerebourg) in [#281](maxlerebourg/crowdsec-bouncer-traefik-plugin#281)
- 📝 docs: fix Mermaid diagrams not rendering (lowercase destroy keyword) by [@mathieuHa](https://github.com/mathieuHa) in [#335](maxlerebourg/crowdsec-bouncer-traefik-plugin#335)
- ✨ tests: CI end-to-end suite — Traefik binary + mock LAPI by [@mathieuHa](https://github.com/mathieuHa) in [#329](maxlerebourg/crowdsec-bouncer-traefik-plugin#329)
- ⬆️ Bump actions/upload-artifact from 4 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#336](maxlerebourg/crowdsec-bouncer-traefik-plugin#336)
- ⬆️ Bump actions/checkout from 6 to 7 by [@dependabot](https://github.com/dependabot)\[bot] in [#341](maxlerebourg/crowdsec-bouncer-traefik-plugin#341)
- 🐛 fall back to system trust store when no custom TLS CA is set by [@mathieuHa](https://github.com/mathieuHa) in [#331](maxlerebourg/crowdsec-bouncer-traefik-plugin#331)
- Add parameter to configure the ban page Content-Type response header by [@highpingblorg](https://github.com/highpingblorg) in [#325](maxlerebourg/crowdsec-bouncer-traefik-plugin#325)
- ⬆️ Bump actions/cache from 5 to 6 by [@dependabot](https://github.com/dependabot)\[bot] in [#344](maxlerebourg/crowdsec-bouncer-traefik-plugin#344)
- Consider 502, 503 and 504 as unavaible for appsec by [@dani](https://github.com/dani) in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- 🐛 appsec: do not buffer unreadable (gRPC/HTTP2) request bodies by [@mathieuHa](https://github.com/mathieuHa) in [#332](maxlerebourg/crowdsec-bouncer-traefik-plugin#332)
- 🤖 chore: adopt Renovate for automated dependency updates by [@mathieuHa](https://github.com/mathieuHa) in [#330](maxlerebourg/crowdsec-bouncer-traefik-plugin#330)
- ⬆️ renovate: Update all (major) by [@maxlerebourg](https://github.com/maxlerebourg) in [#345](maxlerebourg/crowdsec-bouncer-traefik-plugin#345)
- 🐛 ci(renovate): fix OOM crash and stop using Mend's default gitAuthor by [@mathieuHa](https://github.com/mathieuHa) in [#347](maxlerebourg/crowdsec-bouncer-traefik-plugin#347)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#349](maxlerebourg/crowdsec-bouncer-traefik-plugin#349)
- Do not consider body unreadable when it's http.NoBody by [@dani](https://github.com/dani) in [#352](maxlerebourg/crowdsec-bouncer-traefik-plugin#352)
- feat: Allow cache reading from replicas by [@samtoxie](https://github.com/samtoxie) in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)
- ✨ Renovate update version.go by [@maxlerebourg](https://github.com/maxlerebourg) in [#360](maxlerebourg/crowdsec-bouncer-traefik-plugin#360)
- ⬆️ renovate: Update all by [@maxlerebourg](https://github.com/maxlerebourg) in [#350](maxlerebourg/crowdsec-bouncer-traefik-plugin#350)

#### New Contributors

- [@dani](https://github.com/dani) made their first contribution in [#338](maxlerebourg/crowdsec-bouncer-traefik-plugin#338)
- [@samtoxie](https://github.com/samtoxie) made their first contribution in [#342](maxlerebourg/crowdsec-bouncer-traefik-plugin#342)

**Full Changelog**: <maxlerebourg/crowdsec-bouncer-traefik-plugin@v1.6.0...v1.7.0>
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.

[FEATURE] Support CrowdsecLapiHost with a valid certificate

2 participants