$KREW_ROOT/store/<plugin>/* has wrong permissions, 0700 instead of 0755

[NicolasGoeddel]

Hi there,

I tried to install krew using a different KREW_ROOT, exactly like documented here: https://krew.sigs.k8s.io/docs/user-guide/advanced-configuration/

I basically ran this command from a sudo user:

sudo KREW_ROOT=/usr/local/krew ./krew-linux_amd64 install krew

After that the binary was not accessible from a normal user. This is because the symlink pointed to a file that was only accessible from root:

ls -l /usr/local/krew/bin/
lrwxrwxrwx 1 root root   38 Sep 11 18:42 kubectl-krew -> /usr/local/krew/store/krew/v0.4.4/krew

ls -l /usr/local/krew/store/krew
total 12
drwx------ 2 root root 4096 Sep 11 18:42 v0.4.4/

After a sudo chmod 750 /usr/local/krew/store/krew/v0.4.4 it worked fine.

Unfortunately this happens with every plugin that is installed this way. The subdirectories in /usr/local/krew/store/*/v* always have the wrong permissions.

For example I installed resource-capacity from the sudo user like this:

sudo PATH="/usr/local/krew/bin:$PATH" KREW_ROOT=/usr/local/krew kubectl krew install resource-capacity

Then the new directory had the wrong permissions again:

sudo ls -l /usr/local/krew/store/resource-capacity
total 4
drwx------ 2 root root 4096 Sep 11 18:54 v0.7.4

[ahmetb]

Feel free to dig into the code to figure out why this might be happening. However, I'm not seeing (in a setup with default paths) that .krew/store directories have drwx------.

ls -l $HOME/.krew/store
total 0
drwxr-xr-x@ 3 abalkan  abalkan  96 Jun  7 15:47 access-matrix
drwxr-xr-x@ 3 abalkan  abalkan  96 Aug 16 22:19 ca-cert
drwxr-xr-x@ 3 abalkan  abalkan  96 Jul 14 12:03 deprecations
drwxr-xr-x@ 3 abalkan  abalkan  96 May 23 22:35 edit-status

This might have something to do with sudo command or something else in the picture masking the provided chown flags.

/kind support

[NicolasGoeddel]

@ahmetb Thanks for your answer.

My Go skills are not that good anymore. I coded something way back in 2011 at university. Can you maybe point me to the right source file that creates these paths? I will also do more tests with that. Maybe directly as root user and not with sudo. Maybe the real and effective UIDs have something to do with that? I don't know, I am just guessing.

[NicolasGoeddel]

I created a complete script so it can be tested very easily. It downloads the installer, installs krew, shows the content of ${KREW_ROOT}/store/krew and deletes everything again.

#!/bin/bash

KREW_ROOT=/usr/local/krew
KREW_URL="https://github.com/kubernetes-sigs/krew/releases/latest/download/krew-linux_amd64.tar.gz"
KREW_INSTALL_BIN_FILENAME="krew-linux_amd64"


# END OF CONFIG
set -e

if (( "$(id -u)" != 0 )); then
    echo "Please run as root." >&2
    exit 1
fi

TMP_DIR="$(mktemp -d)"
KREW_INSTALL_BIN="${TMP_DIR}/${KREW_INSTALL_BIN_FILENAME}"

# Download
curl -sL "${KREW_URL}" | tar -C "${TMP_DIR}" --gzip --file=- --extract "./${KREW_INSTALL_BIN_FILENAME}" 

export PATH="${KREW_ROOT}/bin:$PATH"
export KREW_ROOT

${KREW_INSTALL_BIN} install krew

ls -la "${KREW_ROOT}/store/krew/"

rm -rf "${TMP_DIR}"
rm -rf "${KREW_ROOT}"

The output on my machine is:

Adding "default" plugin index from https://github.com/kubernetes-sigs/krew-index.git.
Updated the local copy of plugin index.
Installing plugin: krew
Installed plugin: krew
\
 | Use this plugin:
 | 	kubectl krew
 | Documentation:
 | 	https://krew.sigs.k8s.io/
 | Caveats:
 | \
 |  | krew is now installed! To start using kubectl plugins, you need to add
 |  | krew's installation directory to your PATH:
 |  | 
 |  |   * macOS/Linux:
 |  |     - Add the following to your ~/.bashrc or ~/.zshrc:
 |  |         export PATH="${KREW_ROOT:-$HOME/.krew}/bin:$PATH"
 |  |     - Restart your shell.
 |  | 
 |  |   * Windows: Add %USERPROFILE%\.krew\bin to your PATH environment variable
 |  | 
 |  | To list krew commands and to get help, run:
 |  |   $ kubectl krew
 |  | For a full list of available plugins, run:
 |  |   $ kubectl krew search
 |  | 
 |  | You can find documentation at
 |  |   https://krew.sigs.k8s.io/docs/user-guide/quickstart/.
 | /
/
total 12
drwxr-xr-x 3 root root 4096 Sep 13 13:59 .
drwxr-xr-x 3 root root 4096 Sep 13 13:59 ..
drwx------ 2 root root 4096 Sep 13 13:59 v0.4.4

As you can see the directory v0.4.4 has the wrong permissions. It should have at least drwxr-xr-x in my opinion.

[NicolasGoeddel]

By replacing the line

${KREW_INSTALL_BIN} install krew

with

strace -ff --output ${KREW_INSTALL_BIN_FILENAME} ${KREW_INSTALL_BIN} install krew

I found out that at some point during the installation process a folder created in /tmp simply gets renamed to /usr/local/krew/store/krew/v0.4.4 without setting the permissions again:

mkdirat(AT_FDCWD, "/tmp/krew-temp-move420776093", 0700) = 0
newfstatat(AT_FDCWD, "/tmp/krew-downloads3998705052/krew-linux_amd64", {st_mode=S_IFREG|0755, st_size=12384307, ...}, 0) = 0
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093", {st_mode=S_IFDIR|0700, st_size=4096, ...}, 0) = 0
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093/krew", 0xc000120858, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093/krew", 0xc000120928, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
renameat(AT_FDCWD, "/tmp/krew-downloads3998705052/krew-linux_amd64", AT_FDCWD, "/tmp/krew-temp-move420776093/krew") = 0
newfstatat(AT_FDCWD, "/tmp/krew-downloads3998705052/LICENSE", {st_mode=S_IFREG|0644, st_size=11358, ...}, 0) = 0
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093", {st_mode=S_IFDIR|0700, st_size=4096, ...}, 0) = 0
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093/LICENSE", 0xc000120d38, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/tmp/krew-temp-move420776093/LICENSE", 0xc000121078, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
renameat(AT_FDCWD, "/tmp/krew-downloads3998705052/LICENSE", AT_FDCWD, "/tmp/krew-temp-move420776093/LICENSE") = 0
newfstatat(AT_FDCWD, "/usr/local/krew/store/krew/v0.4.4", 0xc0001213b8, 0) = -1 ENOENT (No such file or directory)
newfstatat(AT_FDCWD, "/usr/local/krew/store/krew/v0.4.4", 0xc000121898, AT_SYMLINK_NOFOLLOW) = -1 ENOENT (No such file or directory)
renameat(AT_FDCWD, "/tmp/krew-temp-move420776093", AT_FDCWD, "/usr/local/krew/store/krew/v0.4.4") = 0
unlinkat(AT_FDCWD, "/tmp/krew-temp-move420776093", 0) = -1 ENOENT (No such file or directory)
unlinkat(AT_FDCWD, "/tmp/krew-temp-move420776093", AT_REMOVEDIR) = -1 ENOENT (No such file or directory)

And as you can see in the very first line of that snippet, the temporary folder that later is renamed was created with the permissions 0700. So I don't think it's only an issue on my side. It's clearly a problem of the installation itself.

[NicolasGoeddel]

I guess the line in question is this one:

        tmp, err := os.MkdirTemp("", "krew-temp-move")

Renaming/copying then happens here:

	err = os.Rename(from, to)
	// Fallback for invalid cross-device link (errno:18).
	if isCrossDeviceRenameErr(err) {
		klog.V(2).Infof("Cross-device link error while copying, fallback to manual copy")
		return errors.Wrap(copyTree(from, to), "failed to copy directory tree as a fallback")
	}

So I think at this point it would be good to also call a os.chmod(to, 0o755) right after.

[NicolasGoeddel]

The background is that a default mktemp CLI command usually sets 0700 for directories and 0600 for files. And it looks like golang does the same.

[ahmetb]

🤔 but why do dirs in ls -l $HOME/.krew/store have 0o755 right now?

[NicolasGoeddel]

On my system it has not. I just installed it without setting KREW_ROOT before.

ngoeddel@stretched-b1:~/bin$ ll ~/.krew/store/krew/
total 12
drwxr-xr-x 3 ngoeddel ngoeddel 4096 Sep 13 16:38 ./
drwxr-xr-x 3 ngoeddel ngoeddel 4096 Sep 13 16:38 ../
drwx------ 2 ngoeddel ngoeddel 4096 Sep 13 16:38 v0.4.4/

I can only think of differences in the underlying operating system. Maybe there are settings for creating temporary folders somewhere?

At least I found this statement:

For GLIBC, versions 2.0.6 and earlier, the file is created with permissions 0666; for GLIBC, versions 2.0.7 and later, the file is created with permissions 0600. On NetBSD, the file is created with permissions 0600. This creates a security risk in that an attacker will have write access to the file immediately after creation. Consequently, programs need a private version of the mkstemp() function in which this issue is known to be fixed.

[NicolasGoeddel]

Btw this is my system:

Distributor ID:	Ubuntu
Description:	Ubuntu 20.04.4 LTS
Release:	20.04
Codename:	focal

[ahmetb]

Ah I see, the dirs directly under store/<plugin> are ok but the dirs like store/<plugin>/<ver> are not. Yeah I think this is a bug. /kind bug

[NicolasGoeddel]

Do you know when this will be solved and included in the next release?

[NicolasGoeddel]

Do you know it now?

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[NicolasGoeddel]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[ngoeddel-openi]

/remove-lifecycle stale