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

Refer to Spring Cloud Vault in documentation for property encryption #13618

Closed
ESegundoRolon opened this issue Jun 29, 2018 · 10 comments
Closed
Assignees
Labels
type: documentation A documentation update
Milestone

Comments

@ESegundoRolon
Copy link

ESegundoRolon commented Jun 29, 2018

Hello,
The following code snippet running on Spring Boot 1.5.12 RELEASE:

public class UnicodePropertiesPropertySourceLoader implements PropertySourceLoader {

    @Override
    public String[] getFileExtensions() {
        return new String[]{"properties"};
    }
    @Override
    public PropertySource<?> load(String name, Resource resource, String profile) throws IOException {
        if (profile == null) {
            Properties properties = new Properties();
            PropertyResourceBundle bundle = new PropertyResourceBundle(new InputStreamReader(resource.getInputStream(), "UTF-8"));
            Enumeration<String> keys = bundle.getKeys();
            while (keys.hasMoreElements()) {
                String key = keys.nextElement();
                properties.setProperty(key, bundle.getString(key));
            }
            if (!properties.isEmpty()) {
                return new PropertiesPropertySource(name, properties);
            }
        }
        return null;
    }
}

Has to be migrated for Spring Boot 2.0.3 RELEASE:

public class UnicodePropertiesPropertySourceLoader implements PropertySourceLoader {

    private static final String PROPERTIES_FILE_EXTENSION = ".properties";

    @Override
    public String[] getFileExtensions() {
        return new String[]{"properties"};
    }

    @Override
    public List<PropertySource<?>> load(String name, Resource resource) throws IOException {
            Properties properties = new Properties();
            PropertyResourceBundle bundle = new PropertyResourceBundle(new InputStreamReader(resource.getInputStream(), "UTF-8"));
            Enumeration<String> keys = bundle.getKeys();
            while (keys.hasMoreElements()) {
                String key = keys.nextElement();
                properties.setProperty(key, bundle.getString(key));
            }
            if (!properties.isEmpty()) {
                return Collections
                        .singletonList(new OriginTrackedMapPropertySource(name, properties));
            }

        return null;
    }

After and before the migration, debugging the load method, the properties are loaded just fine for encoded rows, like:

email.hello=!Congrats

But when you load the property using:

@Value("${email.hello}")

The property becomes wrong for the new version of Spring Boot:

Â!Congrats

My spring.factories under META-INF is set to:

# Custom PropertySource Loaders
org.springframework.boot.env.PropertySourceLoader=my.package.UnicodePropertiesPropertySourceLoader

I know that my class UnicodePropertiesPropertySourceLoader is loaded just fine with all the properties encoded to UTF-8, but the problem is when I try to obtain each property with the @Value annotation, this happen for the version of SpringBoot 2.0.3.Release

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 29, 2018
@philwebb
Copy link
Member

Could you please provide a sample application that shows the problem. It's a little hard to tell what the actual values you're using are from the GitHub comment.

It looks like you need UTF-8 encoded properties. I wonder if we might be able to make the existing OriginTrackedPropertiesLoader easier to subclass to do that.

@wilkinsona wilkinsona changed the title Custom PropertySourceLoader beging ignored when use of @Value annotation Custom PropertySourceLoader being ignored when use of @Value annotation Jul 2, 2018
@wilkinsona wilkinsona changed the title Custom PropertySourceLoader being ignored when use of @Value annotation Custom PropertySourceLoader is ignored when using @Value annotation Jul 2, 2018
@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Jul 2, 2018
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jul 9, 2018
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged labels Jul 16, 2018
@ryber
Copy link

ryber commented Aug 24, 2018

Hello, @spring-issuemaster I am also getting this issue as well. I have it isolated down to a simple project here: https://github.com/ryber/spring-props/

Note:

  • I have the spring.factory set correctly (I think)
  • When you start the application you can clearly see in the sys.out where the encrypted properties are being read (several times for some reason)
  • When you go to the root controller (In Hello.java) the @Value value is not encrypted.

@philwebb philwebb reopened this Aug 27, 2018
@philwebb philwebb added the status: waiting-for-triage An issue we've not yet triaged label Aug 27, 2018
@philwebb
Copy link
Member

@ryber The PropertySourceLoader interface is really designed to allow additional file types to be loaded. Since there's a built in PropertiesPropertySourceLoader providing properties file support I think your loader might be getting called in addition the the regular one.

We should probably guard against that, or at least improve the Javadoc. You might want to look into the EnvironmentPostProcessor as an alternative way of modifying properties. I think that's how Spring Cloud Config supports encryption.

@philwebb philwebb added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 27, 2018
@philwebb philwebb added this to the 2.0.x milestone Aug 27, 2018
@ryber
Copy link

ryber commented Aug 28, 2018

thanks @philwebb, now that I'm looking at EnvironmentPostProcessor I totally remember doing it this way before. (I was sure I never did one of those old school META-INF factories before)

In any case, just FYI I went down the PropertySourceLoader path because googling lead to some blogs and SO questions implying that was the right way. So you may get more of those from time to time without maybe some official Spring documentation or logging that points in a better direction.

FYI, in my case I couldn't just use the jasypt-spring-boot-starter because we are using KMS in this project.

@snicoll
Copy link
Member

snicoll commented Aug 28, 2018

So you may get more of those from time to time without maybe some official Spring documentation or logging that points in a better direction.

@ryber thanks for the feedback. There are 3 somewhat-related reference to PropertySourceLoader in the documentation, two of them in this section that describes the EnvironmentPostProcessor should be used to customize the Environment.

What am I missing? Do you have something else in mind to avoid users choosing the wrong path?

@ryber
Copy link

ryber commented Aug 28, 2018

I think the main issue is that EnvironmentPostProcessor is kind of general catch all for doing "stuff" to the environments. I was looking specifically for encrypting properties. And so are a lot of other people. "Security By Design" is starting to be a major thing (thank goodness) and I think it would be great if Spring Boot incorporated that our of the gate.

If you google for "Encrypt Spring Boot properties" the Spring documentation is nowhere in the entire first page. and even the description of EnvironmentPostProcessor isn't clear about specifically overriding or manipulating the existing spring managed properties.

I would encourage Spring Boot to include a first line support of encrypted properties and some clear documentation for people to do it on day 1. I would hope that such a feature might have 1 that works out of the box (possibly jasypt) and also the ability to override for those who want other decryptors like KMS, Vault, etc.

@philwebb
Copy link
Member

Some discussion on encrypted property source is in #1312. My current feeling is that providing good encryption support is hard and we should leave it to Spring Cloud Vault (although I agree that the lack of documentation is problematic).

@philwebb
Copy link
Member

although I agree that the lack of documentation is problematic

By this I mean that it seems like encryption is a common issue so it would be nice if we mention it in the docs, even if it's just a link to Spring Cloud Vault.

@mbhave mbhave added type: documentation A documentation update and removed type: task A general task labels Sep 11, 2018
@philwebb philwebb changed the title Custom PropertySourceLoader is ignored when using @Value annotation Refer to Spring Cloud Vault in documentation for property encryption Oct 11, 2018
@philwebb philwebb self-assigned this Oct 11, 2018
@philwebb philwebb modified the milestones: 2.0.x, 2.1.0.RC1, 2.0.6 Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

No branches or pull requests

7 participants