Skip to content
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

Implement uniform password "encryption/decryption" mechanism #2558

Open
ppkarwasz opened this issue May 2, 2024 · 4 comments
Open

Implement uniform password "encryption/decryption" mechanism #2558

ppkarwasz opened this issue May 2, 2024 · 4 comments

Comments

@ppkarwasz
Copy link
Contributor

Many of Log4j components require a password attribute (e.g. SslConfiguration, JdbcAppender, etc.), but only BasicAuthorizationProvider uses a pluggable PasswordDecryptor service to interpret the meaning of the password field.

I would propose to extend the usage of this service to all password fields and provide two implementations of PasswordDecryptor:

  • one implementation that interprets the "password" as the path to a file that contains the password,
  • one implementation that interprets the "password" as the name of an environment variable that contains the password.

This would allow us to deprecate configuration properties such as log4j2.keyStorePasswordFile and log4j2.keyStorePasswordEnvironmentVariable.

Disclaimer: I am fully aware that a real password encryption/decryption mechanism doesn't make sense, since configuration sources must be trusted anyway and I totally agree with the remarks about password encryption on Tomcat's Wiki.

However some auditors and analysis tools might have problems with plain text passwords in configuration files and this feature will allow users to provide their own workarounds.

@rgoers
Copy link
Member

rgoers commented May 3, 2024

FYI, in our configurations we use something like this in our Spring application's bootstrap.yml. This allows us to retrieve our logging configuration, with overrides, from our Spring Cloud Config Service. Note that the user name and password specified in bootstrap.yml ONLY work for developer testing on their laptop. When deployed we specify the spring username and password via system property overrides on the command line.Those are stored in a vault and accessed only from the deployment scripts. SCC supports Basic authentication. In other words, in my case I would have no use for the password decryptor as whatever we do has to be supported by Spring.

spring:
  application:
    name: my-application-service
  cloud:
    config:
      uri: https://spring-configuration-server.apache.foundation
      username: my-user@serviceuser.apache.foundation
      password: 123Abc!@#


logging:
  config: classpath:log4j2-console.yml
  label: ${spring.cloud.config.label:master}
  auth:
    username: ${spring.cloud.config.username}
    password: ${spring.cloud.config.password}

---
spring:
  config:
    activate:
      on-profile: local

---
spring:
  config:
    activate:
      on-profile: dev
  cloud:
    config:
      uri: https://spring-configuration-server.apache.foundation

logging:
  config: "https://spring-configuration-server.apache.foundation/my-application-service/default/${logging.label}/log4j2-json-dev.xml"
  log4j2:
    config:
      override: "https://spring-configuration-server.apache.foundation/my-application-service/default/${logging.label}/log4j2-my-application-service-dev.xml"

@ppkarwasz
Copy link
Contributor Author

@rgoers,

So it seems that the most uniform way to provide passwords is to either provide them literally or use a lookup?

Should we deprecate PasswordDecryptor and the alternative ways to provide a password for the key stores?

@rgoers
Copy link
Member

rgoers commented May 30, 2024

@ppkarwasz Sorry, I should have replied sooner. In the case of Spring the passwords are encrypted in the configuration and specified as {cipher}some_encrypted_string. When the app gets them all the ciiphers have been converted to clear text. But Spring certainly isn't the only thing around and not everyone uses Spring Cloud Config.

But this is also about more than configuration. If you look at PostMan's authorization tab it lists a dozen or so mechanism for authenticating. This is why we have an AuthorizationProvider. In the default case we just populate the Authorization header using Basic Auth with the credentials. But other mechanisms might do much more than that. For example, with OAuth a token will have to be retrieved using the client's credentials, client id, and client secret. along with a couple of urls. My suggestion would be to a) ensure AuthorizationProvider has sufficient methods and b) ensure it is being used wherever authentication needs to be performed.

@ppkarwasz
Copy link
Contributor Author

[I remember writing a longer comment on this issue but I might have forgotten to press the "Comment" button]

I agree that AuthorizationProvider needs to stay and be expanded, but I think that:

  • PasswordDecryptor can be removed: users can implement their own AuthorizationProvider instead. I totally agree with Tomcat's "Why are plain text passwords in the config files?" Wiki entry: centralizing passwords using Spring Cloud Config makes sense, encrypting them only to put them in another file doesn't.
  • The *.passwordEnvVar and *.passwordFile properties of SslConfigurationFactory can be removed for the same reason. We can replace them with an SslConfigurationFactoryProvider that has a single createSslContext() method, so that users can use alternative ways to create an SSL connection.
  • We should probably simplify the Transport Security properties. Their names were already long in your properties enhancement, my refactoring made them excessively long.
    I would propose to base the naming convention of these properties on Spring Boot SSL, i.e. have properties named:
    • log4j.ssl.verifyHostName,
    • log4j.ssl.truststore.provider (instead of algorithm)
    • log4j.ssl.truststore.location,
    • log4j.ssl.truststore.password,
    • log4j.ssl.truststore.type,
    • ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants