From cbaf0d4c2a972d2e71d23625f5fc91d93880109b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Oct 2024 20:07:07 +0200 Subject: [PATCH 1/3] pam/gdmmodel: Iterate over poll results only if debugging is enabled Fixes: ca47562d3153 --- pam/internal/adapter/gdmmodel.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go index 0c1b49d8b..6f76d8197 100644 --- a/pam/internal/adapter/gdmmodel.go +++ b/pam/internal/adapter/gdmmodel.go @@ -96,13 +96,15 @@ func (m *gdmModel) pollGdm() tea.Cmd { }) } - for _, result := range gdmPollResults { - // Don't log EventData_IsAuthenticatedRequested because it contains - // the user password - if result.GetIsAuthenticatedRequested() != nil { - log.Debugf(context.TODO(), "GDM poll returned: IsAuthenticatedRequested") - } else { - log.Debugf(context.TODO(), "GDM poll returned: %s", result.Data) + if log.IsLevelEnabled(log.DebugLevel) { + for _, result := range gdmPollResults { + // Don't log EventData_IsAuthenticatedRequested because it contains + // the user password + if result.GetIsAuthenticatedRequested() != nil { + log.Debugf(context.TODO(), "GDM poll returned: IsAuthenticatedRequested") + } else { + log.Debugf(context.TODO(), "GDM poll returned: %s", result.Data) + } } } From e9a71f22fc428bfb0eb7fe77e161d0535f2bfcad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Oct 2024 21:16:00 +0200 Subject: [PATCH 2/3] pam/gdm: Show a sanitized challenge value when printing auth data in debug mode Instead of hiding the content authentication data completely, show the whole challenge when `pam_gdm_debug` build tag is used, or when in testing mode. Otherwise, just show a sanitized challenge so that we don't miss the fact that the event has happened Fixes: ca47562d3153 --- pam/internal/adapter/gdmmodel.go | 8 +------ pam/internal/gdm/debug.go | 1 + pam/internal/gdm/export_test.go | 1 + pam/internal/gdm/protocol.go | 37 ++++++++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/pam/internal/adapter/gdmmodel.go b/pam/internal/adapter/gdmmodel.go index 6f76d8197..9e6aa036c 100644 --- a/pam/internal/adapter/gdmmodel.go +++ b/pam/internal/adapter/gdmmodel.go @@ -98,13 +98,7 @@ func (m *gdmModel) pollGdm() tea.Cmd { if log.IsLevelEnabled(log.DebugLevel) { for _, result := range gdmPollResults { - // Don't log EventData_IsAuthenticatedRequested because it contains - // the user password - if result.GetIsAuthenticatedRequested() != nil { - log.Debugf(context.TODO(), "GDM poll returned: IsAuthenticatedRequested") - } else { - log.Debugf(context.TODO(), "GDM poll returned: %s", result.Data) - } + log.Debugf(context.TODO(), "GDM poll response: %v", result.SafeString()) } } diff --git a/pam/internal/gdm/debug.go b/pam/internal/gdm/debug.go index 367203e5f..fa7756518 100644 --- a/pam/internal/gdm/debug.go +++ b/pam/internal/gdm/debug.go @@ -5,4 +5,5 @@ package gdm func init() { checkMembersFunc = checkMembersDebug validateJSONFunc = validateJSONDebug + stringifyEventDataFunc = stringifyEventDataDebug } diff --git a/pam/internal/gdm/export_test.go b/pam/internal/gdm/export_test.go index 0438841fe..99b96fd3e 100644 --- a/pam/internal/gdm/export_test.go +++ b/pam/internal/gdm/export_test.go @@ -3,4 +3,5 @@ package gdm func init() { checkMembersFunc = checkMembersDebug validateJSONFunc = validateJSONDebug + stringifyEventDataFunc = stringifyEventDataDebug } diff --git a/pam/internal/gdm/protocol.go b/pam/internal/gdm/protocol.go index 1a46285fd..351f5a6f7 100644 --- a/pam/internal/gdm/protocol.go +++ b/pam/internal/gdm/protocol.go @@ -9,6 +9,7 @@ import ( "reflect" "slices" + "github.com/ubuntu/authd" "google.golang.org/protobuf/encoding/protojson" ) @@ -167,3 +168,39 @@ func (d *Data) JSON() ([]byte, error) { return bytes, err } + +var stringifyEventDataFunc = stringifyEventDataFiltered + +func stringifyEventDataDebug(ed *EventData) string { + return ed.String() +} + +func stringifyEventDataFiltered(ed *EventData) string { + authReq, ok := ed.GetData().(*EventData_IsAuthenticatedRequested) + if !ok { + return ed.String() + } + + item := authReq.IsAuthenticatedRequested.GetAuthenticationData().Item + if _, ok = item.(*authd.IARequest_AuthenticationData_Challenge); !ok { + return ed.String() + } + + return (&EventData{ + Type: ed.Type, + Data: &EventData_IsAuthenticatedRequested{ + IsAuthenticatedRequested: &Events_IsAuthenticatedRequested{ + AuthenticationData: &authd.IARequest_AuthenticationData{ + Item: &authd.IARequest_AuthenticationData_Challenge{ + Challenge: "**************", + }, + }, + }, + }, + }).String() +} + +// SafeString creates a string of EventData with confidential content removed. +func (ed *EventData) SafeString() string { + return stringifyEventDataFunc(ed) +} From c608853d2b6db17120e3a930f924f41dca331f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 2 Oct 2024 21:19:29 +0200 Subject: [PATCH 3/3] pam/gdm/conversation: Show the Poll response if non-empty In case the gdm data poll response has content, it's still something we want to show, even though we should sanitize the value when it contains the challenge secret Fixes: 40fa85db968ff --- pam/internal/gdm/conversation.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/pam/internal/gdm/conversation.go b/pam/internal/gdm/conversation.go index b675d3d17..6c936d8d9 100644 --- a/pam/internal/gdm/conversation.go +++ b/pam/internal/gdm/conversation.go @@ -6,6 +6,7 @@ import ( "context" "errors" "fmt" + "regexp" "sync/atomic" "github.com/msteinert/pam/v2" @@ -13,6 +14,7 @@ import ( ) var conversations atomic.Int32 +var challengeRegex = regexp.MustCompile(`"challenge"\s*:\s*"(?:[^"\\]|\\.)*"`) // ConversationInProgress checks if conversations are currently active. func ConversationInProgress() bool { @@ -63,9 +65,16 @@ func SendData(pamMTx pam.ModuleTransaction, d *Data) (*Data, error) { } gdmData, err := NewDataFromJSON(jsonValue) - // Log unless it's a poll, which are so frequently that it would be + // Log unless it's an empty poll, which are so frequently that it would be // too verbose to log them. - if d.Type != DataType_poll { + if gdmData.Type == DataType_pollResponse && len(gdmData.GetPollResponse()) == 0 { + jsonValue = nil + } + if log.IsLevelEnabled(log.DebugLevel) && jsonValue != nil && + gdmData != nil && gdmData.Type == DataType_pollResponse { + jsonValue = challengeRegex.ReplaceAll(jsonValue, []byte(`"challenge":"**************"`)) + } + if jsonValue != nil { log.Debugf(context.TODO(), "Got from GDM: %s", jsonValue) } if err != nil {