Add pam-devel and its dependencies pam, audit-libs, cracklib, cracklib-dicts, and libpwquality

thomann commented 2 years ago

Please see the repo readme for directions on how to make PRs on this repo.

Checklist:

[x] if you have added a CDT, it appears in the cdt_slugs.yaml file and you have rerun the script python gen_cdt_recipes.py.
[x] if you have changed the CDT generator code (rpm.py), you have bumped the build number in conda_build_config.yaml and have remade all of the recipes via running python gen_cdt_recipes.py --force
[x] if you have added a custom CDT recipe, you have added the name of the CDT with custom: true in the cdt_slugs.yaml file.
[x] all CDT recipes have build number set by {{ cdt_build_number }} for old-style/legacy CDTs or {{ cdt_build_number|int + 1000 }} for new-style CDTs
[x] if you see a warning about a CDT not having a license, you have added the license_file key in the cdt_slugs.yaml file with the path to the appropriate license in licenses/

NOTE: If you make any changes to cd_slugs.yaml, you need to reun the generator code via python gen_cdt_recipes.py.

thomann commented 2 years ago

Hello, we need this to add RStudio as a conda-forge package https://github.com/conda-forge/staged-recipes/pull/13760/

isuruf commented 2 years ago

Can you explain why these cannot be conda packages?

isuruf commented 2 years ago

I mean why do they have to be CDTs instead of regular conda packages?

thomann commented 2 years ago

Thanks @isuruf for the quick answer!

I'm not sure - they are cdt's in pkgs/main and we used them as such before conda-forge dropped the defaults channel last month.

You think they would better be conda packages?

isuruf commented 2 years ago

You think they would better be conda packages?

I have no idea. Sorry. I thought you would have an idea.

thomann commented 2 years ago

@isuruf I'm not whether it is (easy enough) possible to implement pam-devel and it's dependencies directly as conda-packages. It used to be usable as cdt() until a month ago, maybe then it is ok? Probably the cdt-builds team has to decide.

(bringing @h-vetinari into the loop)

h-vetinari commented 2 years ago

@thomann: @isuruf I'm not whether it is (easy enough) possible to implement pam-devel and it's dependencies directly as conda-packages.

Are there some guidelines how to determine cdt-vs-regular packages? The documentation is a bit sparse on that, but I guess it's mostly around things having to do with libGL?

Would you prefer to attempt to package these things as regular packages, @isuruf?

@thomann: It used to be usable as cdt() until a month ago, maybe then it is ok? Probably the cdt-builds team has to decide.

Is there anyone else we should be bringing into this discussion, @isuruf?

