Skip to content
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

PAM: Add support for more native pam features and initial support for gdm #2

Closed
wants to merge 9 commits into from

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Jul 21, 2023

Various changes while learning this on the go... In particular:

  • Added more pam native features so that we can do proper PAM conversation and show messages
  • Refactor the code so that the module can behave differently depending on the client capabilities
  • Add initial support (potentially done, but we have connection issues with the daemon) for standard PAM clients as SSH clients
  • Implement support for the GDM private protocol for choice lists to allow user selection of actions to do

Some shell interaction can be tested with hacks like:

diff --git a/js/gdm/util.js b/js/gdm/util.js
index 78c45cbbaa..58e9f93c12 100644
--- a/js/gdm/util.js
+++ b/js/gdm/util.js
@@ -46,6 +46,9 @@ var DISABLE_USER_LIST_KEY = 'disable-user-list';
 var USER_READ_TIME = 48;
 const FINGERPRINT_ERROR_TIMEOUT_WAIT = 15;
 
+// const SHELL_TEST_MODULE = 'gdm-shell-test';
+const SHELL_TEST_MODULE = 'gdm-authd';
+
 var MessageType = {
     // Keep messages in order by priority
     NONE: 0,
@@ -465,6 +468,7 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
             return;
         }
 
+        print('GDM: Client using choice list?', this._client.get_user_verifier_choice_list);
         if (this._client.get_user_verifier_choice_list)
             this._userVerifierChoiceList = this._client.get_user_verifier_choice_list();
         else
@@ -510,6 +514,9 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
             'service-unavailable', this._onServiceUnavailable.bind(this),
             'reset', this._onReset.bind(this),
             'verification-complete', this._onVerificationComplete.bind(this),
+            'conversation-started', (v, service) => {
+                print('GDM: Conversation started with',service)
+            },
             this);
 
         if (this._userVerifierChoiceList) {
@@ -602,10 +609,15 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
             this._fingerprintReaderType !== FingerprintReaderType.NONE &&
             !this.serviceIsForeground(FINGERPRINT_SERVICE_NAME))
             this._startService(FINGERPRINT_SERVICE_NAME);
+
+        print('GDM: Starting service', SHELL_TEST_MODULE)
+        this._startService(SHELL_TEST_MODULE);
     }
 
     _onChoiceListQuery(client, serviceName, promptMessage, list) {
-        if (!this.serviceIsForeground(serviceName))
+        log('GDM: Choice list query got from', serviceName, 'msg', promptMessage,
+            'list', list.deepUnpack());
+        if (!this.serviceIsForeground(serviceName) && serviceName !== SHELL_TEST_MODULE)
             return;
 
         this.emit('show-choice-list', serviceName, promptMessage, list.deepUnpack());
@@ -629,13 +641,19 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
                 this._queueMessage(serviceName, _('(or place finger on reader)'),
                     MessageType.HINT);
             }
+        } else if (serviceName === SHELL_TEST_MODULE) {
+            print('GDM: got message from',serviceName, info)
+            this._queueMessage(serviceName, info, MessageType.INFO);
         }
     }
 
     _onProblem(client, serviceName, problem) {
         const isFingerprint = this.serviceIsFingerprint(serviceName);
 
-        if (!this.serviceIsForeground(serviceName) && !isFingerprint)
+        print('GDM: Got probelem on',serviceName, ':', problem);
+
+        if (!this.serviceIsForeground(serviceName) && !isFingerprint &&
+            serviceName !== SHELL_TEST_MODULE)
             return;
 
         this._queuePriorityMessage(serviceName, problem, MessageType.ERROR);
@@ -669,14 +687,14 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
     }
 
     _onInfoQuery(client, serviceName, question) {
-        if (!this.serviceIsForeground(serviceName))
+        if (!this.serviceIsForeground(serviceName) && serviceName !== SHELL_TEST_MODULE)
             return;
 
         this.emit('ask-question', serviceName, question, false);
     }
 
     _onSecretInfoQuery(client, serviceName, secretQuestion) {
-        if (!this.serviceIsForeground(serviceName))
+        if (!this.serviceIsForeground(serviceName) && serviceName !== SHELL_TEST_MODULE)
             return;
 
         let token = null;
@@ -768,15 +786,18 @@ var ShellUserVerifier = class extends Signals.EventEmitter {
 
     _onServiceUnavailable(_client, serviceName, errorMessage) {
         this._unavailableServices.add(serviceName);
+        print('GDM: Service unavailable', serviceName, errorMessage)
 
         if (!errorMessage)
             return;
 
-        if (this.serviceIsForeground(serviceName) || this.serviceIsFingerprint(serviceName))
+        if (this.serviceIsForeground(serviceName) || this.serviceIsFingerprint(serviceName) ||
+            serviceName === SHELL_TEST_MODULE)
             this._queueMessage(serviceName, errorMessage, MessageType.ERROR);
     }
 
     _onConversationStopped(client, serviceName) {
+        print('GDM: Conversation stopped with',serviceName)
         // If the login failed with the preauthenticated oVirt credentials
         // then discard the credentials and revert to default authentication
         // mechanism.

@3v1n0 3v1n0 requested a review from a team as a code owner July 21, 2023 21:06
@3v1n0 3v1n0 force-pushed the pam-hacks branch 2 times, most recently from 83160c0 to b3fbd82 Compare July 21, 2023 21:36
Copy link
Member

@didrocks didrocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work!
I’m not a big fan of installing an external action to install dependencies, but we will get to it.

As a remark, we are slightly going to change the API with some customer's feedback, but doing this (and reforming the PAM module with a better UI model approach), I will cherry-pick some of your changes there to incorporate them, so that you can rebase on it.

We need to discuss also about the user experience (I think the broker should have an image associated with it, also the broker could be changed after the user name if needed and so on…)

@3v1n0
Copy link
Collaborator Author

3v1n0 commented Jul 28, 2023

Ok, mean time I've rebased this on main.

It's just better as it can be used both by terminal apps and other
interactive UIs, without any assumption on the terminal in use, this is
mostly due to the fact that a PAM client may be remote (as in SSH case)
and so we can't assume that is always using a terminal.

As per this, generalize the case trying to use the PAM APIs all the
times, while using the terminal specific features if that's possible.

So that ideally we can also share some of them with GDM ops.
Use a more object-oriented implementation, for what go allows to
generalize the clients types.

Basically for now we support two kinds of clients, that may behave
differently:
 - Simple standard PAM client (as for remote access)
 - Terminal client (an interactive terminal as in sudo)

This will allow to support graphical clients in an easier way.
@didrocks
Copy link
Member

Let’s archive this for now as the code moved quite a lot.

@didrocks didrocks closed this Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants