From 521bae51a932430b20125478cf1193324de48c9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 3 Oct 2024 00:14:40 +0200 Subject: [PATCH] PAM: Build GDM support only on the library We don't really use all the gdm-related code in the PAM Exec child that we use in normal PAM transactions, so no need to compile it at all by default while we need it in the PAM library that GDM will consume. --- .github/workflows/qa.yaml | 8 +++--- pam/generate.go | 2 +- pam/generate_debug.go | 2 +- .../gdm-module-handler_test.go | 2 ++ pam/integration-tests/gdm_test.go | 28 ++++++++++++++++++- pam/integration-tests/helpers_test.go | 26 ----------------- pam/internal/adapter/authentication.go | 4 +-- pam/internal/adapter/gdmmodel.go | 3 +- .../adapter/gdmmodel_convhandler_test.go | 2 ++ pam/internal/adapter/gdmmodel_default.go | 9 ++++++ pam/internal/adapter/gdmmodel_disabled.go | 9 ++++++ pam/internal/adapter/gdmmodel_helpers_test.go | 2 ++ pam/internal/adapter/gdmmodel_test.go | 2 ++ pam/internal/adapter/gdmmodel_uimodel_test.go | 2 ++ pam/internal/adapter/model.go | 2 +- pam/internal/gdm/conversation.go | 2 ++ pam/internal/gdm/conversation_test.go | 2 ++ pam/internal/gdm/export_test.go | 2 ++ pam/internal/gdm/extension.go | 2 ++ pam/internal/gdm/extension_test.go | 2 ++ pam/internal/gdm/extension_unsupported.go | 12 ++++++++ pam/internal/gdm/gdm.pb.go | 2 ++ pam/internal/gdm/generate.go | 5 ++++ pam/internal/gdm/protocol.go | 2 +- pam/internal/gdm/protocol_test.go | 2 ++ pam/internal/gdm_test/gdm_utils.go | 2 ++ pam/internal/proto/pam.pb.go | 6 ++-- pam/pam_module.go | 4 +-- pam/pam_module_debug.go | 4 +-- 29 files changed, 106 insertions(+), 46 deletions(-) create mode 100644 pam/internal/adapter/gdmmodel_default.go create mode 100644 pam/internal/adapter/gdmmodel_disabled.go create mode 100644 pam/internal/gdm/extension_unsupported.go create mode 100644 pam/internal/gdm/generate.go diff --git a/.github/workflows/qa.yaml b/.github/workflows/qa.yaml index 99b4ec9ac..202cec73e 100644 --- a/.github/workflows/qa.yaml +++ b/.github/workflows/qa.yaml @@ -201,7 +201,7 @@ jobs: # Overriding the default coverage directory is not an exported flag of go test (yet), so # we need to override it using the test.gocoverdir flag instead. #TODO: Update when https://go-review.googlesource.com/c/go/+/456595 is merged. - go test -timeout ${GO_TESTS_TIMEOUT} -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}" + go test -tags withgdmmodel -timeout ${GO_TESTS_TIMEOUT} -cover -covermode=set ./... -coverpkg=./... -shuffle=on -args -test.gocoverdir="${raw_cov_dir}" # Convert the raw coverage data into textfmt so we can merge the Rust one into it go tool covdata textfmt -i="${raw_cov_dir}" -o="${cov_dir}/coverage.out" @@ -220,7 +220,7 @@ jobs: env: GO_TESTS_TIMEOUT: 35m run: | - go test -timeout ${GO_TESTS_TIMEOUT} -race ./... + go test -tags withgdmmodel -timeout ${GO_TESTS_TIMEOUT} -race ./... - name: Run PAM tests (with Address Sanitizer) if: matrix.test == 'asan' @@ -233,14 +233,14 @@ jobs: # Use these flags to give ASAN a better time to unwind the stack trace GO_GC_FLAGS: -N -l run: | - go test -C ./pam/internal -asan -gcflags=all="${GO_GC_FLAGS}" -timeout ${GO_TESTS_TIMEOUT} ./... || exit_code=$? + go test -C ./pam/internal -tags withgdmmodel -asan -gcflags=all="${GO_GC_FLAGS}" -timeout ${GO_TESTS_TIMEOUT} ./... || exit_code=$? if [ "${exit_code}" -ne 0 ]; then cat "${AUTHD_TEST_ARTIFACTS_PATH}"/asan.log* || true exit ${exit_code} fi pushd ./pam/integration-tests - go test -asan -gcflags=all="${GO_GC_FLAGS}" -c + go test -tags withgdmmodel -asan -gcflags=all="${GO_GC_FLAGS}" -c # FIXME: Suppression may be removed with newer libpam, as the one we ship in ubuntu as some leaks env LSAN_OPTIONS=suppressions=$(pwd)/lsan.supp \ ./integration-tests.test \ diff --git a/pam/generate.go b/pam/generate.go index 3e3e26f07..1b649051c 100644 --- a/pam/generate.go +++ b/pam/generate.go @@ -2,6 +2,6 @@ //go:generate go generate -C internal/proto -//go:generate ./generate.sh -tags "!pam_binary_cli && !pam_debug" +//go:generate ./generate.sh -tags "!pam_binary_cli && !pam_debug" -build-tags withgdmmodel package main diff --git a/pam/generate_debug.go b/pam/generate_debug.go index b775a98b7..e22ce7ada 100644 --- a/pam/generate_debug.go +++ b/pam/generate_debug.go @@ -2,6 +2,6 @@ //go:generate go generate -C internal/proto -//go:generate env CFLAGS=-g3 CGO_CFLAGS=-g3 ./generate.sh -tags "!pam_binary_cli && pam_debug" -build-tags pam_gdm_debug -output pam_module_debug.go +//go:generate env CFLAGS=-g3 CGO_CFLAGS=-g3 ./generate.sh -tags "!pam_binary_cli && pam_debug" -build-tags "pam_gdm_debug,withgdmmodel" -output pam_module_debug.go package main diff --git a/pam/integration-tests/gdm-module-handler_test.go b/pam/integration-tests/gdm-module-handler_test.go index 31b8587cc..af82061b9 100644 --- a/pam/integration-tests/gdm-module-handler_test.go +++ b/pam/integration-tests/gdm-module-handler_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package main_test import ( diff --git a/pam/integration-tests/gdm_test.go b/pam/integration-tests/gdm_test.go index 42e83ce0f..f63e9a951 100644 --- a/pam/integration-tests/gdm_test.go +++ b/pam/integration-tests/gdm_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package main_test import ( @@ -14,12 +16,15 @@ import ( "github.com/stretchr/testify/require" "github.com/ubuntu/authd" "github.com/ubuntu/authd/internal/brokers" + "github.com/ubuntu/authd/internal/services/errmessages" "github.com/ubuntu/authd/internal/testutils" localgroupstestutils "github.com/ubuntu/authd/internal/users/localgroups/testutils" "github.com/ubuntu/authd/pam/internal/gdm" "github.com/ubuntu/authd/pam/internal/gdm_test" "github.com/ubuntu/authd/pam/internal/pam_test" "github.com/ubuntu/authd/pam/internal/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) func enableGdmExtension() { @@ -888,7 +893,7 @@ func buildPAMModule(t *testing.T) string { libPath := filepath.Join(t.TempDir(), "libpam_authd.so") t.Logf("Compiling PAM library at %s", libPath) - cmd.Args = append(cmd.Args, "-tags=pam_debug,pam_gdm_debug", "-o", libPath) + cmd.Args = append(cmd.Args, "-tags=withgdmmodel,pam_debug,pam_gdm_debug", "-o", libPath) out, err := cmd.CombinedOutput() require.NoError(t, err, "Setup: could not compile PAM module: %s", out) if string(out) != "" { @@ -924,3 +929,24 @@ func testQrcodeUILayoutData(reqN int) *authd.UILayout { Entry: base.Entry, } } + +func requirePreviousBrokerForUser(t *testing.T, socketPath string, brokerName string, user string) { + t.Helper() + + conn, err := grpc.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) + 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) +} diff --git a/pam/integration-tests/helpers_test.go b/pam/integration-tests/helpers_test.go index 4a67bc60f..3c1156a0f 100644 --- a/pam/integration-tests/helpers_test.go +++ b/pam/integration-tests/helpers_test.go @@ -1,7 +1,6 @@ package main_test import ( - "context" "errors" "io/fs" "math" @@ -11,11 +10,7 @@ import ( "time" "github.com/stretchr/testify/require" - "github.com/ubuntu/authd" - "github.com/ubuntu/authd/internal/services/errmessages" "github.com/ubuntu/authd/internal/testutils" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" ) func prepareFileLogging(t *testing.T, fileName string) string { @@ -35,27 +30,6 @@ 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.NewClient("unix://"+socketPath, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(errmessages.FormatErrorMessage)) - 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) -} - func saveArtifactsForDebugOnCleanup(t *testing.T, artifacts []string) { t.Helper() t.Cleanup(func() { saveArtifactsForDebug(t, artifacts) }) diff --git a/pam/internal/adapter/authentication.go b/pam/internal/adapter/authentication.go index 7a3a6749f..9a535cae5 100644 --- a/pam/internal/adapter/authentication.go +++ b/pam/internal/adapter/authentication.go @@ -93,9 +93,7 @@ type isAuthenticatedResultReceived struct { } // isAuthenticatedCancelled is the event to cancel the auth request. -type isAuthenticatedCancelled struct { - msg string -} +type isAuthenticatedCancelled struct{} // reselectAuthMode signals to restart auth mode selection with the same id (to resend sms or // reenable the broker). diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go index 3a3841ebb..69e90c757 100644 --- a/pam/internal/adapter/gdmmodel.go +++ b/pam/internal/adapter/gdmmodel.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package adapter import ( @@ -286,7 +288,6 @@ func (m gdmModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, sendEvent(m.emitEventSync(&gdm.EventData_AuthEvent{ AuthEvent: &gdm.Events_AuthEvent{Response: &authd.IAResponse{ Access: brokers.AuthCancelled, - Msg: msg.msg, }}, })) } diff --git a/pam/internal/adapter/gdmmodel_convhandler_test.go b/pam/internal/adapter/gdmmodel_convhandler_test.go index 29249cc7f..334da0954 100644 --- a/pam/internal/adapter/gdmmodel_convhandler_test.go +++ b/pam/internal/adapter/gdmmodel_convhandler_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package adapter import ( diff --git a/pam/internal/adapter/gdmmodel_default.go b/pam/internal/adapter/gdmmodel_default.go new file mode 100644 index 000000000..773a15437 --- /dev/null +++ b/pam/internal/adapter/gdmmodel_default.go @@ -0,0 +1,9 @@ +//go:build withgdmmodel + +package adapter + +import "github.com/msteinert/pam/v2" + +func getGdmModel(pamMTx pam.ModuleTransaction) gdmModeler { + return gdmModel{pamMTx: pamMTx} +} diff --git a/pam/internal/adapter/gdmmodel_disabled.go b/pam/internal/adapter/gdmmodel_disabled.go new file mode 100644 index 000000000..20340948e --- /dev/null +++ b/pam/internal/adapter/gdmmodel_disabled.go @@ -0,0 +1,9 @@ +//go:build !withgdmmodel + +package adapter + +import "github.com/msteinert/pam/v2" + +func getGdmModel(pamMTx pam.ModuleTransaction) gdmModeler { + return nil +} diff --git a/pam/internal/adapter/gdmmodel_helpers_test.go b/pam/internal/adapter/gdmmodel_helpers_test.go index 896585cf0..513fdb2e9 100644 --- a/pam/internal/adapter/gdmmodel_helpers_test.go +++ b/pam/internal/adapter/gdmmodel_helpers_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package adapter import ( diff --git a/pam/internal/adapter/gdmmodel_test.go b/pam/internal/adapter/gdmmodel_test.go index 7bf79c034..4d5cd39c6 100644 --- a/pam/internal/adapter/gdmmodel_test.go +++ b/pam/internal/adapter/gdmmodel_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package adapter import ( diff --git a/pam/internal/adapter/gdmmodel_uimodel_test.go b/pam/internal/adapter/gdmmodel_uimodel_test.go index 8860fbda1..aef37fe07 100644 --- a/pam/internal/adapter/gdmmodel_uimodel_test.go +++ b/pam/internal/adapter/gdmmodel_uimodel_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package adapter import ( diff --git a/pam/internal/adapter/model.go b/pam/internal/adapter/model.go index 5e9ac96bd..dbb3d7ba1 100644 --- a/pam/internal/adapter/model.go +++ b/pam/internal/adapter/model.go @@ -107,7 +107,7 @@ func (m *UIModel) Init() tea.Cmd { switch m.ClientType { case Gdm: - m.gdmModel = gdmModel{pamMTx: m.PamMTx} + m.gdmModel = getGdmModel(m.PamMTx) cmds = append(cmds, m.gdmModel.Init()) case Native: m.nativeModel = nativeModel{pamMTx: m.PamMTx, nssClient: m.NssClient} diff --git a/pam/internal/gdm/conversation.go b/pam/internal/gdm/conversation.go index b675d3d17..56e99dc19 100644 --- a/pam/internal/gdm/conversation.go +++ b/pam/internal/gdm/conversation.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm import "C" diff --git a/pam/internal/gdm/conversation_test.go b/pam/internal/gdm/conversation_test.go index cdc26922d..3576eb673 100644 --- a/pam/internal/gdm/conversation_test.go +++ b/pam/internal/gdm/conversation_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm import ( diff --git a/pam/internal/gdm/export_test.go b/pam/internal/gdm/export_test.go index 0438841fe..2dd7eea22 100644 --- a/pam/internal/gdm/export_test.go +++ b/pam/internal/gdm/export_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm func init() { diff --git a/pam/internal/gdm/extension.go b/pam/internal/gdm/extension.go index f593c5814..fbf76953a 100644 --- a/pam/internal/gdm/extension.go +++ b/pam/internal/gdm/extension.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm /* diff --git a/pam/internal/gdm/extension_test.go b/pam/internal/gdm/extension_test.go index a1764082a..8ca6c78a8 100644 --- a/pam/internal/gdm/extension_test.go +++ b/pam/internal/gdm/extension_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm import ( diff --git a/pam/internal/gdm/extension_unsupported.go b/pam/internal/gdm/extension_unsupported.go new file mode 100644 index 000000000..34fd71f93 --- /dev/null +++ b/pam/internal/gdm/extension_unsupported.go @@ -0,0 +1,12 @@ +//go:build !withgdmmodel + +// Package gdm is the package for the GDM pam module handing. +package gdm + +// PamExtensionCustomJSON is the gdm PAM extension for passing string values. +const PamExtensionCustomJSON = "" + +// IsPamExtensionSupported returns if the provided extension is supported. +func IsPamExtensionSupported(extension string) bool { + return false +} diff --git a/pam/internal/gdm/gdm.pb.go b/pam/internal/gdm/gdm.pb.go index 1d785e82e..234e34985 100644 --- a/pam/internal/gdm/gdm.pb.go +++ b/pam/internal/gdm/gdm.pb.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.34.2 diff --git a/pam/internal/gdm/generate.go b/pam/internal/gdm/generate.go new file mode 100644 index 000000000..2f5722dec --- /dev/null +++ b/pam/internal/gdm/generate.go @@ -0,0 +1,5 @@ +//go:build generate + +package gdm + +//go:generate ../../../tools/generate-proto.sh -I../../.. -I../proto --with-build-tag withgdmmodel gdm.proto diff --git a/pam/internal/gdm/protocol.go b/pam/internal/gdm/protocol.go index 1a46285fd..6b15a7542 100644 --- a/pam/internal/gdm/protocol.go +++ b/pam/internal/gdm/protocol.go @@ -1,4 +1,4 @@ -//go:generate ../../../tools/generate-proto.sh -I../../.. -I../proto gdm.proto +//go:build withgdmmodel // Package gdm is the package for the GDM pam module handing. package gdm diff --git a/pam/internal/gdm/protocol_test.go b/pam/internal/gdm/protocol_test.go index 582a26abf..d18d04609 100644 --- a/pam/internal/gdm/protocol_test.go +++ b/pam/internal/gdm/protocol_test.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm_test import ( diff --git a/pam/internal/gdm_test/gdm_utils.go b/pam/internal/gdm_test/gdm_utils.go index 7bfa17273..920cd227e 100644 --- a/pam/internal/gdm_test/gdm_utils.go +++ b/pam/internal/gdm_test/gdm_utils.go @@ -1,3 +1,5 @@ +//go:build withgdmmodel + package gdm_test import ( diff --git a/pam/internal/proto/pam.pb.go b/pam/internal/proto/pam.pb.go index 30796f151..3f219cf0c 100644 --- a/pam/internal/proto/pam.pb.go +++ b/pam/internal/proto/pam.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.34.2 -// protoc v4.25.1 +// protoc-gen-go v1.31.0 +// protoc v3.12.4 // source: pam.proto package proto @@ -105,7 +105,7 @@ func file_pam_proto_rawDescGZIP() []byte { } var file_pam_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_pam_proto_goTypes = []any{ +var file_pam_proto_goTypes = []interface{}{ (Stage)(0), // 0: pam.Stage } var file_pam_proto_depIdxs = []int32{ diff --git a/pam/pam_module.go b/pam/pam_module.go index e6a37edc9..7772256da 100644 --- a/pam/pam_module.go +++ b/pam/pam_module.go @@ -1,8 +1,8 @@ -// Code generated by "pam-moduler -libname pam_authd.so -type pamModule -no-main -tags !pam_binary_cli && !pam_debug"; DO NOT EDIT. +// Code generated by "pam-moduler -libname pam_authd.so -type pamModule -no-main -tags !pam_binary_cli && !pam_debug -build-tags withgdmmodel"; DO NOT EDIT. //go:build !pam_binary_cli && !pam_debug -//go:generate go build "-ldflags=-extldflags -Wl,-soname,pam_authd.so" -buildmode=c-shared -o pam_authd.so -tags go_pam_module +//go:generate go build "-ldflags=-extldflags -Wl,-soname,pam_authd.so" -buildmode=c-shared -o pam_authd.so -tags go_pam_module,withgdmmodel // Package main is the package for the PAM module library. package main diff --git a/pam/pam_module_debug.go b/pam/pam_module_debug.go index 4c842438f..a7f36120c 100644 --- a/pam/pam_module_debug.go +++ b/pam/pam_module_debug.go @@ -1,8 +1,8 @@ -// Code generated by "pam-moduler -libname pam_authd.so -type pamModule -no-main -tags !pam_binary_cli && pam_debug -build-tags pam_gdm_debug -output pam_module_debug.go"; DO NOT EDIT. +// Code generated by "pam-moduler -libname pam_authd.so -type pamModule -no-main -tags !pam_binary_cli && pam_debug -build-tags pam_gdm_debug,withgdmmodel -output pam_module_debug.go"; DO NOT EDIT. //go:build !pam_binary_cli && pam_debug -//go:generate go build "-ldflags=-extldflags -Wl,-soname,pam_authd.so" -buildmode=c-shared -o pam_authd.so -tags go_pam_module,pam_gdm_debug +//go:generate go build "-ldflags=-extldflags -Wl,-soname,pam_authd.so" -buildmode=c-shared -o pam_authd.so -tags go_pam_module,pam_gdm_debug,withgdmmodel // Package main is the package for the PAM module library. package main