strimzi-kafka-operator icon indicating copy to clipboard operation
strimzi-kafka-operator copied to clipboard

[Bug]: local keystore passwords appear in the process table

Open freedge opened this issue 10 months ago • 12 comments

Bug Description

on clusters running kafka deployments deployed with strimzi-operator, we find various processes called with a password as argument. The password appears in the process table and is recorded by auditing tools

Steps to reproduce

  1. deploy a kafka cluster with strimzi
  2. observe the cluster through stackrox

Expected behavior

no password appearing on the command line

Strimzi version

strimzi-cluster-operator.v0.40.0

Kubernetes version

OpenShift 4.14

Installation method

from community operators

Infrastructure

Bare-metal

Configuration files and logs

No response

Additional context

example of code passing a password as argument to a process: https://github.com/strimzi/strimzi-kafka-operator/blob/4e8f6e45b0f81fd73be07890b05686562620229f/topic-operator/scripts/tls_prepare_certificates.sh#L19

processes found by stackrox:

 frigo     | zookeeper      | openssl     | pkcs12 -export -in /opt/kafka/zookeeper-node-certs/my-second-cluster-zookeeper-1.crt -inkey /opt/kafka/zookeeper-node-certs/my-second-cluster-zookeeper-1.key -chain -CAfile /opt/kafka/cluster-ca-certs/ca.crt -name my-second-cluster-zookeeper-1 -password pass:V0JM6KjQSw12rpep... -out /tmp/zookeeper/cluster.keystore.p12
 -certpbe aes-128-cbc -keypbe aes-128-cbc -macalg sha256

 frigo     | zookeeper      | keytool     | -keystore /tmp/zookeeper/cluster.truststore.p12 -storepass 2UwZDzZFaPqtOmoRD9Tmb-1fkdY... -noprompt -alias ca -import -file /opt/kafka/cluster
-ca-certs/ca.crt -storetype PKCS12

 frigo     | topic-operator | keytool     | -keystore /tmp/topic-operator/replication.truststore.p12 -storepass 9D1_RpQQJoWJhSTcoVvhqqynkc9... -noprompt -alias ca -import -file /etc/tls-
sidecar/cluster-ca-certs/ca.crt -storetype PKCS12

freedge avatar Apr 12 '24 20:04 freedge

These are the passwords generated at pod startup and used for the PKCS12 stores used by the ZooKeeper / Kafka. They will be also stored in their configuration files. Plus the original PEM files are mounted from the secrets next to them. So I do not think these passwords really leak anything secret that you cannot otherwise obtain.

scholzj avatar Apr 12 '24 23:04 scholzj

for example a pod running as user nobody on the node, and started with hostPID=true (such as a node exporter pod for example) would be able to see the password when running a ps a the right time. It does not have access to the configuration files or PEM files or secret or mounts from strimzi pods, but it can obtain the secret from the process table.

freedge avatar Apr 13 '24 14:04 freedge

for example a pod running as user nobody on the node, and started with hostPID=true (such as a node exporter pod for example) would be able to see the password when running a ps a the right time. It does not have access to the configuration files or PEM files or secret or mounts from strimzi pods, but it can obtain the secret from the process table.

Sure. But the passwords are generated at the container startup and used for the PKCS12 files generated in the container. So you need to access the PKCS12 files in the container to use the passwords and if you can do that, you have the password anyway in the config file next to it.

I do not have a problem if anyone wants to improve this. But want to make it more clear what these passwords are.

scholzj avatar Apr 13 '24 16:04 scholzj

Discussed on the community call on 18.4.: As explained above, this is not a real security issue. But would be nice to have it fixed.

@freedge do you plan to contribute this? Or can someone else look into it?

scholzj avatar Apr 18 '24 16:04 scholzj

hi, not really. (I made a list of findings and this one is at the bottom since it's not really an issue). Also it's not super trivial, in addition to fixing a bunch of scripts, https://github.com/strimzi/strimzi-kafka-operator/blob/51442a7acb8e81e14b8a4b7a066cad4e909b3bab/kafka-agent/src/main/java/io/strimzi/kafka/agent/KafkaAgent.java#L452 will need a new way to pass a password. It would be nice if there was no password appearing though, that would make the overall environment easier to audit (but, not safer).

freedge avatar Apr 18 '24 16:04 freedge

Hi @scholzj, as I leaved comments under the duplicated [issue] (https://github.com/strimzi/strimzi-kafka-operator/issues/10399), my colleague @wangshu3000 and me are working together on this issue currently, we have completed the poc and tested in our local, @wangshu3000 will share our solution here, please help review and share your feedback. thank you!

haijun2022 avatar Aug 06 '24 02:08 haijun2022

Change Plan:

  1. KafkaAgent.java Update parameter validation logic, check parameter number < 3 Remove the 4 parameters sslKeyStorePath/sslKeyStorePass/sslTrustStorePath/sslTrustStorePass from the args, add properties file path parameter. Combine the 4 parameters into a properties file. Read/parse the parameter file to retrieve the 4 parameter then pass to KafkaAgent constructor method

  2. kafka_run.sh Generate a properties file before building KAFKA_OPTS parameter. The properties file includes the 4 parameters sslKeyStorePath/sslKeyStorePass/sslTrustStorePath/sslTrustStorePass Save the properties file as /tmp/kafka-agent.properties, in same folder as strimzi.properties file Remove the 4 parameters from KAFKA_OPTS and use the properties file instead

Test Step:

  1. in kafka broker & controller pod, cd /proc/
  2. cd to the 7xx instance folder, and check the content of cmdline file, this file will include the process startup parameter, make sure there is no plaintext pwd in the end of the command.
  3. Functional testing to make sure the agent works as expected.

The process startup command will be changed from: ......-javaagent:/opt/kafka/libs/kafka-agent-0.42.0.jar=::/tmp/kafka/cluster.keystore.p12:xxxxxxxxxxxxxxxxxx:/tmp/kafka/cluster.truststore.p12:xxxxxxxxxxxxxxxxxx to: ......-javaagent:/opt/kafka/libs/kafka-agent-0.42.0.jar=::/tmp/kafka-agent.properties

wangshu3000 avatar Aug 06 '24 05:08 wangshu3000

@scholzj / @wangshu3000 Any plan to fix another security issue in kafka strimzi code that CERTS_STORE_PASSWORD is exposed as environment variable. This env variable exposed and used for certificate creation of cruise control, mirror maker, kafka connect and many other places.

ekowsal avatar Aug 06 '24 05:08 ekowsal

@ekowsal I do not think there is any issue if someone wants to improve it through some reasonable code. But once again, there is no security issue here. Anyone who can get the keystore with the key can also get the password in any case. So you are not doing any real "security" by this.

@wangshu3000 I think using some properties file for the agent sounds reasonable.

scholzj avatar Aug 06 '24 08:08 scholzj

@ekowsal I do not think there is any issue if someone wants to improve it through some reasonable code. But once again, there is no security issue here. Anyone who can get the keystore with the key can also get the password in any case. So you are not doing any real "security" by this.

@wangshu3000 I think using some properties file for the agent sounds reasonable.

Thanks @scholzj & @ekowsal

@scholzj I think you mean we're good to make the change right? Moving the 4 parameters to a properties file. We'll raise PR shortly.

Thanks.

CC: @haijun2022

wangshu3000 avatar Aug 06 '24 08:08 wangshu3000

@scholzj I think you mean we're good to make the change right? Moving the 4 parameters to a properties file. We'll raise PR shortly.

Obviously, I'm only one of the @strimzi/maintainers nd can speak only for my self. But it sounds like something what you can open a PR for.

scholzj avatar Aug 06 '24 08:08 scholzj

I am totally with @scholzj that we are not going to make the overall process more secure with the suggested change. Said that, I am fine with passing paramaters to the Kafka Agent via a properties file.

ppatierno avatar Aug 06 '24 09:08 ppatierno