Skip to content

fix(server): use tlsConfig nil-check to decide between Serve and ServeTLS#420

Open
mesaglio wants to merge 1 commit intocesanta:mainfrom
mesaglio:fix/letsencrypt
Open

fix(server): use tlsConfig nil-check to decide between Serve and ServeTLS#420
mesaglio wants to merge 1 commit intocesanta:mainfrom
mesaglio:fix/letsencrypt

Conversation

@mesaglio
Copy link

When LetsEncrypt was configured, autocert.Manager set tlsConfig.GetCertificate but left CertFile and KeyFile empty. The previous condition checked for empty cert/key strings, which caused the server to call hs.Serve() (plain HTTP) instead of hs.ServeTLS(), silently ignoring the TLS configuration entirely.

How to reproduce locally

  1. Generate a token signing certificate (required by the auth server; separate from the server TLS cert):
mkdir -p /tmp/docker-auth-le-test/sslcache
openssl req -x509 -newkey rsa:2048 \
  -keyout /tmp/docker-auth-le-test/token.key \
  -out /tmp/docker-auth-le-test/token.crt \
  -days 1 -nodes -subj '/CN=localhost'
  1. Create the config at /tmp/docker-auth-le-test/config-letsencrypt.yml:
server:
  addr: ":5001"
  letsencrypt:
    email: test@example.com
    cache_dir: /tmp/docker-auth-le-test/sslcache

token:
  issuer: "Test auth server"
  expiration: 900
  certificate: /tmp/docker-auth-le-test/token.crt
  key: /tmp/docker-auth-le-test/token.key

users:
  "admin":
    password: "$2y$05$LO.vzwpWC5LZGqThvEfznu8qhb5SGqvBSWY1J3yZ4AxtMRZ3kN5jC"  # badmin

acl:
  - match: {account: "admin"}
    actions: ["*"]
  1. Build and run from auth_server:
go build -o auth-server . && ./auth-server /tmp/docker-auth-le-test/config-letsencrypt.yml
  1. Validate with curl:
# Should FAIL (400) if TLS is active — plain HTTP must be rejected
curl -s -o /dev/null -w "HTTP  → %{http_code}\n" http://localhost:5001/auth

# Should reach the TLS stack — autocert rejects 'localhost' as invalid ACME hostname, but the handshake is attempted
curl -sk -o /dev/null -w "HTTPS → %{http_code} (%{errormsg})\n" https://localhost:5001/auth

Expected output (bug present — before fix):
HTTP  → 401     ← server accepted plain HTTP, LetsEncrypt config was silently ignored
HTTPS → 000     ← TLS handshake never happened; server spoke plain HTTP

…eTLS

When LetsEncrypt was configured, autocert.Manager set tlsConfig.GetCertificate
but left CertFile and KeyFile empty. The previous condition checked for empty
cert/key strings, which caused the server to call hs.Serve() (plain HTTP)
instead of hs.ServeTLS(), silently ignoring the TLS configuration entirely.

Replacing the condition with a tlsConfig == nil check ensures that any
configured TLS path — whether via explicit cert files or LetsEncrypt — always
results in ServeTLS being called. ServeTLS with empty certFile/keyFile is valid
in Go and delegates certificate provisioning to GetCertificate.
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