- 
                Notifications
    You must be signed in to change notification settings 
- Fork 39
Add PAM module stub generator, modules implmentations and integration tests #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
65b004b    to
    bceb1f8      
    Compare
  
    | This is now rebased on #15 instead as it makes few things better to handle. | 
24bf30b    to
    b3c9311      
    Compare
  
    …inds A pam handler can be used both by a module and by an Application, go-pam is meant to be used in the application side right now, but it can be easily changed to also create modules. This is the prerequisite work to support this.
This allows to easily define go-handlers for module operations. We need to expose few more types externally so that it's possible to create the module transaction handler and return specific transaction errors
If returned binaries are nil, we should pass them as nil and not as an empty bytes array.
A PAM module can be generated using pam-moduler and implemented fully in go without having to manually deal with the C setup. Module can be compiled using go generate, so go:generate directives can be used to make this process automatic, with a single go generate call as shown in the example.
This will make it easier to avoid exporting unexpected symbols to the generated PAM libraries. Also it makes less messy handling C code inside go files.
This function is only needed when using go PAM for creating applications so it's not something we expect to have exported to library modules. To prevent this use an `asPamModule` tag to prevent compilation of application-only features.
In this way all these features not even compiled when creating modules, avoiding generating unused code.
So we can reduce the generated code and add more unit tests
… works We mimic what pam_debug.so does by default, by implementing a similar module fully in go, generated using pam-moduler. This requires various utilities to generate the module and run the tests that are in a separate internal modules so that it can be shared between multiple implementations
In order to properly test the interaction of a module transaction from the application point of view, we need to perform operation in the module and ensure that the expected values are returned and handled In order to do this, without using the PAM apis that we want to test, use a simple trick: - Create an application that works as server using an unix socket - Create a module that connects to it - Pass the socket to the module via the module service file arguments - Add some basic protocol that allows the application to send a request and to the module to reply to that. - Use reflection and serialization to automatically call module methods and return the values to the application where we do the check
We can now finally test this properly both using a mock and through the interactive module that will do the request for us in various conditions.
Module data is data associated with a module handle that is available for the whole module loading time so it can be used also during different operations. We use cgo handles to preserve the life of the go objects so any value can be associated with a pam transaction.
Modules have the ability to start PAM conversations, so while the transaction code can handle them we did not have a way to init them. Yet. So add some APIs allowing this, making it easier from the go side to handle the conversations. In this commit we only support text-based conversations, but code is designed with the idea of supporting binary cases too. Added the integration tests using the module that is now able to both start conversation and handle them using Go only.
A module can now initiate a binary conversation decoding the native pointer value as it wants. Added tests to verify the main cases
Pam conversations per se may also run in parallel, but this implies that the application supports this. Since this normally not the case, do not create modules that may invoke the pam conversations in parallel by default, adding a mutex to protect such calls.
We have lots of cgo interaction here so better to check things fully. This also requires manually checking for leaks, so add support for this.
…andler This requires the allocating function to provide a binary pointer that will be free'd by the conversation handlers finalizers. This is for a more advanced usage scenario where the binary conversion may be handled manually.
We have test utils in other packages that are not shown as tested, while they definitely are.
| Any progress on this? Looks very promising. | 
| @StephenBrown2 I think we can basically merge this since it's something we've been using actively for some time in https://github.com/ubuntu/authd However, there are also problems when the modules are strictly done in go, since the go runtime may break other imported modules (dead-locking in SSH as per its specific thread implementation), so it's probably safer to have a different approach: 
 | 
| Agreed, I've experience the dead-locking in SSH first-hand with my own PAM module implementation, and have found several work-arounds for that, including in pam_fscrypt, by comparing the PID value between the Auth and session Open calls,  I wanted to see if I could write a PAM module that communicates with another local service via gRPC, and it looks like authd does just that. Do you do that via DBUS or the C wrapper during SSH authentication due to the forking issue? | 
| 
 Yeah, but google one is just avoiding the calls and it's really not a solution (and still may be affected),  So, eventually we decided that it was too risky to put unknown go runtime behavior in the PAM applications (since it was not reliable and too versions-dependent), and just go simple with a C wrapper that: 
 All the rest of the gRPC communication happens in go though, the C side only invokes actions in the go binary. So we can have in the same code both a go-library (that we use in safe cases such as GDM) and a PAM-module client application that is invoked through the module. We used DBus because it was the easier way to handle this from the C side (using GLib's Gio), given that we also want to support binary conversations, but in general it could have been using simple fd's or something like varlink. The C-module is simple and generic enough that can be used by anyone else though. In fact I was considering including it in this repo too, although it's a bit complex to expose it well in a go world. Thus the code of this PR this matters, since all the abstraction that is here can be used in both ways. See also: | 
| Thanks for the detailed description, that's a lot to think about. Sorry for slightly going off topic for this PR. Best wishes on getting it merged! | 
For the way go-pam is structured it made easy to create pam applications, but most of the code can be useful also for implementing modules in go.
As per this:
github.com/msteinert/pam/cmd/pam-moduler) to generate pam-module cgo stubsKeeping this as a draft for now as it requires some refactors that were part of #12, but also it may be adapted a bit if we want to change Transaction errors to be less prone to potential races.