christgau / wsdd

A Web Service Discovery host daemon.
MIT License
814 stars 98 forks source link

Revise instructions for installing Debian packages, for added clarity #130

Closed brainchild0 closed 1 year ago

brainchild0 commented 2 years ago

Request clarifies the instructions for package management under Debian and related distributions.

Issue #127 exposed the requirement of root permissions for executing the command for adding the repository key. The issue was marked resolved by showing terminal logs from a shell session running as root. This approach sidesteps the reality that few users conduct regular terminal interaction, or indeed any terminal interactions, in a root shell.

The revised instructions follow a more standard pattern of expressing commands that may be executed from an unprivileged terminal, by acquiring elevated privileges through sudo. It also eliminates the suggestion of manually editing files, in favor of simple automation.

christgau commented 2 years ago

Thanks for your contribution!

I have mixed feelings with this PR: On the one hand, I appreciate to be more clear with the instructions. While I think they are quite obvious for admins, providing command lines for adding the repo key, e.g., might be more expressive and convenient to use. On the other hand, all installation commands from the README require superuser/root permissions - regardless of the distribution in use. Executing the commands without required permissions would end up with error messages which was actually the case in #127.

Noting such errors and acting accordingly is - no offense intended - basic system administration knowledge and IMO the wsdd README is not the right place to start teaching those. Unwarily applying sudo commands from a "random" software's (like wsdd) README file is something I would prefer not to support/encourage.

That being said, I propose to include the command lines but without sudo. One might also consider to simply use stable instead of the manually replaced distro name in the apt source file, but I am not sure whether this is generally a good solution/best practice. Maybe @fxrb or @JedMeister can elaborate on that (if they notice their mentioning here 😉)

brainchild0 commented 2 years ago

While I think they are quite obvious for admins, providing command lines for adding the repo key, e.g., might be more expressive and convenient to use.

It is obvious, but it is also accurate, which is what matters. Mostly everyone is not running a root shell, and anyone who is doing so probably should reconsider. If you provide commands, they should be command that actually will work, not ones that require a mutation omitted because it is considered obvious.

Obvious or not, I fail to find a reason for objecting to publishing documentation that is more accurate.

If nothing else, you would be allowing everyone the convenience of simple copy and paste.

Please note, also, not everyone who installs software is an "admin" in any meaningful sense. I am the "admin" for my own hardware, because I am the only user. If I don't install software, no one will.

Putting the technical questions aside of root versus non-root shell, the language in this section expresses a very disorganized structure. Showing working commands, in clear formatting, in the correct order, is an approach that might be considered a helpful improvement.

brainchild0 commented 2 years ago

Note the format I suggested follows a nearly-universal convention.

It is no wonder I forgot myself to include the sudo, as mostly I am not needing to think of it.

Please refer to one of many available examples.

fxrb commented 2 years ago

Noting such errors and acting accordingly is - no offense intended - basic system administration knowledge and IMO the wsdd README is not the right place to start teaching those. Unwarily applying sudo commands from a "random" software's (like wsdd) README file is something I would prefer not to support/encourage.

I fully agree. Lot's of the systems I manage don't even have sudo installed. Cluttering the README with such instructions is therefor not a good idea.

That being said, I propose to include the command lines but without sudo. One might also consider to simply use stable instead of the manually replaced distro name in the apt source file, but I am not sure whether this is generally a good solution/best practice. Maybe @fxrb or @JedMeister can elaborate on that (if they notice their mentioning here wink)

In my humble opinion using stable as distribution name is somewhat dangerous as its meaning can change, for instance from Buster to Bullseye. This could trigger a probably unwanted upgrade of the entire distribution.

fxrb commented 2 years ago

Issue #127 exposed the requirement of root permissions for executing the command for adding the repository key. The issue was marked resolved by showing terminal logs from a shell session running as root. This approach sidesteps the reality that few users conduct regular terminal interaction, or indeed any terminal interactions, in a root shell.

I consider the terminal log provided by @christgau very helpful as it shows precisely what happens during the installation of wsdd. Including the fact that the user should have root privileges.

brainchild0 commented 2 years ago

Lot's of the systems I manage don't even have sudo installed.

If any are Debian or Ubuntu, then I suspect you had changed the configuration after installation, or had used a non-standard installation process or options selection.

Debian and Ubuntu have moved toward the preference of controlling root access through sudo. The approach offers a safe means for providing fine control over which commands have elevated privileges, and which users may invoke any commands with such privileges, without the requirement of users sharing a password second to their own.

I consider the terminal log provided by @christgau very helpful as it shows precisely what happens during the installation of wsdd. Including the fact that the user should have root privileges.

You may find it helpful, but it's not in the Readme file. Improvements to the Readme file is the purpose the proposed revisions.


The comments broadly seem to sidestep the three issues I would identify as essential.

