fix(debian/authd.gdm-authd.pam): Use pam_authd.so in account step too (…

…#293)

In order to be able to read and save the module data in the same way we
should use the very same pam module.

In this case the exec module could be skipped explicitly, but this is
happening anyways implicitly since no data for it will be set, so no
need to add more complexity.

---

This is the simplest alternative to #292 

UDENG-2646

debian/authd.gdm-authd.pam

@@ -8,7 +8,12 @@ auth    substack        common-auth
 auth    requisite       pam_nologin.so
 auth    optional        pam_gnome_keyring.so
 
+account [default=ignore success=ok user_unknown=ignore]	pam_authd.so
+# This is potentially loading pam_authd.again but we've checks in AcctMgmt() to
+# prevent this to happen when the gdm-authd service is used without GDM extensions.
+# Plus the model used by the services is different, so there's no risk for this to happen.
 @include common-account
+
 # SELinux needs to be the first session rule. This ensures that any
 # lingering context has been cleared. Without this it is possible
 # that a module could execute code in the wrong domain.

pam/integration-tests/gdm_test.go

@@ -20,10 +20,14 @@ import (
 	"github.com/ubuntu/authd/pam/internal/proto"
 )
 
-func init() {
+func enableGdmExtension() {
 	gdm.AdvertisePamExtensions([]string{gdm.PamExtensionCustomJSON})
 }
 
+func init() {
+	enableGdmExtension()
+}
+
 const (
 	exampleBrokerName = "ExampleBroker"
 	localBrokerName   = "local"
@@ -284,7 +288,7 @@ func testGdmModule(t *testing.T, libPath string, args []string) {
 			t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) })
 			moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog)
 
-			serviceFile := createServiceFile(t, "module-loader", libPath,
+			serviceFile := createServiceFile(t, "gdm-authd", libPath,
 				moduleArgs)
 
 			gh := newGdmTestModuleHandler(t, serviceFile, tc.pamUser)
@@ -333,16 +337,21 @@ func testGdmModule(t *testing.T, libPath string, args []string) {
 			require.Equal(t, tc.wantPamInfoMessages, gh.pamInfoMessages,
 				"PAM Info messages do not match")
 
+			requirePreviousBrokerForUser(t, socketPath, "", tc.pamUser)
+
 			require.ErrorIs(t, gh.tx.AcctMgmt(pamFlags), tc.wantAcctMgmtErr,
 				"Account Management PAM Error messages do not match")
 
 			if tc.wantError != nil {
+				requirePreviousBrokerForUser(t, socketPath, "", tc.pamUser)
 				return
 			}
 
 			user, err := gh.tx.GetItem(pam.User)
 			require.NoError(t, err, "Can't get the pam user")
 			require.Equal(t, tc.pamUser, user, "PAM user name does not match expected")
+
+			requirePreviousBrokerForUser(t, socketPath, gh.selectedBrokerName, user)
 		})
 	}
 }
@@ -364,6 +373,105 @@ func TestGdmModuleWithCWrapper(t *testing.T) {
 	testGdmModule(t, wrapperLibPath, []string{libPath})
 }
 
