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

[freedge]

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:

strimzi-kafka-operator/topic-operator/scripts/tls_prepare_certificates.sh

Line 19 in 4e8f6e4

keytool ${KEYTOOL_OPTS} -keystore "$1" -storepass "$2" -noprompt -alias "$4" -import -file "$3" -storetype PKCS12

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

[scholzj]

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.

[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.

[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.

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]

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?

[freedge]

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,

strimzi-kafka-operator/kafka-agent/src/main/java/io/strimzi/kafka/agent/KafkaAgent.java

Line 452 in 51442a7

public static void premain(String agentArgs) {

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).

[haijun2022]

Hi @scholzj, as I leaved comments under the duplicated [issue] (#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!

[wangshu3000]

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

[ekowsal]

@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.

[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.

[wangshu3000]

@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

[scholzj]

@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.

[ppatierno]

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.

[katheris]

Is there any outstanding work needed for this bug or can it be closed?