fix(security): stabilize kafka secure bootstrap readiness#45
Conversation
| KAFKA_SECURITY_PROTOCOL=PLAINTEXT | ||
| KAFKA_SASL_MECHANISM=SCRAM-SHA-512 | ||
| KAFKA_BROKER_USERNAME=broker-user | ||
| KAFKA_BROKER_PASSWORD= |
There was a problem hiding this comment.
WARNING: Empty password default - ensure users set a secure password for KAFKA_BROKER_PASSWORD before running in secure mode.
| KAFKA_INTER_BROKER_LISTENER_NAME: SASL_SSL | ||
| KAFKA_SASL_ENABLED_MECHANISMS: SCRAM-SHA-512 | ||
| KAFKA_SASL_MECHANISM_INTER_BROKER_PROTOCOL: SCRAM-SHA-512 | ||
| ZOOKEEPER_SASL_ENABLED: "false" |
There was a problem hiding this comment.
WARNING: Empty password default for KAFKA_BROKER_PASSWORD in .env.example - ensure users set a secure password before running in secure mode.
| SECRETS_DIR="$ROOT_DIR/docker/kafka/secrets" | ||
|
|
||
| required_env=( | ||
| KAFKA_BROKER_USERNAME |
There was a problem hiding this comment.
SUGGESTION: Consider adding a check to ensure password variables contain non-empty values. Currently, setting KAFKA_BROKER_PASSWORD="" would pass preflight but fail at runtime.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
The implementation adds broker SCRAM authentication for secure Kafka mode. The changes are well-structured and follow existing patterns. The main concerns are around ensuring users provide non-empty passwords before running in secure mode. Reviewed by minimax-m2.5-20260211 · 447,245 tokens |
There was a problem hiding this comment.
Pull request overview
Stabilizes the Docker-based “Kafka secure mode” bootstrap by adding broker SCRAM credentials/artifacts to preflight, wiring broker JAAS into the secure compose overlay, and improving TLS certificate SAN coverage to prevent hostname-related TLS failures.
Changes:
- Add broker SCRAM env vars and require new broker JAAS/admin-client artifacts during secure preflight.
- Generate broker certs with SANs and emit
kafka_server_jaas.conf+kafka_admin_client.propertiesduring secret generation. - Update secure compose overlay to disable ZooKeeper SASL under broker JAAS, add a Kafka healthcheck, and wire endpoint-identification env vars.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/kafka-secure-preflight.sh |
Extends secure-mode preflight to require broker SCRAM env vars and new broker/admin artifacts. |
scripts/kafka-generate-secrets.sh |
Adds SAN-enabled cert generation and emits broker JAAS + admin client properties. |
scripts/kafka-bootstrap-scram-users.sh |
Bootstraps broker SCRAM user alongside backend/connect users. |
docker-compose.secure.yml |
Wires broker JAAS, disables ZooKeeper SASL, adds Kafka healthcheck, and sets Kafka/Connect TLS envs. |
.env.example |
Documents broker SCRAM env vars for secure-mode runs. |
| CONNECT_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | ||
| CONNECT_PRODUCER_SECURITY_PROTOCOL: SASL_SSL | ||
| CONNECT_PRODUCER_SASL_MECHANISM: SCRAM-SHA-512 | ||
| CONNECT_PRODUCER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | ||
| CONNECT_PRODUCER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | ||
| CONNECT_PRODUCER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | ||
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | ||
| CONNECT_CONSUMER_SECURITY_PROTOCOL: SASL_SSL | ||
| CONNECT_CONSUMER_SASL_MECHANISM: SCRAM-SHA-512 | ||
| CONNECT_CONSUMER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | ||
| CONNECT_CONSUMER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | ||
| CONNECT_CONSUMER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | ||
| CONNECT_CONSUMER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: |
There was a problem hiding this comment.
CONNECT_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM is set to an empty value, which disables TLS hostname verification for Kafka Connect. Since the broker certs now include SANs for kafka/localhost, this should remain enabled (remove this override or set it to HTTPS) to avoid weakening transport security.
| CONNECT_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | |
| CONNECT_PRODUCER_SECURITY_PROTOCOL: SASL_SSL | |
| CONNECT_PRODUCER_SASL_MECHANISM: SCRAM-SHA-512 | |
| CONNECT_PRODUCER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | |
| CONNECT_PRODUCER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | |
| CONNECT_PRODUCER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | |
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | |
| CONNECT_CONSUMER_SECURITY_PROTOCOL: SASL_SSL | |
| CONNECT_CONSUMER_SASL_MECHANISM: SCRAM-SHA-512 | |
| CONNECT_CONSUMER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | |
| CONNECT_CONSUMER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | |
| CONNECT_CONSUMER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | |
| CONNECT_CONSUMER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | |
| CONNECT_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: HTTPS | |
| CONNECT_PRODUCER_SECURITY_PROTOCOL: SASL_SSL | |
| CONNECT_PRODUCER_SASL_MECHANISM: SCRAM-SHA-512 | |
| CONNECT_PRODUCER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | |
| CONNECT_PRODUCER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | |
| CONNECT_PRODUCER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | |
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: HTTPS | |
| CONNECT_CONSUMER_SECURITY_PROTOCOL: SASL_SSL | |
| CONNECT_CONSUMER_SASL_MECHANISM: SCRAM-SHA-512 | |
| CONNECT_CONSUMER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | |
| CONNECT_CONSUMER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | |
| CONNECT_CONSUMER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | |
| CONNECT_CONSUMER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: HTTPS |
| CONNECT_PRODUCER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | ||
| CONNECT_PRODUCER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | ||
| CONNECT_PRODUCER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | ||
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: |
There was a problem hiding this comment.
CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM is set to an empty value, disabling hostname verification for producer connections. With SAN-correct certs, prefer leaving the default (HTTPS) rather than disabling verification.
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | |
| CONNECT_PRODUCER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: HTTPS |
| CONNECT_CONSUMER_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_CONNECT_USERNAME}" password="${KAFKA_CONNECT_PASSWORD}"; | ||
| CONNECT_CONSUMER_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/connect.truststore.jks | ||
| CONNECT_CONSUMER_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_CONNECT_TRUSTSTORE_PASSWORD} | ||
| CONNECT_CONSUMER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | ||
| volumes: |
There was a problem hiding this comment.
CONNECT_CONSUMER_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM is set to an empty value, disabling hostname verification for consumer connections. Please remove this or set it to HTTPS so TLS verifies the broker hostname.
| KAFKA_SASL_MECHANISM: SCRAM-SHA-512 | ||
| KAFKA_SASL_JAAS_CONFIG: org.apache.kafka.common.security.scram.ScramLoginModule required username="${KAFKA_BACKEND_USERNAME}" password="${KAFKA_BACKEND_PASSWORD}"; | ||
| KAFKA_SSL_TRUSTSTORE_LOCATION: /etc/kafka/secrets/backend.truststore.jks | ||
| KAFKA_SSL_TRUSTSTORE_PASSWORD: ${KAFKA_BACKEND_TRUSTSTORE_PASSWORD} | ||
| KAFKA_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM: | ||
| volumes: |
There was a problem hiding this comment.
KAFKA_SSL_ENDPOINT_IDENTIFICATION_ALGORITHM is set to an empty value, which disables TLS hostname verification for the backend Kafka client. Given the new SAN-enabled broker certs, keep hostname verification enabled (omit this variable or set it to HTTPS).
| sasl.jaas.config=org.apache.kafka.common.security.scram.ScramLoginModule required username="${BROKER_USER}" password="${BROKER_PASS}"; | ||
| ssl.truststore.location=/etc/kafka/secrets/kafka.truststore.jks | ||
| ssl.truststore.password=${TRUSTPASS} | ||
| ssl.endpoint.identification.algorithm= |
There was a problem hiding this comment.
The generated kafka_admin_client.properties explicitly sets ssl.endpoint.identification.algorithm= (blank), which disables TLS hostname verification for the admin client used in healthchecks. With SANs being generated now, this should be enabled (remove the line or set it to HTTPS) to avoid masking hostname/cert issues.
| ssl.endpoint.identification.algorithm= |
| BROKER_USER="${KAFKA_BROKER_USERNAME:-broker-user}" | ||
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:-broker-password}" |
There was a problem hiding this comment.
BROKER_PASS (and BROKER_USER) fall back to hard-coded defaults, but the secure preflight now requires these env vars to be explicitly set. Keeping defaults here can silently generate JAAS/admin artifacts with one credential while the user later sets different values, leading to confusing auth failures. Consider failing fast if these env vars are missing/empty (or have preflight allow defaults), so all steps stay consistent.
| BROKER_USER="${KAFKA_BROKER_USERNAME:-broker-user}" | |
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:-broker-password}" | |
| BROKER_USER="${KAFKA_BROKER_USERNAME:?KAFKA_BROKER_USERNAME must be set and non-empty}" | |
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:?KAFKA_BROKER_PASSWORD must be set and non-empty}" |
| BROKER_USER="${KAFKA_BROKER_USERNAME:-broker-user}" | ||
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:-broker-password}" |
There was a problem hiding this comment.
BROKER_USER / BROKER_PASS default values mean this script can create SCRAM credentials that don’t match the values later required by kafka-secure-preflight.sh, causing hard-to-debug startup failures. Align behavior by requiring these vars to be set (fail fast) rather than defaulting to broker-user/broker-password.
| BROKER_USER="${KAFKA_BROKER_USERNAME:-broker-user}" | |
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:-broker-password}" | |
| BROKER_USER="${KAFKA_BROKER_USERNAME:?KAFKA_BROKER_USERNAME must be set}" | |
| BROKER_PASS="${KAFKA_BROKER_PASSWORD:?KAFKA_BROKER_PASSWORD must be set}" |
Summary
Validation
Scope