Reprehensibly insecure installation instructions

[jbrucker]

Reprehensibly insecure installation instructions

[williammartin]

Hi @jbrucker, thanks for opening an issue to try and improve the CLI.

Firstly,

Did someone from Microsoft write this stuff?

While I'm sure that quipping makes you feel good, it's not going to advance the technical discussion, so I'd ask that you leave it out. We're all here to try to make gh a valuable piece of software, otherwise I'm sure you wouldn't have taken the time to open this issue.

Now, onto the actual details. The original authors of these instructions are long gone so let's work together and make sure this gets the attention it needs. Firstly, I assume you're talking about this snippet:

type -p curl >/dev/null || (sudo apt update && sudo apt install curl -y)
curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg \
&& sudo chmod go+r /usr/share/keyrings/githubcli-archive-keyring.gpg \
&& echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null \
&& sudo apt update \
&& sudo apt install gh -y

So let's talk about your specific concerns:

completely trust whatever junk you download via a pipe

I assume what you're getting at here is that there is no verification that the contents of /usr/share/keyrings/githubcli-archive-keyring.gpg are what the user expects? Can you be more specific about the security implications of this file containing unexpected data (noting that to do so would require either for a user to ignore certificate errors, to have a broken trust store, or for https://cli.github.com to be compromised, which also serves the packages themselves).

make his system less secure by adding a keyring with global write permisions

I don't see this. The keyring is made world readable in chmod go+r /usr/share/keyrings/githubcli-archive-keyring.gpg which was a response to systems that have umask set to 600. The documentation for signed-by states that everyone should have read permissions on these.

It is specified as a list of absolute paths to keyring files (have to be accessible and readable for the _apt system user, so ensure everyone has read-permissions on the file) and fingerprints of keys to select from these keyrings.

install the 'gh' package and dependencies using the -y flag so the poor user has no chance to change his mind after seeing what will be installed.

I can see why this would have some merit. I don't think it would be a big deal to remove this and allow the user a moment to double check.

pipe output to /dev/null so the user cannot see the output

Are you talking about this line?

echo "deb [arch=$(dpkg --print-architecture) signed-by=/usr/share/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list > /dev/null

What would you expect to happen here if the pipe to /dev/null was removed? The only thing you'd see would be the line that is being echoed with the evaluation of dpkg --print-architecture being included right? I don't really see what the security implications of this are.

Cheers.

[andyfeller]

@jbrucker : I'm closing this issue for now due to inactivity. If you feel there is more to contribute to the conversation, please capture your ideas on how to address these concerns in a practical way as well as review our Code of Conduct.