+func TestGdmModuleAuthenticateWithoutGdmExtension(t *testing.T) {
+	// This cannot be parallel!
+	t.Cleanup(pam_test.MaybeDoLeakCheck)
+
+	libPath := buildPAMModule(t)
+	moduleArgs := []string{libPath}
+
+	gpasswdOutput := filepath.Join(t.TempDir(), "gpasswd.output")
+	groupsFile := filepath.Join(testutils.TestFamilyPath(t), "gpasswd.group")
+	ctx, cancel := context.WithCancel(context.Background())
+	socketPath, stopped := testutils.RunDaemon(ctx, t, daemonPath,
+		testutils.WithEnvironment(grouptests.GPasswdMockEnv(t, gpasswdOutput, groupsFile)...))
+	t.Cleanup(func() {
+		cancel()
+		<-stopped
+	})
+	moduleArgs = append(moduleArgs, "socket="+socketPath)
+
+	gdmLog := prepareFileLogging(t, "authd-pam-gdm.log")
+	t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) })
+	moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog)
+
+	serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs)
+	pamUser := "user1"
+	gh := newGdmTestModuleHandler(t, serviceFile, pamUser)
+	t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") })
+
+	// We disable gdm extension support, as if it was the case when the module is loaded
+	// outside GDM.
+	gdm.AdvertisePamExtensions(nil)
+	t.Cleanup(enableGdmExtension)
+
+	var pamFlags pam.Flags
+	if !testutils.IsVerbose() {
+		pamFlags = pam.Silent
+	}
+
+	require.ErrorIs(t, gh.tx.Authenticate(pamFlags), pam.ErrSystem,
+		"Authentication should fail")
+	requirePreviousBrokerForUser(t, socketPath, "", pamUser)
+}
+
+func TestGdmModuleAcctMgmtWithoutGdmExtension(t *testing.T) {
+	// This cannot be parallel!
+	t.Cleanup(pam_test.MaybeDoLeakCheck)
+
+	libPath := buildPAMModule(t)
+	moduleArgs := []string{libPath}
+
+	gpasswdOutput := filepath.Join(t.TempDir(), "gpasswd.output")
+	groupsFile := filepath.Join(testutils.TestFamilyPath(t), "gpasswd.group")
+	ctx, cancel := context.WithCancel(context.Background())
+	socketPath, stopped := testutils.RunDaemon(ctx, t, daemonPath,
+		testutils.WithEnvironment(grouptests.GPasswdMockEnv(t, gpasswdOutput, groupsFile)...))
+	t.Cleanup(func() {
+		cancel()
+		<-stopped
+	})
+	moduleArgs = append(moduleArgs, "socket="+socketPath)
+
+	gdmLog := prepareFileLogging(t, "authd-pam-gdm.log")
+	t.Cleanup(func() { saveArtifactsForDebug(t, []string{gdmLog}) })
+	moduleArgs = append(moduleArgs, "debug=true", "logfile="+gdmLog)
+
+	serviceFile := createServiceFile(t, "gdm-authd", libPath, moduleArgs)
+	pamUser := "user1"
+	gh := newGdmTestModuleHandler(t, serviceFile, pamUser)
+	t.Cleanup(func() { require.NoError(t, gh.tx.End(), "PAM: can't end transaction") })
+
+	gh.supportedLayouts = []*authd.UILayout{pam_test.FormUILayout()}
+	gh.protoVersion = gdm.ProtoVersion
+	gh.selectedBrokerName = exampleBrokerName
+	gh.selectedAuthModeIDs = []string{passwordAuthID}
+	gh.eventPollResponses = map[gdm.EventType][]*gdm.EventData{
+		gdm.EventType_startAuthentication: {
+			gdm_test.IsAuthenticatedEvent(&authd.IARequest_AuthenticationData_Challenge{
+				Challenge: "goodpass",
+			}),
+		},
+	}
+
+	var pamFlags pam.Flags
+	if !testutils.IsVerbose() {
+		pamFlags = pam.Silent
+	}
+
+	require.NoError(t, gh.tx.Authenticate(pamFlags), "Setup: Authentication failed")
+	requirePreviousBrokerForUser(t, socketPath, "", pamUser)
+
+	// We disable gdm extension support, as if it was the case when the module is loaded
+	// again from the exec module.
+	gdm.AdvertisePamExtensions(nil)
+	t.Cleanup(enableGdmExtension)
+
+	require.ErrorIs(t, gh.tx.AcctMgmt(pamFlags), pam_test.ErrIgnore,
+		"Account Management PAM Error message do not match")
+	requirePreviousBrokerForUser(t, socketPath, "", pamUser)
+}
+
 func buildPAMModule(t *testing.T) string {
 	t.Helper()
 

pam/integration-tests/helpers_test.go

@@ -1,13 +1,17 @@
 package main_test
 
 import (
+	"context"
 	"errors"
 	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"github.com/ubuntu/authd"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
 )
 
 func prepareFileLogging(t *testing.T, fileName string) string {
@@ -25,3 +29,27 @@ func prepareFileLogging(t *testing.T, fileName string) string {
 
 	return cliLog
 }
+
+func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName string, user string) {
+	t.Helper()
+
+	conn, err := grpc.Dial(
+		"unix://"+socketPath,
+		grpc.WithTransportCredentials(insecure.NewCredentials()),
+		grpc.WithBlock(),
+	)
+	require.NoError(t, err, "Can't connect to authd socket")
+	t.Cleanup(func() { conn.Close() })
+	pamClient := authd.NewPAMClient(conn)
+	brokers, err := pamClient.AvailableBrokers(context.TODO(), nil)
+	require.NoError(t, err, "Can't get available brokers")
+	prevBroker, err := pamClient.GetPreviousBroker(context.TODO(), &authd.GPBRequest{Username: user})
+	require.NoError(t, err, "Can't get previous broker")
+	var prevBrokerID string
+	for _, b := range brokers.BrokersInfos {
+		if b.Name == brokerName {
+			prevBrokerID = b.Id
+		}
+	}
+	require.Equal(t, prevBroker.PreviousBroker, prevBrokerID)
+}

pam/integration-tests/testdata/TestGdmModuleAcctMgmtWithoutGdmExtension

@@ -0,0 +1 @@
+TestGdmModule

pam/integration-tests/testdata/TestGdmModuleAuthenticateWithoutGdmExtension

@@ -0,0 +1 @@
+TestGdmModule

pam/pam.go

@@ -42,6 +42,10 @@ const (
 	// we've already authenticated with this module and so that we should not
 	// do this again.
 	alreadyAuthenticatedKey = "authd.already-authenticated-flag"
+
+	// gdmServiceName is the name of the service that is loaded by GDM.
+	// Keep this in sync with the service file installed by the package.
+	gdmServiceName = "gdm-authd"
 )
 
 var supportedArgs = []string{
@@ -332,6 +336,16 @@ func (h *pamModule) AcctMgmt(mTx pam.ModuleTransaction, flags pam.Flags, args []
 	}
 	logArgsIssues()
 
+	// We ignore AcctMgmt in case we're loading the module through the exec client
+	serviceName, err := mTx.GetItem(pam.Service)
+	if err != nil {
+		log.Warningf(context.TODO(), "Impossible to get PAM service name: %v", err)
+		return pam.ErrIgnore
+	}
+	if serviceName == gdmServiceName && !gdm.IsPamExtensionSupported(gdm.PamExtensionCustomJSON) {
+		return pam.ErrIgnore
+	}
+
 	brokerData, err := mTx.GetData(authenticationBrokerIDKey)
 	if err != nil && errors.Is(err, pam.ErrNoModuleData) {
 		return pam.ErrIgnore