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

Add Ability to skip property value decryption (e.g. "decryptProperties=false") #2632

Closed
marnee01 opened this issue Nov 11, 2024 · 12 comments · Fixed by #2708
Closed

Add Ability to skip property value decryption (e.g. "decryptProperties=false") #2632

marnee01 opened this issue Nov 11, 2024 · 12 comments · Fixed by #2708
Milestone

Comments

@marnee01
Copy link
Contributor

marnee01 commented Nov 11, 2024

Is your feature request related to a problem? Please describe.
Yes, this is related to a problem. We make use of the endpoint that returns resolved properties in ".properties" format. We do this for our prod profiles for all of our applications once a month before release. We provide these to our dev teams to aid troubleshooting. We can't have the sensitive encrypted property values appear in these files. Therefore, we put the request to the config server in one of our lower environments.

The issue is that when a property references another property which is encrypted, the result is not as we'd like.

Example:

Default File:

SharedPassword=Fill_me_in

application-prd.properties:

SharedPassword={cipher}BobLoblaw
Property1=${SharedPassword}

Then when we get the properties for prd profile, it is returned with the value from the default file:

Property1=Fill_me_in

The reason being that it can't decrypt SharedPassword, it sets it to invalid.SharedPassword. When Property1 is resolved, it finds only the one in the default file. This can be misleading. This is just one example. We get undesired results in different scenarios due to this.

Describe the solution you'd like
We'd like to have the endpoint support a query parameter like decryptProperties. It would default to true. We would call it with false for creating our reports. When false, the endpoint logic would skip the decryption step and leave the values with the {cipher} tag.

And thus, in our example, it would return the value as-is: Property1={cipher}BobLoblaw.

I looked at making this change myself. However, I believe this change would entail touching many subclasses of EnvironmentRepository. I am not set up to test all of those. I am also unsure which of these support encrypted values. Except where it is obvious that it does, it would require more time to investigate than my company would allow me to take.

Workaround
Posting our planned workaround (in case it helps someone else). Our plan is to add @Profile("!noencrypt") to our beans that provide the decryption logic. We will run a special config server with "noencrypt" profile and call that when generating these resolved-properties reports. Note that encrypted properties are returned in their encrypted form, but are not returned with the "{cipher}" tag.

@marnee01 marnee01 changed the title Add query parameter decryptProperties Add query parameter to skip property value decryption (e.g. "decryptProperties=false") Nov 11, 2024
@ryanjbaxter
Copy link
Contributor

I want to make sure I completely understand your use case.

Is it that sometimes you want the properties files to return the decrypted values and sometimes you don't? By default serving plain text files will not return the decrypted values unless you set spring.cloud.config.server.encrypt.enabled=true and spring.cloud.config.server.encrypt.plainTextEncrypt=true. I am assuming you have both set to true? I would think you could run a special config server with these properties set to false and achieve what you need without using the profile you suggested in your workaround.

@marnee01
Copy link
Contributor Author

marnee01 commented Nov 14, 2024

Yes, I want to sometimes decrypt and sometimes not. Ideally on the same ConfigServer with a decryptProperties query param.

I wasn't aware of these properties: spring.cloud.config.server.encrypt.enabled=true and spring.cloud.config.server.encrypt.plainTextEncrypt=true. We are not setting them.

I am not discussing the endpoint that provides the original/plain text file. We are calling the one that resolves it all into a final set of properties, resolving variables, then returning in format of .properties, .json, or .yml.

