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

[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

[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

[NicolasGoeddel]

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

[NicolasGoeddel]

Do you know it now?