First is clarity. The current structure of this section is chaotic. Full commands are formatted inline (even while yet others are placed in separate blocks), and mixed into the pros with text that represents only fragments of commands. Full, accurate, functional commands, one per block, or one per line, in an appropriate sequence, is the conventional style of presentation, for good reason.

The second issue is accuracy. Following the instructions as given is ineffective. It is a flimsy defense, of giving inaccurate information, that most receiving the information would recognize the inaccuracy (e.g. "Yeah, but you knew what I meant"). Accuracy requires inserting calls to sudo, if not instead explaining the assumption of using a root shell, which as noted would be more strange to Debian and Ubuntu users.

The final issue is convention. Of all the many installation instructions for Debian and Ubuntu packages I recall following, commands have been given in a literal form that is functional from a non-root shell. Please consider reviewing documentation from other projects, in addition to the project I referenced earlier, to gain a feeling for what what readers of your documentation are likely to expect.

JedMeister commented 2 years ago

In response to @brainchild0 re sudo use:

Mostly everyone is not running a root shell, and anyone who is doing so probably should reconsider.

And more recent post:

Debian and Ubuntu have moved toward the preference of controlling root access through sudo. The approach offers a safe means for providing fine control over which commands have elevated privileges, and which users may invoke any commands with such privileges, without the requirement of users sharing a password second to their own.

I'm not sure where that info is coming from, but that's not the case, at least not with regards to Debian. And security can't be considered a binary "on"/"off". It is a continuum with trade-offs all the way along the line. And sudo itself can be considered a potential point of vulnerability, so to suggest that a system without it is less secure is naive at best.

Certainly use of sudo is one way to minimise the amount of time that root user system access occurs. And there is no argument with regards to the value of sudo on a desktop system, where a user is (almost?) always logged in when the system is running. Even more-so for newb users, although it's well worth noting that lots of damage can still be done, say if the user blindly copy/pastes sudo prefixed malicious instructions. Or even with sudo enabled, a malicious user with access to a sudo enabled account can still do a lot of damage (perhaps even setting up an attack that will run later once the user uses sudo). So it's a tool that has it's use, but iit's certainly no security silver bullet...

In the case of a headless server though, any real value of sudo is contentious and generally comes down to the preference of the administrator. That is because you are only ever logging in to administer the server! In that context, using sudo is more like playing "simon says" than actually giving any security advantage! As @fxrb notes, it's not uncommon on a server for sudo to not even be installed. In fact in my experience, most seasoned Linux admins prefer it without sudo. Instead of suod, the sort of security measures you take would be locked down SSH config (probably only supporting keypair authentication, maybe on a non-standard port), appropriate firewall rules, some sort of brute force protection; such as fail2ban (blocking IPs from repeated failed authentication), no externally accessible processes running as root, etc.

As I noted above, it's a completely different situation on a desktop system. Although even then, by default when installing Debian, if you set a root password, then the main (non-root) user isn't given the sudo privilege! In that scenario, you need to explicitly log into a root terminal (e.g. via tty1).

Unfortunately @christgau, your suggestion:

I propose to include the command lines but without sudo.

is likely to still cause issues to the uninitiated. As you can see in the modifications provided by @brainchild0 - to write the sources.list file, the sudo is prefixed to the tee command. Unfortunately, the uninitiated may not immediately realise where the sudo needs to be injected (after the pipe, rather than at the start of the line).

That could be worked around by providing the command in a slightly different format, such as explicitly via a shell (note I also included the lsb-release command to get the relevant distro codename dynamically). E.g.:

sh -c "echo 'deb https://pkg.ltec.ch/public/ $(lsb_release -sc) main' > /etc/apt/sources.list.d/wsdd.list"

As @fxrb writes:

In my humble opinion using stable as distribution name is somewhat dangerous as its meaning can change, for instance from Buster to Bullseye. This could trigger a probably unwanted upgrade of the entire distribution.

I agree! I'm still running Debian 10/Buster on a few systems. That's now oldstable. So that could certainly cause an issue...

Having said all that, I don't think that there is necessarily an issue with giving explicit copy/paste commands. But if you were to do that, I would ensure that it runs whether sudo exists or not. That would require something like this:

[ "$(whoami)" = "root" ] || SUDO=sudo

Then instead of using sudo in following commands, use $SUDO (so it will only be called when run as a non root user).

Regardless, it should also be noted that apt-key has been deprecated and Bullseye is the last Debian release which will include it. The main reason why it's being deprecated is because adding all keys to one single big fat keyring, then trusting anything signed by any of those keys is a huge potential security risk! Whilst many have moved to manually putting all keys in trusted.gpg.d, that only solves the first half of the issue (any key in trusted.gpg.d will still be trusted for any sources.list entry!). Best practice dictates that any third party key(s) should be saved in their own file and explicitly noted in the relevant sources.list entry. That way, that key will only be trusted in context of that particular repository. E.g.:

