michaeljmarshall opened a new issue, #20862:
URL: https://github.com/apache/pulsar/issues/20862

   ### Search before asking
   
   - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) 
and found nothing similar.
   
   
   ### Motivation
   
   Provide a generic way to inject secrets into the `config` map for connectors 
without requiring a rewrite of each source/sink and without leaking them on the 
command line used to start the connector.
   
   ## Context
   
   The recent 
[CVE-2023-37579](https://github.com/apache/pulsar/wiki/CVE%E2%80%902023%E2%80%9037579)
 resulted in the potential to leak source/sink credentials because some 
credentials are stored in configuration instead of in the `secrets` map.
   
   The current way to configure secrets for connectors requires each source or 
sink to implement correct secret handling by either getting a secret from the 
[SecretsProvider](https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-functions/secrets/src/main/java/org/apache/pulsar/functions/secretsprovider/SecretsProvider.java#L28C1-L42)
 or by using [custom 
annotations](https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-io/core/src/main/java/org/apache/pulsar/io/core/annotations/FieldDoc.java#L28-L66)
 and this [special configuration 
loader](https://github.com/apache/pulsar/blob/674655347da95305cf671f0696f113dcca88b44d/pulsar-io/common/src/main/java/org/apache/pulsar/io/common/IOConfigUtils.java#L38-L103).
 This implementation assumes that users have a `Configuration` class that can 
be annotated, but that is not always the case because the connector framework 
passes a configuration map of `Map<String, Object>`. Note that t
 he current mechanism is not well documented and is not used by all of the 
official Apache Pulsar connectors.
   
   ### Solution
   
   I propose that we materialize and merge all secrets from the `secrets` map 
into the `config` that is passed to the connector when we call `Source#open` or 
`Sink#open`. Materializing the secrets would look like:
   
   ```java
       private void mergeSecretsIntoConfigs(Map<String, Object> secrets, 
Map<String, Object> configs) {
           for (Map.Entry<String, Object> entry : secrets.entrySet()) {
               Object oldValue = configs.put(entry.getKey(),
                       secretsProvider.provideSecret(entry.getKey(), 
entry.getValue()));
               if (oldValue != null) {
                   log.warn("Config value for {} replaced by secret's 
configuration value.", entry.getKey());
               }
           }
       }
   ```
   
   They key benefit to this solution is that it will work for all sinks and 
sources, and it will leverage the `SecretsProvider` interface to materialize 
the secrets.
   
   ## Trade off
   
   The one drawback to this solution is that it could theoretically break 
existing connector configuration. However, I think this is very unlikely 
because it only breaks when a configuration and a secret are passed with the 
same `key`.
   
   ### Alternatives
   
   We could consider interpreting configuration values that start with a well 
known prefix, like `env:`, as values that need to be read from the environment. 
The primary drawback to this solution is that there is not an easy way to 
configure the function at this point in the code, which means that 
   
   This solution would look something like adding this code block
   
   ```java
       // Replace environment variable pointers with their environment variable 
values
       for (Map.Entry<String, Object> entry : config.entrySet()) {
           if (entry.getValue() instanceof String && ((String) 
entry.getValue()).toLowerCase().startsWith("env:")) {
               String envVariableName = ((String) 
entry.getValue()).substring("env:".length());
               String envVariableValue = System.getenv(envVariableName);
               entry.setValue(envVariableValue);
           }
       }
   ```
   
   to this method
   
   
https://github.com/apache/pulsar/blob/f7c0b3c49c9ad8c28d0b00aa30d727850eb8bc04/pulsar-functions/instance/src/main/java/org/apache/pulsar/functions/instance/JavaInstanceRunnable.java#L884-L929
   
   ### Anything else?
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [X] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to