Ignore permission error when accessing certs.d directory #37619
Conversation
|
Please sign your commits following these rules: $ git clone -b "ignore-permission-error-when-accessing-certs.d-directory" [email protected]:dims/moby.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -fAmending updates the existing PR. You DO NOT need to open a new one. |
GordonTheTurtle
added
dco/no
CertsDir is hardcoded to "/etc/docker/certs.d". On some distros the
/etc/docker directory is locked down (700). So we get a permission
denied error. At this point we can't tell if the directory is actually
present or not. but either way we can't read anything under it.
At this point we have 2 options, we fail immediately which is the
current behavior. The other other option is to ignore the permission
denied and hope that the certs are not needed and try to continue. In
this change, we use the 2nd option to allow the process to continue
("docker manifest create"). Added a debug statement to log if we do have
a permission issue so it's easy to spot by using "-D" on the docker CLI
command line.
This basically helps non-root users be able to execute the "docker
manifest create" without having to "sudo".
Change-Id: I6449372d375bcf67884715caf4ac13c214f135ab
Signed-off-by: Davanum Srinivas <[email protected]>
dims
force-pushed
the
ignore-permission-error-when-accessing-certs.d-directory
branch
from
a602844 to
81fc371
Compare
GordonTheTurtle
removed
the
dco/no
|
Really wondering if this is the correct approach; note that ping @clnperez @silvin-lubecki PTAL |
|
@thaJeztah i am happy to re-work this any way you recommend. |
What happens if the certs are needed? Does it fail too? Or do we have a risk to silently fallback on an unsecured connexion? |
|
@silvin-lubecki if the certs are needed (required by server) then the connection will fail i believe. |
The cli vendors in some docker/moby registry code so we do get some of the engine logging. |
|
Also, as seen in docker/cli#1358, the permissions error is logged by the manifest command. For other things, though, this might be useful? |
|
#37847 was merged, but let's discuss the logging here |
Codecov Report
@@ Coverage Diff @@
## master #37619 +/- ##
==========================================
- Coverage 35.85% 35.61% -0.24%
==========================================
Files 606 611 +5
Lines 44704 44965 +261
==========================================
- Hits 16029 16016 -13
- Misses 26466 26732 +266
- Partials 2209 2217 +8 |
|
Ack @thaJeztah, thanks! @thaJeztah @clnperez so what can i change here in this PR? |
|
Re-reading, I don't think we should ignore the permission error. Logging it more explicitly if it's a permission error could be good, in case the caller does not, since that's something a user can fix themselves. I think it's up the the caller to decide whether or not to ignore the error or not. BTW, I submitted a PR for |
|
With #37847, and docker/cli#1378, perhaps we don't need to make changes. From the linked issue, the error message would look like this; $ docker manifest create gcr.io/kubernetes-e2e-test-images/net gcr.io/kubernetes-e2e-test-images/net-ppc64le gcr.io/kubernetes-e2e-test-images/net-arm64 gcr.io/kubernetes-e2e-test-images/net-arm gcr.io/kubernetes-e2e-test-images/net-amd64
open /etc/docker/certs.d/gcr.io: permission deniedWhich (I think) is an acceptable error |
|
@thaJeztah #37847 and docker/cli#1378 are merged so can we close this one? |
|
@olljanat sounds good to me! thanks |
|
Yes, I think we should close this one. Thanks so much for working on this @dims! Sorry that it didn't get merged, but it's really appreciated 🤗 |
|
@thaJeztah my pleasure! i am glad it help nudge :) |
8 participants
- What I did
CertsDir is hardcoded to "/etc/docker/certs.d". On some distros the
/etc/docker directory is locked down (700). So we get a permission
denied error. At this point we can't tell if the directory is actually
present or not. but either way we can't read anything under it.
Please see docker/for-linux#396 for a detailed description
- How I did it
At this point we have 2 options, we fail immediately which is the
current behavior. The other other option is to ignore the permission
denied and hope that the certs are not needed and try to continue. In
this change, we use the 2nd option to allow the process to continue
("docker manifest create"). Added a debug statement to log if we do have
a permission issue so it's easy to spot by using "-D" on the docker CLI
command line.
- How to verify it
This basically helps non-root users be able to execute the "docker
manifest create" without having to "sudo".
- Description for the changelog
Allow non-root users to run some CLI commands even when /etc/docker/certs.d/ directory is not accessible
- A picture of a cute animal (not mandatory but encouraged)