Skip to content

APM secret token briefly exists in plain text #89439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pugnascotia opened this issue Aug 17, 2022 · 13 comments · Fixed by #91058
Closed

APM secret token briefly exists in plain text #89439

pugnascotia opened this issue Aug 17, 2022 · 13 comments · Fixed by #91058
Assignees
Labels
:Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team

Comments

@pugnascotia
Copy link
Contributor

PR #88443 implemented a tracer using the Elastic APM Java agent. As discussed in the PR, it was necessary to write a temporary config file for the agent, in order to configure settings that cannot be dynamically set at runtime. Although this config file is rapidly deleted again, it is not ideal that we have to do it at all.

We need to investigate alternatives, which will require a conversation with the APM team about alternative approaches to configure the agent.

@pugnascotia pugnascotia added the :Core/Infra/Core Core issues without another label label Aug 17, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Aug 17, 2022
@grcevski grcevski self-assigned this Aug 31, 2022
@graphaelli
Copy link
Member

@AlexanderWert can help coordinate APM support for this (fyi @felixbarny for when you return)

@felixbarny
Copy link
Member

How would you ideally want to configure the secret token for the agent? Maybe add a custom configuration source to the agent that can read out the secret token from the ES key store?

@grcevski
Copy link
Contributor

Hi @felixbarny,

Yes that would be a possible solution for us, if we could make and supply somehow our own configuration, source then we can then provide the tokens from the Elasticsearch secure settings (e.g. keystore).

Another question I have is if it's possible to make the secret_token and the api_key dynamic properties? Is there a particular technical reason they aren't mutable, like server_urls is? If they were mutable, we could implement key rotation. Also, perhaps if we could change those properties at runtime, maybe we can use Java system properties to set them instead of using the file? I'm not sure of the security implications there though.

@felixbarny
Copy link
Member

Could you link to the place in code where you initialize the agent? Or are you starting the agent by adding the -javaagent JVM option?

Another question I have is if it's possible to make the secret_token and the api_key dynamic properties?

That would be feasible, I think. However, note that the agent is trying to connect to the server as soon as its started. That happens even if no server_url is provided at startup as the agent defaults to http://localhost:8200. So if either the url or the key/token is missing, the agent will log the failing attempts to invoke the health check endpoint.

@grcevski
Copy link
Contributor

We start by adding the -javaagent option. Essentially, we have a Java based launcher, which parses various options, applies JVM ergonomics and eventually it launches the Elastisearch process. The launch happens here:

https://github.com/elastic/elasticsearch/blob/main/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/ServerProcess.java#L91

The call to JvmOptionsParser::determineJvmOptions eventually trickles down to https://github.com/elastic/elasticsearch/blob/main/distribution/tools/server-cli/src/main/java/org/elasticsearch/server/cli/APMJvmOptions.java#L137

where we write a temporary config file and point the agent to it via the command line.

Eventually once the Elasticsearch node starts, it checks to see if it was launched with the APM agent command line argument and it deletes the file:

https://github.com/elastic/elasticsearch/blob/main/server/src/main/java/org/elasticsearch/node/Node.java#L407

I'm not sure if we ever attempted to use the attach method. I guess in that case we'd be able to use the Java system properties, since we'll be in the same address space.

@pugnascotia
Copy link
Contributor Author

I made a brief attempt to use the attach method from within the ES APM module, but the security manager was not happy at all.

@grcevski
Copy link
Contributor

Thanks @pugnascotia, that makes sense, there's very little that happens before we install the security manager.

@felixbarny
Copy link
Member

The reason you don't set the api_key as a system property or a environment variable is that you're concerned that plugins may read and exfiltrate the key, is that correct?

What's your ideal startup sequence for the agent? Would you want to start the agent in a mode where it avoids any connectivity to APM Server (but still initializes instrumentation) and then set the server_url and api_key via an API which then makes the agent connect to APM Server? I think that would be possible but definitely more complex compared to setting the connection info as environment variables in the Elasticsearch process builder.

@grcevski
Copy link
Contributor

The reason you don't set the api_key as a system property or a environment variable is that you're concerned that plugins may read and exfiltrate the key, is that correct?

Yes that's correct, environment variables and system properties would be less secure, even without external plugins, there are number of monitoring tools that regularly grab them and the secrets will essentially leak.

As you mentioned, an API where we can set these settings would be ideal. To be honest, I think the current design with the temporary file is fine, the main concern is key rotation, however modifying secrets in the Java keystore currently results in a node restart. We plan to improve this situation, so if API is too complex, is there a way for us to add a custom configuration source (or is that what you mean by API)? At least if we had the ability to dynamically change the secrets just like we can the server URLs, we'd have the ability to rotate the keys without a restart. I'm not sure how often people do this, perhaps it's not a typical usecase?

@felixbarny
Copy link
Member

I've created

This should be trivial to implement.

An API to set a config option or a custom configuration source would be feasible, too. But just reemphasizing that currently, the connection information needs to be available right when starting the agent (in your case at JVM start) as the agent tries to connect to the server immediately. However, it would be possible to add a mode to the agent where it starts but doesn't connect to APM server yet. Then you'd be able to provide the connection details after ES has started, for example, with an agent API that lets you add a configuration source.

@grcevski
Copy link
Contributor

Thanks @felixbarny, I'm going to experiment a bit and see if I can work around the security manager issues that we encountered while trying to initialize the agent in code. Otherwise we can keep the existing design and when we hot reload keys on rotation, we can maybe use the System properties source to pass the new keys.

@felixbarny
Copy link
Member

Version 1.35.0 of the Java agent has been released which adds support for dynamically reloading secret_token and api_key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants