Giter Club home page Giter Club logo

Comments (21)

coastalwhite avatar coastalwhite commented on August 27, 2024 3

Hey!

I would love to co-maintain these crates. I will have a look how to use bindgen to create a complete cover for both Linux-PAM, OpenPAM and any possible other implementations 😉.

I saw that greetd also uses pam-sys, they seemingly ran into the same problem I did a bit ago that the pam crate does some weird things with the environment.

I would really like to keep using bindgen here, to keep this crate as simple as (reasonably) possible.

I agree. I manually did it in libpam-sys, which allowed me to have fine control over all things that were generated. I do think we want to provide the values of linux-pam and openpam in a submodule and then have the "natural default" in root module. This natural default should probably be changable by an environment variable or possibly by a cargo feature. One problem I see with this is that we need a way to disable the module that is not the default since they symbols will be unresolvable.

Any API improvements in pam are greatly appreciated as I only briefly explored the login manager use-case and the API might not satisfy other scenarios.

I have been trying a bit on my own to come up with a better API for PAM clients. Specifically, I wanted to make PAM conversations easier, more secure and more expandable. I have a working prototype which can properly detail with org domains, username, password and OTP tokens. I will see what I can do to get it in a bit more presentable of state. Then you can see whether you want to merge the pam crate or whether you would rather have that be something separate.

One problem is that I currently don't really deal with modules. I am not sure about that for the moment.

Regarding the linux-pam vs open-pam issue. I think we agree on just exposing the bindings as they are and leave ?
another crate figure out the differences between both. Am I right?

Yes, I would agree with this statement.

In general, I will have a look at improving these bindings and then improving the API of the pam crate. I will keep you up to date.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024 1

Here is the current status of my investigations as of so far.

The OpenPAM support is a difficult problem. I got started with the issue, and there are numerous downsides to all solutions.

In essence, it is a not a "multiplatform" support problem, but a "multilibrary" support problem. I have tried this in a bunch of different ways, and I have come to the conclusion that having one pam-sys crate does not really make too much sense. Linux PAM and OpenPAM are just two different libraries. And although they share some shared interface, these interfaces are reasonably limited. Here are a couple of my suggestions on how to deal with that.

Separate crates

Have 2 crates: linuxpam-sys and openpam-sys. Both can then just use bindgen without any problems. Then, the pam-sys crate is merely there to merge these two and decide which one should be linked, depending on the platform and some environment variables. The API would provide only shared functions and reexport the non-shared functions as a submodule. This, to me, seems like the most ergonomic and proper solution.

The large problem with this solution. Only a single crate can links = "pam". If it is the pam-sys crate, the openpam-sys and linuxpam-sys crates do not really make sense on their own. It is the openpam-sys and linuxpam-sys crates, you need to decide with cargo features which one you want. The first seems like the more plausible option.

Single Crate, 2 Submodules

This is essentially equivalent to before and solves the links = "pam" problem. We have the singular pam-sys crate that has two submodules openpam and linux_pam. The root just exports which one got selected. Similar to how libpam-sys does it. This time using bindgen, however.

However, generating the documentation with bindgen will require for all the header files to be present in the repository, as the system may not have both linux_pam and openpam available. I am not convinced how logical that is. We should probably do that with git submodules. We also need to find a way to properly do reexports with documentation.

Those are the two solutions. The second solution seems the best, so will be focusing on that. I will have a look at doing this in the somewhat near future. It seems as if the ecosystem is already really split on pam-sys libraries:

I feel like PAM is a good target for Rust and that it is important that there is a good FFI library for it.

I would love to get the feedback from @1wilkens and @pvdrz.

And I hope this provided a status update for @chipsenkbeil.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024 1

The problem with libpam-sys as @1wilkens said as well, is that we need to manually declare the bindings. I am not opposed to that as I feel like these bindings almost never change and when they do we want to know about it anyway, but I do get the argument for wanting to use bindgen.

I did manage to make quite some progress on what I said in my previous post. I will replace the old PR with a new PR hopefully this weekend.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

I have looked at it, since we are using this library as well as the wrapper pam library in lemurs as well.

A lot is actually needed to do this.

TLDR on all the changed:

  • OpenPAM is used by macOS and most BSD-based distros (with the notable exception of OpenBSD)
  • OpenPAM (mostly) follows the XSSO specification made by the Solaris team. It adds some extensions, though. Linux PAM is quite different from the specification.
  • When it comes to many things, this library with OpenPAM is silently broken.
    • Status codes don't match
    • Flags don't match (e.g. PAM_SILENT being different)

