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

Include source of invalid base64 data in error messages #1469

Merged
merged 1 commit into from
Apr 8, 2021

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Apr 8, 2021

Description of changes:

Include source of invalid base64 data in error messages
    
The user data content could be sensitive.

Note for host-containers/bootstrap-containers: we don't save the output of restart-commands (including host-containers) today, so this is just to make sure we don't forget this if we do start to save it later.

Note for all of them: these shouldn't even be possible to hit with today's API protections, so again it's just being safe; see testing note...

Testing done:

These have the same problem, in that it's not really possible to hit the "invalid base64" error case without removing a bunch of protections in apiserver. The base64 has to be valid to be accepted in the first place, and has to be valid later on for the apiserver to deserialize it from disk and give it to these programs. Given the tiny changes and the strengths around this, I'm not sure it's worth ripping up everything else..?

I confirmed that it builds, tests pass, an AMI worked fine, admin and control host containers still worked fine.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@tjkirch tjkirch requested a review from bcressey April 8, 2021 21:13
@tjkirch tjkirch added this to the 1.0.8 milestone Apr 8, 2021
The user data content could be sensitive.
@tjkirch tjkirch changed the title host-containers: log name of container if given invalid base64 user data Include source of invalid base64 data in error messages Apr 8, 2021
@tjkirch
Copy link
Contributor Author

tjkirch commented Apr 8, 2021

^ This push cleans up everywhere we handle a base64::DecodeError to print some useful source information (like container name, template name, etc.) rather than the base64 content.

@tjkirch tjkirch requested review from jpculp and bcressey April 8, 2021 22:08
@tjkirch tjkirch merged commit 8337c51 into bottlerocket-os:develop Apr 8, 2021
@tjkirch tjkirch deleted the host-containers-log-name branch April 8, 2021 22:35
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

Successfully merging this pull request may close these issues.

3 participants