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

whitelist /tmp/apt-key-gpghome.* directory #1000

Merged
merged 2 commits into from Jan 31, 2020

Conversation

tejal29
Copy link
Member

@tejal29 tejal29 commented Jan 25, 2020

Fixes #769

Description
In this PR

  • Whitelist can now accept patterns like /tmp/tdd.*/
  • Add /tmp/apt-key-gpghome.* to whitelist since apt-key add adds temporary files in this directory.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes unit tests
  • Adds integration tests if needed.

See the contribution guide for more details.

Reviewer Notes

  • The code flow looks good.
  • Unit tests and or integration tests added.

Release Notes

Describe any changes here so maintainer can include it in the release notes, or delete this block.

Directories matching `/tmp/apt-key-gpghome.*` pattern are not whitelisted. Dockerfiles which run `apt-key add` will now work.

Copy link
Contributor

@cvgw cvgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we think this will ever need to be configurable? Are we going to break someone's build by whitelisting this?

@tejal29
Copy link
Member Author

tejal29 commented Jan 29, 2020

I don't see the need to make this configurable since users running apt-key add adds the keys in tmp. They get deleted when user runs

apt-get autoclean

or

apt-get clean

It is expected these files get cleaned up.

@cvgw
Copy link
Contributor

cvgw commented Jan 30, 2020

This seems like a reasonable and relatively minor change, but I'm having some trouble understanding it.

My impression is that perhaps apt-get will sometimes delete a file in /tmp which corresponds to a key of sorts? Perhaps the scenario is similar to the follow example;

// layer 0 includes the temporary key for `apt` which is located in /tmp
FROM some-base-image // layer 0
// layer 1 deletes the temp key from layer 0 as part of using `apt`
RUN apt-get update && apt-get install foo // layer 1

Because the temp key from layer 0 is removed (opposed to be marked as deleted with a .wh. file) the LayerMap hits an unexpected error as the file no longer exists.

Is that correct?

@tejal29 tejal29 merged commit 3f73230 into GoogleContainerTools:master Jan 31, 2020
@tejal29
Copy link
Member Author

tejal29 commented Jan 31, 2020

This seems like a reasonable and relatively minor change, but I'm having some trouble understanding it.

My impression is that perhaps apt-get will sometimes delete a file in /tmp which corresponds to a key of sorts? Perhaps the scenario is similar to the follow example;

// layer 0 includes the temporary key for `apt` which is located in /tmp
FROM some-base-image // layer 0
// layer 1 deletes the temp key from layer 0 as part of using `apt`
RUN apt-get update && apt-get install foo // layer 1

Because the temp key from layer 0 is removed (opposed to be marked as deleted with a .wh. file) the LayerMap hits an unexpected error as the file no longer exists.

Is that correct?

Yes. usually folks have apt-key, apt-get and apt-get clean all in one run command.
apt-get update && apt-key add key-from-base-image && apt-get install foo && apt-get clean

We need to debug more on why

  1. kaniko first detects files add in /tmp/apt-get-temp-workdir and
  2. Are the commands still running in background and maybe kaniko detected them as finished
    Hence, files got deleted in between "Taking snapshot" and "Actually adding to layer"

Does that make sense?

@cvgw
Copy link
Contributor

cvgw commented Jan 31, 2020

I guess I'm still unclear about

  1. Where the "key" is coming from (base image, RUN command, etc)
  2. What causes they key to be removed
  3. When the key is removed (during RUN, after layer has finished, etc.)

My impression from your last comment is that perhaps some of these are currently unknown?

@tejal29
Copy link
Member Author

tejal29 commented Jan 31, 2020

I guess I'm still unclear about

  1. Where the "key" is coming from (base image, RUN command, etc)

The keys can come from anywhere. e.g

  • in the command below it was from internet.
  • it can also be a k8 secret.
RUN echo "deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main" > /etc/apt/sources.list.d/chrome.list && \
    curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg |  apt-key add - && \
  1. What causes they key to be removed

Usually, apt-get clean will cause the key to be removed.

  1. When the key is removed (during RUN, after layer has finished, etc.)

My impression from your last comment is that perhaps some of these are currently unknown?

yes. we need to debug more on why was it detected as added.

@tejal29 tejal29 deleted the 769 branch April 26, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate-release cla: yes CLA signed by all commit authors
Projects
None yet
3 participants