# download the key, deamor it (convert from ascii to binary format) and write it to a unique key file
wget -O- https://pkg.ltec.ch/public/conf/ltec-ag.gpg.key  | gpg --dearmor | $SUDO tee /usr/share/keyrings/wsdd.gpg
# add sources.list with explict key location
echo "deb [signed-by=/usr/share/keyrings/wsdd.gpg] https://pkg.ltec.ch/public/ $(lsb_release -sc) main" | $SUDO tee /etc/apt/sources.list.d/wsdd.list

So with this additional context, if best practice is desired, then providing lines to be copy pasted may be a good idea?!

brainchild0 commented 2 years ago

I'm not sure where that info is coming from, but that's not the case, at least not with regards to Debian.

Project-specific guidelines are available for Debian and Ubuntu concerning root access. Debian makes it elective to select a root password in the installation sequence, whereas all fresh Ubuntu systems begin with a locked root account. I may have been too forceful with my original language, failing to respect the variety of opinions on the subject, but the fact remains that use of sudo in preference to elevated shells rests on a secure rational, and represents a firm preference by much of the community. You will see this preference reflected in instructions, especially those directed at Ubuntu systems, with privilege elevation limited to the finest granularity that achieves some result, as with the example of writing wget output through a pipe to sudo tee, while keeping the former process restricted to a regular privilege that would prevent it from writing directly to the target file.

The broader considerations you have raised I think are the ones most relevant, that instructions are most appropriate when they follow the format and conventions familiar to the targeted community and systems, and when they work correctly by direct transfer to the default interactive environment, even by less experienced users, without any alterations, even those whose necessity might be considered "obvious".

brainchild0 commented 2 years ago

Regardless, it should also be noted that apt-key has been deprecated and Bullseye is the last Debian release which will include it.

Yes, good point. I can't speak on behalf of the project maintainers, but it would seem to me a valuable contribution to show the steps using tools that are currently supported by the underlying system.

christgau commented 2 years ago

The comments broadly seem to sidestep the three issues I would identify as essential.

First is clarity. The current structure of this section is chaotic.

"Chaotic" is really a hard description and IMO it does not apply. Also, I don't think that the current description is totally unclear, however...

Full commands are formatted inline (even while yet others are placed in separate blocks), and mixed into the pros with text that represents only fragments of commands. Full, accurate, functional commands, one per block, or one per line, in an appropriate sequence, is the conventional style of presentation, for good reason.

... I agree that the section for Debian/Ubuntu can be improved, but as the reply of @JedMeister has shown it might be not that easy to get a "one-size fits all" command line that works reliably like a snap. For example, the lsb_release -sc returns una on Linux Mint 20.3, but I assume that focal, the Ubuntu release name, is actually the value to be used there. The shown command line does not work on Mint with what is available at the time of writing.

As for the formatting, I don't think there is a need to have one command per block. IMO a block with one command per line would be sufficient as also shown in Github's Markdown documentation.

Besides that, conventions appear to vary. The Debian documentation also shows commands inline in the text and the usage of sudo appears to depend on the chapters in the manual. One also has to know that # indicates a root shell and $ a user shell within the Debian docs. Opposite to that, the Ubuntu docs have sudo in front of almost every command.

The second issue is accuracy. Following the instructions as given is ineffective. It is a flimsy defense, of giving inaccurate information, that most receiving the information would recognize the inaccuracy (e.g. "Yeah, but you knew what I meant").

I agree on the ineffectiveness, that's why I appreciate your efforts with this PR.

Accuracy requires inserting calls to sudo, if not instead explaining the assumption of using a root shell, which as noted would be more strange to Debian and Ubuntu users.

I understand your arguments. Nevertheless, I still wont go into the direction of adding sudo to the commands. Adding a hint on the required privileges would be my preference.

Regardless of the discussion about the style of README for Debian/Ubuntu, the situation changed a little meanwhile. Official Debian and Ubuntu packages are underway. Thus, the README can be simplified.

brainchild0 commented 2 years ago

might be not that easy to get a "one-size fits all" command line that works reliably like a snap.

I think it's not a problem to instruct the user for manual substitution of a distribution name. I understand the demand for accuracy as relative, meaning not supplying an instruction that is less accurate than another possible instruction.

IMO a block with one command per line

Sure.

Besides that, conventions appear to vary.

For configuring repositories in Ubuntu and similar systems, the same convention is nearly universal, with separate lines for adding the key, repository, and updating the cache, each beginning with a call to sudo. Any counter-example would be vert uncommon from my experience.

Nevertheless, I still wont go into the direction of adding sudo to the commands

I suggest following the examples, from other sources, of operations that overall are most similar to the one you are attempting to illustrate. I have referred to examples for installation of package repositories on Ubuntu systems. I think the general Debian documentation that you cited is not the most relevant example.

christgau commented 1 year ago

The README has been updated w.r.t. Debian-based distros. sudo still is and will not be included in the commands, but a note on the required privileges is included.