I have been looking at fixing these things, since we experienced some problems with Ubuntu/Debian's PAM as well. I created a -sys of my own that addresses most of these issues. However, I didn't publish it, since it is really harmful to the ecosystem to have two -sys crates that link against the same system library.

Are you still willing to work on these crates, or have you moved on to other things? (which is totally understandable and fine) If so, I can submit a PR that addresses most of these issues. The problem being that we cannot use bindgen anymore, since we need to manually consider the differences between OpenPAM and Linux PAM.

How to actually deal with these differences is also a good question because somewhere the distinction between OpenPAM and Linux PAM needs to be made. I see a couple of possibilities.

  • pam-sys provides and labels both and lets pam-rs deal with the difference.
  • pam-sys decides based on the operating system (can be changed with an environment variable).
  • pam-sys decides based on cargo features.

I am curious whether you are willing to get involved with this.

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

👋 bindgen contributor here. Is there any way I could help so you can use bindgen? Would it be feasible to run different bindgen invocations based on the target?

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

Hey @pvdrz, I am not actually sure what the proper way to represent this would be. Take the library with the changes I am proposing. If you want to keep cross-compilation possibilities and not have people need to do build time configuration of a library 5 levels down the dependency tree, it is actually quite difficult. You would like to be able to detect the installed version at compile-time and have something to overwrite that detection to do cross-compilation or linking against a different binary.

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

What I was thinking was basically your option of

  • pam-sys provides and labels both and lets pam-rs deal with the difference.

Where bindings are generated for linux-pam if the target is linux and openpam for macos and the rest of the bsd family.

I'd say that pam-rs should be the crate responsible of handling the differences and provide opaque abstractions around the flags and status code just like std::io::ErrorKind works right now. It should also keep the parts of the API that don't intersect behind extension traits in the same fashion as std::os::linux::fs::MetadataExt and other similar traits.

Of course this is easier said than done and I'd be happy to help.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

I feel like this is the best solution, but automatically generating proper bindings will present a problem I fear. The problem is that the target platform alone is not enough. There are BSD systems using Linux-PAM and Linux systems using OpenPAM.

OpenPAM is currently used by FreeBSD, PC-BSD, Dragonfly BSD, NetBSD, Mac OS X and a few Linux distributions.

We could detect the installed version at runtime by dynamically linking the library at runtime and seeing whether the openpam specific symbols are present.

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

I'd be very interested in knowing which are those "few Linux distributions" because I did a quick search and got nothing 😅. Same for bsd, the only thing I know is that OpenBSD doesn't use PAM at all.

But yeah we could use libloading like you do instead and call the right bindgen invocation for each target.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

I'd be very interested in knowing which are those "few Linux distributions" because I did a quick search and got nothing sweat_smile.

As far as I know, and I have never seen anyone actually do it, some of the Anti-SystemD distributions have the option of using OpenPAM as it is supposedly more "lightweight".

It might be fine to use the target platform and have an environment variable to overwrite it if needed.

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

yeah I think providing the "natural" default in a simple manner and allowing the user to select whatever they want with env-vars sounds convenient.

from pam-sys.

1wilkens avatar 1wilkens commented on August 27, 2024

Hey sorry for the late reply!
My intent is to continue maintaining the pam crate family, however work got the better of my time in the last months. It motivates me to read, that people are using these libraries as I kinda abandoned my original use-case (also a login/display manager). (I really have to take a look at lemurs as I kinda got lost in the ways that libpam and systemd interact.)

Some thoughts on the matter:

  • I would really like to keep using bindgen here, to keep this crate as simple as (reasonably) possible.
  • The crate should also link against the "natural default" unless otherwise specified to avoid the "configure the dependency deep down the tree"-problem. People that run a system that willingly deviates from the expected case are probably savvy enough to configure things right anyway.
  • Any API improvements in pam are greatly appreciated as I only briefly explored the login manager use-case and the API might not satisfy other scenarios.

@coastalwhite how should we proceed? I can likely spare some time on this, however likely not before June. Would you be interested in opening a PR to start the discussion? Also, as lemurs seems like a great application leveraging pam/pam-sys, would you be interested in co-maintaining the crates? :)

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

👋

