Skip to content

crane: Support configuring unqualified search registries instead of only docker.io #1364

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

Open
trevor-vaughan opened this issue May 12, 2022 · 8 comments
Labels
bug Something isn't working lifecycle/frozen

Comments

@trevor-vaughan
Copy link

Describe the bug

Running crane commands with typos leaks data to docker.io

To Reproduce

Run crane ls secretprojectname

Expected behavior

crane does not submit secretprojectname to docker.io

Actual behavior

Error: reading tags for secretprojectname: GET [https://index.docker.io/v2/library/secretprojectname/tags/list?n=1000](https://www.google.com/url?q=https://index.docker.io/v2/library/secretprojectname/tags/list?n%3D1000&sa=D&source=buganizer&usg=AOvVaw3EOpvge9bXCmcd-c9Kw34y): UNAUTHORIZED: authentication required; [map[Action:pull Class: Name:library/secretprojectname Type:repository]]

Additional context

It is common for CI/CD systems to use variables in place of hostnames. Should one of these variables be misconfigured, then sensitive internal project names may be sent to docker.io without the users knowledge. Additionally, the hostnames themselves could be sensitive and a misconfiguration on the part of the project name could lead to the hostname leaking to docker.io.

This maps directly to OWASP M4: Unintended Data Leakage

Proposed Solution:

Ensure that the user must specify a default repository on all operations. Supporting a configuration file would be ideal.

  • Output of crane version: v0.8.0
  • Registry used: Internal
@trevor-vaughan trevor-vaughan added the bug Something isn't working label May 12, 2022
@imjasonh
Copy link
Collaborator

Thanks for filing this issue.

Would you expect crane ls secretprojectname to fail with an invalid repo name error instead?

In Docker terms, a regular string like secretprojectname is normally interpreted to mean a repo name in the default repo of the default registry, which is by convention index.docker.io/library (I don't love it either, but here we are). So crane expands it into index.docker.io/library/secretprojectname and lists tags for that. This enables crane ls ubuntu for example.

We could make crane ls, and all crane surfaces, require full repo names (i.e., use name.StrictValidation instead of name.WeakValidation), but it would be a breaking change for folks that are calling crane ls ubuntu today.

It might be worthwhile to add a --strict flag (name TBD) to crane surfaces to require strict name validation, but I'd be hesitant to make that the default option at least for now.

@trevor-vaughan
Copy link
Author

@imjasonh I definitely get that it's a convention with all of the libraries and I absolutely hate it due to the potential data leaks for no real gain.

Honestly, if crane just had a configuration file where docker.io is the default and I can change it to some other default, I'd be happy with that and it would be backward compatible.

@trevor-vaughan
Copy link
Author

@imjasonh FWIW, I'm OK with something like a --strict flag as well because it's easy enough to update scripts to use that and it could be changed to --no-strict at some point.

@imjasonh
Copy link
Collaborator

Honestly, if crane just had a configuration file where docker.io is the default and I can change it to some other default, I'd be happy with that and it would be backward compatible.

This could work. I'd want to make sure we don't accidentally load and parse that file every time we parse a reference, but that feels doable.

Is there any prior art for this in Podman maybe today? I know they're also sometimes on a warpath about dockerisms leaking all over the place, I'd be surprised if they didn't have some solution for this we could steal gain inspiration from.

@trevor-vaughan
Copy link
Author

I'm a long time podman user and they use the unqualified-search-registries from /etc/containers/registries.conf.

Following that path would be an awesome solution since it would just fall in line with a lot of other generic tools.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Keep fresh with the 'lifecycle/frozen' label.

@trevor-vaughan
Copy link
Author

Dear Stale Issues Bot,

I'd still like this.

@imjasonh
Copy link
Collaborator

I think I'd be okay having crane respect unqualified-search-registries in /etc/containers/registries.conf.

We've had similar requests for configuring mirror repos (#1200), which I see registries.conf also supports.

There don't seem to (currently) be too many options in that config that we'd have to worry about it being a burden supporting them all, or documenting what we do/don't support.

If someone wants to pick this up I'd be happy to review a PR for it.

@imjasonh imjasonh changed the title crane: M4: Unintended Data Leakage => The defaults for go-containerregistry lead to unintended data leakage to docker.io with malformed commands crane: Support configuring unqualified search registries instead of only docker.io Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lifecycle/frozen
Projects
None yet
Development

No branches or pull requests

2 participants