[Sidenote: Isuru is one of the key people in conda-forge, so he'll likely be involved in such decisions, if he wants to. 🙃]

@thomann: (bringing @h-vetinari into the loop)

Thanks!

beckermr commented 2 years ago

So cdts are for things where building in conda is very impractical / difficult or we always want to use the system version of a package.

The basic guideline is to never use a CDT unless you absolutely have to.

When I moved the CDT builds from defaults to here, I made CDTs for everything we used at the time. Defaults must have added more in the meantime and we didn't notice until we dropped the channel as you say.

I'd ask you all to take a close look at the underlying software before we merge this and figure out why it is hard to build or w/e.

Maybe the anaconda folks have some insight too? @chenghlee?

cc @conda-forge/core for viz

chenghlee commented 2 years ago

If I remember correctly, we decided on a libpam CDT for defaults because:

convenience; and
PAM interacts with pretty heavily with the underlying system configuration and security, and at the time, we didn't have the resources to work through how to build conda package(s) in a safe and responsible way.

beckermr commented 2 years ago

@isuruf what do you think?

h-vetinari commented 2 years ago

@isuruf: I mean why do they have to be CDTs instead of regular conda packages?

To find out (and hopefully move forward one way or another), I opened https://github.com/conda-forge/staged-recipes/pull/16911, though I'm pretty quickly reching the limits of my make-fu.

I also think the following comment above is pretty pertinent if we want to consider packaging it ourselves:

@chenghlee: PAM interacts with pretty heavily with the underlying system configuration and security, and at the time, we didn't have the resources to work through how to build conda package(s) in a safe and responsible way.

sshockwave commented 1 year ago

Hi everyone. Please excuse me for bringing up an old issue. I had another attempt on the regular packaging: https://github.com/conda-forge/staged-recipes/pull/21955. The build is "passed" from a tech perspective, but I don't have the expertise to spot the security issues. I would appreciate some reviews and comments!

h-vetinari commented 1 year ago

Quoting from https://github.com/conda-forge/staged-recipes/pull/21955 (respin of https://github.com/conda-forge/staged-recipes/pull/16911), we finally seem to have direction to move forward with this:

@carterbox: @sshockwave, I talked with some of the core team members at the community meeting today. It seems that a majority would prefer that this package is shipped as a CDT.

As a side note, there were many questions about what downstream conda packages would need to link to libpam (i.e. what is the utility of this package for conda) and even whether this non-CDT libpam package would function on non-selinux distros. Please be prepared to answer these types of questions, so the core team can help package this library appropriately.

Also copying my answer w.r.t. to the last bit:

The main reason we started down this road was to package rstudio-server, I'm not currently aware of other packages that require this.

h-vetinari commented 1 year ago

Ah, just saw a linked PR above that also seems to need it: jupyter-desktop-server...

beckermr commented 1 year ago

I think we're resolved to merge this once it passes and I have a chance to do some review.

beckermr commented 1 year ago

Thanks everyone for working through this. A full year is not great for a pr but we got to the end!

isuruf commented 1 year ago

I think we're resolved to merge this once it passes and I have a chance to do some review.

I thought the consensus was that we needed more information either way.

beckermr commented 1 year ago

Ah I thought the messages above provided that information. Happy to wait too.

jakirkham commented 1 year ago

Yeah my impression was we were leaning towards CDTs, but we also want to know what the use cases are. Sounds like Daniel articulated that we have questions here, but I am not seeing the answers (though could be missing this)

For example one concern is SELinux only features might be in use, which would impact how distributable/reusable downstream packages would be (even when built with CDTs)

Another relates to how some of these packages contain functionality to alter system level items (see sbin binaries). So we would like to understand if those components are needed (and if so how they are used)

cc @chenghlee (in case there are other questions I've forgotten)

beckermr commented 1 year ago

The cdts are for rstudio-server and jupyter-desktop-server per the comments above. I think this was the main question on the call, namely what we were trying to build.

jakirkham commented 1 year ago

Knowing what uses them is helpful. Knowing how they are used is still a question worth answering, which would in turn shed light on what is actually needed from the CDTs

beckermr commented 1 year ago

Sure ok. For that we'll to chat with the upstream devs. Or maybe we just merge them as is since it matches anaconda and unblock these packages?

h-vetinari commented 1 year ago

Knowing how they are used is still a question worth answering, which would in turn shed light on what is actually needed from the CDTs

From here:

RStudio Server uses PAM to authenticate users. Some Unix systems (such as Debian and Ubuntu) use default PAM settings for applications which aren't explicitly registered with PAM, so don't require additional PAM configuration. If however your system requires explicit registration (i.e. Redhat, Fedora, openSUSE) then you need to add an /etc/pam.d/rstudio file to your configuration.

It would be quite a PITA for using a conda-packaged rstudio to have to configure everything twice (once for the system, once for conda-forge) to get the user authentication working correctly. And given how sensitive user authentication is (basically the golden ticket, if it goes wrong), that's another reason I'd rather not package this.

FWIW, there are a bunch of hits for "pam" in the (public version of the) rstudio repo, but none for "selinux".

beckermr commented 1 year ago

Seems like a CDT to me.

isuruf commented 1 year ago

As I mentioned in the core call, we could package libpam as a conda package with the system configuration directory /etc/pam.d, so then there'll be no conda environment specific configuration.

So, Rstudio-server is something that needs to be run as root?

h-vetinari commented 1 year ago

As I mentioned in the core call, we could package libpam as a conda package with the system configuration directory /etc/pam.d, so then there'll be no conda environment specific configuration.

I couldn't join the call today, sorry. I don't know the background that's gone into the "absolutely-no-new-CDTs" stance, but from what little I know, it seems like a bad trade-off to me in this case (risk of security-relevant breakage and bad UX in exchange for what exactly...?).

I'm not sure if someone else but you could pull off what you describe sanely and maintainably. I certainly won't complain if there's a usable libpam in conda-forge that unblocks the rstudio-server effort, but it's not great to leave contributors (especially new ones like @thomann here who wanted to help on rstudio-server specifically) hanging in limbo for months on end without a concrete path forward.

So, Rstudio-server is something that needs to be run as root?

Generally yes (though it can be and often is encapsulated within a container where necessary). As an example, see these docs, where everything starts with sudo rstudio-server ....

isuruf commented 1 year ago

I explained my thoughts on CDTs at https://github.com/conda-forge/conda-forge.github.io/issues/1904

risk of security-relevant breakage

This is just FUD. If a user installed library can muck with security that's a serious security issue and please open a CVE for it.

I'm not sure if someone else but you could pull off what you describe sanely and maintainably. I certainly won't complain if there's a usable libpam in conda-forge that unblocks the rstudio-server effort, but it's not great to leave contributors (especially new ones like @thomann here who wanted to help on rstudio-server specifically) hanging in limbo for months on end without a concrete path forward.

It's the job of the contributor to convince us and not the other way around. I don't like reasons like it's hard to do or I don't know how. Those are not reasons to use CDTs.

isuruf commented 1 year ago

As I mentioned in the core call, we could package libpam as a conda package with the system configuration directory /etc/pam.d, so then there'll be no conda environment specific configuration.

Turns out this is what is done by the now closed staged-recipes PR. This seems like a good option.

isuruf commented 1 year ago

Hmm, that's not entirely true. From my conda-forge.github.io comment

The issue is that the pluggable modules live in a distro specific location. For eg: /usr/lib/x86_64-linux-gnu/security/. The default modules are built into the conda package in $CONDA_PREFIX/lib/security, but custom ones for system-wide configuration are installed into /usr/lib/x86_64-linux-gnu/security/. So, we would need to patch the module to look into both, but the directory /usr/lib/x86_64-linux-gnu/security/ is distro specific and will be hard to

h-vetinari commented 1 year ago

risk of security-relevant breakage

This is just FUD. If a user installed library can muck with security that's a serious security issue and please open a CVE for it.

If you mean OS-level security, then sure, that would need privilege escalation. But the point is that from the POV of the environment that distinction can become meaningless, i.e. if a given deployment always runs from a conda-env, it will not ever see what the system-level would do, as we'd reroute to our implementation. If we mess up any step of the way, one can still end up with privilege escalation as seen from within the environment.

Additionally, for apps like rstudio-server that need to run with root privileges, we don't have the comfort of the user-access model to protect us from potential mistakes.

h-vetinari commented 6 months ago

This came up in the core call in context of #66 today. I brought it as an example of something that should be a CDT, not least due to the security implications and infeasibility of properly testing this. There was general agreement on having libpam as a CDT (particularly from @mariusvniekerk, who seems to know libpam quite well), and no disagreement.

So I'd like to unblock this PR, independently of the alma8 enablement effort.

zmbc commented 3 months ago

Don't want to add noise here, but I would really appreciate a rstudio-server conda package and was surprised to see that it was blocked by this. It sounds like there is broad agreement that this should be a CDT. What are the next steps to get this merged?

h-vetinari commented 11 hours ago

@beckermr, the builds were hanging and I managed to solve this by adding a swapfile, except for the two altarch legacy jobs: cos7_{aarch64,ppc64le}_legacy, which seem to hang on

building recipes:   0%|                                 | 0/175 [11:43<?, ?it/s]

even with 30GB swapfile. Do we still need the legacy CDTs? If so, would you know how to debug/fix what's happening?

beckermr commented 11 hours ago

We can drop all of the cos6 and legacy cdt builds.

h-vetinari commented 8 hours ago

So I cleaned up this long-standing PR (we had agreed to make pam a CDT in a core-call in March), which needed a bit of work because the CentOS 7 repos aren't online anymore. While we're at it (and on @beckermr's hint), I also dropped cos6 & legacy CDTs. Plus some clean-ups w.r.t. skips, configs & scripts. Given the large mechanical changes, it's probably easiest to review per commit.

I have some follow-up work to add some (limited) Alma 8 CDTs on top of this.

PTAL @conda-forge/core

conda-forge / cdt-builds

Add pam-devel and its dependencies pam, audit-libs, cracklib, cracklib-dicts, and libpwquality #55