On bindgen we are trying to keep in touch with the maintainers of sys crates like this one and figure out how to improve their experience. I also have a project from work that depends on pam so I'd be interested on contributing in any way I can.

Regarding the linux-pam vs open-pam issue. I think we agree on just exposing the bindings as they are and leave another crate figure out the differences between both. Am I right?

from pam-sys.

chipsenkbeil avatar chipsenkbeil commented on August 27, 2024

Is this the source of truth to figure out the status of this library (and pam) when it comes to OpenPAM support? I want to be able to integrate PAM as part of chipsenkbeil/distant#155 so I can provide alternative authentication when using distant and the associated neovim plugin, distant.nvim.

I'm also concerned about 1wilkens/pam#25 leading to segfaults on musl platforms, but I'm assuming that's a pam and not pam-sys issue, though? If not, seems like something to be fixed prior to 1.0 of this sys crate, too!

from pam-sys.

chipsenkbeil avatar chipsenkbeil commented on August 27, 2024

@1wilkens @pvdrz any thoughts on this? @coastalwhite has put a lot of work into this and I'd hate to lose the momentum!

A single higher-level crate with two separate sys crates has some precedence with wezterm-ssh using ssh2 and libssh-rs, albeit those sub-crates were themselves wrappers on top of the equivalent sys crates.

I'm also not opposed to the way that libpam-sys seems to approach it. I'm assuming there's never a situation where OpenPAM and LinuxPAM are both available? If there is, would the former high-level crate with two sys crates be easier? Or would the libpam-sys approach also work fine to include both?

My first thought from an end-user with a high-level library would be to have a primary export with further modules in the same way that we have windows and unix modules for platform-specific code, which I believe is how libpam-sys is built today with linux_pam and openpam modules.

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

I like @coastalwhite first proposal as this is, as mentioned by themselves, a multilibrary issue, they just happen to have the same suffix so we have this blurry notion of PAM in our heads. So I agree, having two different sys crates makes total sense. Even though I agree there should be a crate that allows you to use whatever PAM library is available in your system,

I'm not sure if it should be a sys crate or not as it has been mentioned that both libraries differs in things like status codes. Having a non-sys crate with a regular Rust enum Status that doesn't expose its representation but matches the right status codes for linuxpam and openpam based on which one is being used sounds more ergonomic. That being said I'm not that familiar with PAM so take this with a grain of salt.

I'm not aware of any case where someone wants to use both PAM libraries at the same time so I wouldn't worry that much about the links issue. But who knows.

I'm not opposed to the second proposal, the only issue I see is that anyone who wants to do any authentication based on linux without caring about other OSes wouldn't have the option to use something like linuxpam-sys and would have to use the whole crate even if the crate has some limitations in its API for the sake of cross-compatibility.

I'm cc-ing @rnijveld as I think he might have some interesting opinions on this matter.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

Turns out the approach I was attempting with bindgen generating binding for both libraries just does not work. The reason it does not work is quite logical. Since C will not mangle the symbols, there will be symbols with the same name in the linker, which is not allowed. This is not solved by having two separate crates. I suspect that the libpam-sys approach is the only one that allows for the documentation of both bindings to be generated and a selection to be made.

from pam-sys.

1wilkens avatar 1wilkens commented on August 27, 2024

Hey all,

sorry it took so long for me to come back to this crate (and OSS) in general. I was busy finishing my PhD and had a subsequent job change which really captured all my energy.

I have caught up on the the posts here but will probably need another reread to properly disgest the available options. In the meantime I want to thank @coastalwhite for all your investigation and implementation so far!

As said in your PR I really would like to get back into things and release a 1.0 version of at least pam-sys if not pam this year. I'll add you as co-maintainer now but would like to discuss your proposed changes once more after I've had time to properly get what you are proposing.

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

@pvdrz

How do you also feel about coordinating on this, so that in the future sudo-rs could possibly also use this crate and properly support OpenPAM as a backend?

from pam-sys.

coastalwhite avatar coastalwhite commented on August 27, 2024

Another approach that can be used is to use Platform Specific Dependencies. Maybe, this is the better solution actually

from pam-sys.

pvdrz avatar pvdrz commented on August 27, 2024

At the sudo project we try to keep the dependency tree as minimal as possible. I could see the project using this if it were to provide OpenPAM support as well as maintaining bindings for both libraries might be cumbersome. However, that's my personal opinion and other contributors might disagree.

from pam-sys.

Related Issues (12)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.