For example, the EnvironmentController method jsonProperties that has the @GetMapping("{name}-{profiles}.json") annotation. I believe there are six methods for this type of request (3 that take a label, 3 that don't, and then for JSON, YAML, and properties format).

I tried setting spring.cloud.config.server.encrypt.enabled=true and spring.cloud.config.server.encrypt.plainTextEncrypt=true in both the application.yml file and the bootstrap file, and it still returned decrypted values. (We're using the old bootstrap file approach still.)

Either way, standing up a new service for this one need is not ideal on our end. However, this might be such an obscure requirement that I understand if it's not worth the effort to implement my feature request here.

@ryanjbaxter
Copy link
Contributor

Sorry for the long feedback time here.

I am trying to reproduce what you are seeing. I have a config server pointing to the local file system for its environment repository. In the directory there are 2 files:

application.yaml

SharedPassword: Fill_me_in

application-prd.yaml

SharedPassword: '{cipher}AYBKlpcZpaR36OcRDQjNIQl6fmnddAQhetMw/uyTpnn5fDj+unJ9QOEbqiPc9fX0N+CC8i+EJiN6nlH9Xqu6sH1tX/P6zg1CIy+ct/1RWGNbmQ256jc6vQaXhiN8sA8Mr6QiqYnMoBd+Jni/Miir5G3a7G9MmjbEUASKJOhUlIFKqL1IqB81RBT/cv0bg9kAiy5VBF1WppxP/PwtjECzbeUi2Y1jbpYb98rnc/qmRO3ZJam9fDNcPpW09qGFhGgJIujca257F7G4guS2w/7haVzNoyRiwHzZ14oL8AIxHLMBSJJF19ULlsMAkROj9o9TnwhL9r4rX9sAWk28c5eq77+iVpmlT3yoRdZqvMqffzKiibDlzz95Gmms7V7mctxrhNVOOWTwMSJvk94Y9ZPenljKgPJIV3Z1cqqx+W8JxFFeelOuYvMEe4bOVBh1TepGzzdWVdYbylgXJy35uRTZ2drybUe5+jc0hiAuujHz0zdY1FwOHfwzSsSidlYn4syPeuytnxTzn7fbWXeXetTTtDlmLRf8MBSzXzDFWNH0cNGOCQ=='
Property1: ${SharedPassword}

When I make a request to /application-prd.json I get the following response:

HTTP/1.1 200 
Connection: keep-alive
Content-Length: 1156
Content-Type: application/json
Date: Wed, 08 Jan 2025 16:14:43 GMT
Keep-Alive: timeout=60

{
    "Property1": "AYBKlpcZpaR36OcRDQjNIQl6fmnddAQhetMw/uyTpnn5fDj+unJ9QOEbqiPc9fX0N+CC8i+EJiN6nlH9Xqu6sH1tX/P6zg1CIy+ct/1RWGNbmQ256jc6vQaXhiN8sA8Mr6QiqYnMoBd+Jni/Miir5G3a7G9MmjbEUASKJOhUlIFKqL1IqB81RBT/cv0bg9kAiy5VBF1WppxP/PwtjECzbeUi2Y1jbpYb98rnc/qmRO3ZJam9fDNcPpW09qGFhGgJIujca257F7G4guS2w/7haVzNoyRiwHzZ14oL8AIxHLMBSJJF19ULlsMAkROj9o9TnwhL9r4rX9sAWk28c5eq77+iVpmlT3yoRdZqvMqffzKiibDlzz95Gmms7V7mctxrhNVOOWTwMSJvk94Y9ZPenljKgPJIV3Z1cqqx+W8JxFFeelOuYvMEe4bOVBh1TepGzzdWVdYbylgXJy35uRTZ2drybUe5+jc0hiAuujHz0zdY1FwOHfwzSsSidlYn4syPeuytnxTzn7fbWXeXetTTtDlmLRf8MBSzXzDFWNH0cNGOCQ==",
    "SharedPassword": "AYBKlpcZpaR36OcRDQjNIQl6fmnddAQhetMw/uyTpnn5fDj+unJ9QOEbqiPc9fX0N+CC8i+EJiN6nlH9Xqu6sH1tX/P6zg1CIy+ct/1RWGNbmQ256jc6vQaXhiN8sA8Mr6QiqYnMoBd+Jni/Miir5G3a7G9MmjbEUASKJOhUlIFKqL1IqB81RBT/cv0bg9kAiy5VBF1WppxP/PwtjECzbeUi2Y1jbpYb98rnc/qmRO3ZJam9fDNcPpW09qGFhGgJIujca257F7G4guS2w/7haVzNoyRiwHzZ14oL8AIxHLMBSJJF19ULlsMAkROj9o9TnwhL9r4rX9sAWk28c5eq77+iVpmlT3yoRdZqvMqffzKiibDlzz95Gmms7V7mctxrhNVOOWTwMSJvk94Y9ZPenljKgPJIV3Z1cqqx+W8JxFFeelOuYvMEe4bOVBh1TepGzzdWVdYbylgXJy35uRTZ2drybUe5+jc0hiAuujHz0zdY1FwOHfwzSsSidlYn4syPeuytnxTzn7fbWXeXetTTtDlmLRf8MBSzXzDFWNH0cNGOCQ=="
}

This seems like exactly what you want. Maybe I am missing something....

@marnee01
Copy link
Contributor Author

marnee01 commented Jan 8, 2025

In your scenario, did you set this up such that your SharedPassword in application-prd.yaml can't be decrypted?

@ryanjbaxter
Copy link
Contributor

What I did was removed the encrypt.* properties from my bootstrap.yaml file and made the request above. As you can see the response returned the encrypted value, which is what I think you want right?

@marnee01
Copy link
Contributor Author

marnee01 commented Jan 8, 2025

The outcome you showed is how I would want it, yes. I thought you were just trying to reproduce the problem, not try to find a solution. We don't have any encrypt.* properties. If we did, we couldn't remove them on demand from the running service when making this call. We could run a modified version of it locally. (That's what we're doing, although we're getting around the decryption a different way, as described above. That's workable, but is not as convenient as being able to put a call to the running service and requesting that it not decrypt through a query param.)

@ryanjbaxter
Copy link
Contributor

I am trying to reproduce it, I guess I still don't understand how to reproduce the problem. In the "lower environment" do you have a difference encryption configuration and that is why it can't decrypt the properties?

@marnee01
Copy link
Contributor Author

marnee01 commented Jan 8, 2025

Yes, that's correct. We have different encryption for Prod than for the lower environments. (I would guess this is pretty common.)

@ryanjbaxter
Copy link
Contributor

This makes more sense now, thanks.

Would a property to disable prefixing the property with invalid and instead returning just the encrypted property work for you?

@marnee01
Copy link
Contributor Author

marnee01 commented Jan 8, 2025

I do believe that would also work.

@ryanjbaxter
Copy link
Contributor

Cool.

One last thing, when I request /application-prd.json with a different encryption key I do see what you said but I also get an invalid element in the json, do you see this as well?

{
    "Property1": "Fill_me_in",
    "SharedPassword": "Fill_me_in",
    "invalid": {
        "SharedPassword": "<n/a>"
    }
}

@marnee01
Copy link
Contributor Author

marnee01 commented Jan 9, 2025

Yes, I do see the invalid element as well.

ryanjbaxter added a commit to ryanjbaxter/spring-cloud-config that referenced this issue Jan 15, 2025
ryanjbaxter added a commit to ryanjbaxter/spring-cloud-config that referenced this issue Jan 15, 2025
Fixes spring-cloud#2632

Signed-off-by: Ryan Baxter <ryan.baxter@broadcom.com>
@ryanjbaxter ryanjbaxter added this to the 4.1.6 milestone Jan 15, 2025
@ryanjbaxter ryanjbaxter changed the title Add query parameter to skip property value decryption (e.g. "decryptProperties=false") Add Ability to skip property value decryption (e.g. "decryptProperties=false") Jan 15, 2025
@github-project-automation github-project-automation bot moved this to Done in 2023.0.6 Jan 15, 2025
@github-project-automation github-project-automation bot moved this to Done in 2024.0.1 Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants