Skip to content

Commit

Permalink
Fix incorrect uses of the term "challenge"
Browse files Browse the repository at this point in the history
Depending on the context, we now use either "secret" or "password".
  • Loading branch information
adombeck committed Jan 24, 2025
1 parent 03a9fef commit ff194c4
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 184 deletions.
48 changes: 24 additions & 24 deletions examplebroker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -655,10 +655,10 @@ func (b *Broker) sleepDuration(in time.Duration) time.Duration {
}

func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionInfo, authData map[string]string) (access, data string) {
// Decrypt challenge if present.
challenge, err := decodeRawChallenge(b.privateKey, authData["challenge"])
// Decrypt secret if present.
secret, err := decodeRawSecret(b.privateKey, authData["challenge"])
if err != nil {
return auth.Retry, fmt.Sprintf(`{"message": "could not decode challenge: %v"}`, err)
return auth.Retry, fmt.Sprintf(`{"message": "could not decode secret: %v"}`, err)
}

exampleUsersMu.Lock()
Expand All @@ -670,24 +670,24 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI

sleepDuration := b.sleepDuration(4 * time.Second)

// Note that the layouts.Wait authentication can be cancelled and switch to another mode with a challenge.
// Note that the layouts.Wait authentication can be cancelled and switch to another mode with a secret.
// Take into account the cancellation.
switch sessionInfo.currentAuthMode {
case passwordMode.id:
expectedChallenge := user.Password
expectedSecret := user.Password

if challenge != expectedChallenge {
return auth.Retry, fmt.Sprintf(`{"message": "invalid password '%s', should be '%s'"}`, challenge, expectedChallenge)
if secret != expectedSecret {
return auth.Retry, fmt.Sprintf(`{"message": "invalid password '%s', should be '%s'"}`, secret, expectedSecret)
}

case pinCodeMode.id:
if challenge != "4242" {
if secret != "4242" {
return auth.Retry, `{"message": "invalid pincode, should be 4242"}`
}

case totpWithButtonMode.id, totpMode.id:
wantedCode := sessionInfo.allModes[sessionInfo.currentAuthMode].wantedCode
if challenge != wantedCode {
if secret != wantedCode {
return auth.Retry, `{"message": "invalid totp code"}`
}

Expand Down Expand Up @@ -745,27 +745,27 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
}
fallthrough
case mandatoryResetMode:
expectedChallenge := "authd2404"
expectedSecret := "authd2404"
// Reset the password to default if it had already been changed.
// As at PAM level we'd refuse a previous password to be re-used.
if user.Password == expectedChallenge {
expectedChallenge = "goodpass"
if user.Password == expectedSecret {
expectedSecret = "goodpass"
}

if challenge != expectedChallenge {
return auth.Retry, fmt.Sprintf(`{"message": "new password does not match criteria: must be '%s'"}`, expectedChallenge)
if secret != expectedSecret {
return auth.Retry, fmt.Sprintf(`{"message": "new password does not match criteria: must be '%s'"}`, expectedSecret)
}
exampleUsersMu.Lock()
exampleUsers[sessionInfo.username] = userInfoBroker{Password: challenge}
exampleUsers[sessionInfo.username] = userInfoBroker{Password: secret}
exampleUsersMu.Unlock()

// this case name was dynamically generated
case emailMode(sessionInfo.username).id:
// do we have a challenge sent or should we just wait?
if challenge != "" {
// validate challenge given manually by the user
if challenge != "aaaaa" {
return auth.Denied, `{"message": "invalid challenge, should be aaaaa"}`
// do we have a secret sent or should we just wait?
if secret != "" {
// validate secret given manually by the user
if secret != "aaaaa" {
return auth.Denied, `{"message": "invalid secret, should be aaaaa"}`
}
} else if authData[layouts.Wait] == layouts.True {
// we are simulating clicking on the url signal received by the broker
Expand All @@ -783,13 +783,13 @@ func (b *Broker) handleIsAuthenticated(ctx context.Context, sessionInfo sessionI
return auth.Granted, fmt.Sprintf(`{"userinfo": %s}`, userInfoFromName(sessionInfo.username))
}

// decodeRawChallenge extract the base64 challenge and try to decrypt it with the private key.
func decodeRawChallenge(priv *rsa.PrivateKey, rawChallenge string) (string, error) {
if rawChallenge == "" {
// decodeRawSecret extract the base64 secret and try to decrypt it with the private key.
func decodeRawSecret(priv *rsa.PrivateKey, rawSecret string) (string, error) {
if rawSecret == "" {
return "", nil
}

ciphertext, err := base64.StdEncoding.DecodeString(rawChallenge)
ciphertext, err := base64.StdEncoding.DecodeString(rawSecret)
if err != nil {
return "", err
}
Expand Down
75 changes: 38 additions & 37 deletions pam/internal/adapter/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ var (
errorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("#ff0000"))
)

// sendIsAuthenticated sends the authentication challenges or wait request to the brokers.
// sendIsAuthenticated sends the authentication secrets or wait request to the brokers.
// The event will contain the returned value from the broker.
func sendIsAuthenticated(ctx context.Context, client authd.PAMClient, sessionID string,
authData *authd.IARequest_AuthenticationData, challenge *string) tea.Cmd {
authData *authd.IARequest_AuthenticationData, secret *string) tea.Cmd {
return func() (msg tea.Msg) {
log.Debugf(context.TODO(), "Authentication request for session %q: %#v",
sessionID, authData.Item)
Expand All @@ -61,8 +61,8 @@ func sendIsAuthenticated(ctx context.Context, client authd.PAMClient, sessionID
<-time.After(cancellationWait * 3)

return isAuthenticatedResultReceived{
access: auth.Cancelled,
challenge: challenge,
access: auth.Cancelled,
secret: secret,
}
}
return pamError{
Expand All @@ -72,15 +72,15 @@ func sendIsAuthenticated(ctx context.Context, client authd.PAMClient, sessionID
}

return isAuthenticatedResultReceived{
access: res.Access,
msg: res.Msg,
challenge: challenge,
access: res.Access,
msg: res.Msg,
secret: secret,
}
}
}

// isAuthenticatedRequested is the internal events signalling that authentication
// with the given challenge or wait has been requested.
// with the given password or wait has been requested.
type isAuthenticatedRequested struct {
item authd.IARequestAuthenticationDataItem
}
Expand All @@ -95,9 +95,9 @@ type isAuthenticatedRequestedSend struct {
// isAuthenticatedResultReceived is the internal event with the authentication access result
// and data that was retrieved.
type isAuthenticatedResultReceived struct {
access string
challenge *string
msg string
access string
secret *string
msg string
}

// isAuthenticatedCancelled is the event to cancel the auth request.
Expand Down Expand Up @@ -127,7 +127,7 @@ type authenticationModel struct {
currentModel authenticationComponent
currentSessionID string
currentBrokerID string
currentChallenge string
currentSecret string
currentLayout string

authTracker *authTracker
Expand All @@ -153,15 +153,15 @@ type errMsgToDisplay struct {

// newPasswordCheck is sent to request a new password quality check.
type newPasswordCheck struct {
ctx context.Context
challenge string
ctx context.Context
password string
}

// newPasswordCheckResult returns the password quality check result.
type newPasswordCheckResult struct {
ctx context.Context
challenge string
msg string
ctx context.Context
password string
msg string
}

// newAuthenticationModel initializes a authenticationModel which needs to be Compose then.
Expand Down Expand Up @@ -194,10 +194,10 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
return *m, tea.Sequence(m.cancelIsAuthenticated(), sendEvent(AuthModeSelected{}))

case newPasswordCheck:
currentChallenge := m.currentChallenge
currentSecret := m.currentSecret
return *m, func() tea.Msg {
res := newPasswordCheckResult{ctx: msg.ctx, challenge: msg.challenge}
if err := checkChallengeQuality(currentChallenge, msg.challenge); err != nil {
res := newPasswordCheckResult{ctx: msg.ctx, password: msg.password}
if err := checkPasswordQuality(currentSecret, msg.password); err != nil {
res.msg = err.Error()
}
return res
Expand All @@ -213,7 +213,8 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
return *m, sendEvent(isAuthenticatedRequestedSend{
ctx: msg.ctx,
isAuthenticatedRequested: isAuthenticatedRequested{
item: &authd.IARequest_AuthenticationData_Challenge{Challenge: msg.challenge},
// TODO(UDENG-5844): Rename this to "secret" once all broker installations support the auth data field "secret".
item: &authd.IARequest_AuthenticationData_Challenge{Challenge: msg.password},
},
})
}
Expand Down Expand Up @@ -256,23 +257,23 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
return *m, func() tea.Msg {
authTracker.waitAndStart(cancelFunc)

challenge, hasChallenge := msg.item.(*authd.IARequest_AuthenticationData_Challenge)
if hasChallenge && clientType == Gdm && currentLayout == layouts.NewPassword {
return newPasswordCheck{ctx: ctx, challenge: challenge.Challenge}
secret, hasSecret := msg.item.(*authd.IARequest_AuthenticationData_Challenge)
if hasSecret && clientType == Gdm && currentLayout == layouts.NewPassword {
return newPasswordCheck{ctx: ctx, password: secret.Challenge}
}

return isAuthenticatedRequestedSend{msg, ctx}
}

case isAuthenticatedRequestedSend:
log.Debugf(context.TODO(), "%#v", msg)
// no challenge value, pass it as is
plainTextChallenge, err := msg.encryptChallengeIfPresent(m.encryptionKey)
// no password value, pass it as is
plainTextSecret, err := msg.encryptSecretIfPresent(m.encryptionKey)
if err != nil {
return *m, sendEvent(pamError{status: pam.ErrSystem, msg: fmt.Sprintf("could not encrypt challenge payload: %v", err)})
return *m, sendEvent(pamError{status: pam.ErrSystem, msg: fmt.Sprintf("could not encrypt password payload: %v", err)})
}

return *m, sendIsAuthenticated(msg.ctx, m.client, m.currentSessionID, &authd.IARequest_AuthenticationData{Item: msg.item}, plainTextChallenge)
return *m, sendIsAuthenticated(msg.ctx, m.client, m.currentSessionID, &authd.IARequest_AuthenticationData{Item: msg.item}, plainTextSecret)

case isAuthenticatedCancelled:
log.Debugf(context.TODO(), "%#v", msg)
Expand All @@ -281,13 +282,13 @@ func (m *authenticationModel) Update(msg tea.Msg) (authModel authenticationModel
case isAuthenticatedResultReceived:
log.Debugf(context.TODO(), "%#v", msg)

// Resets challenge if the authentication wasn't successful.
// Resets password if the authentication wasn't successful.
defer func() {
// the returned authModel is a copy of function-level's `m` at this point!
m := &authModel
if msg.challenge != nil &&
if msg.secret != nil &&
(msg.access == auth.Granted || msg.access == auth.Next) {
m.currentChallenge = *msg.challenge
m.currentSecret = *msg.secret
}

if msg.access != auth.Next && msg.access != auth.Retry {
Expand Down Expand Up @@ -481,22 +482,22 @@ func dataToMsg(data string) (string, error) {
return r, nil
}

func (authData *isAuthenticatedRequestedSend) encryptChallengeIfPresent(publicKey *rsa.PublicKey) (*string, error) {
// no challenge value, pass it as is
challenge, ok := authData.item.(*authd.IARequest_AuthenticationData_Challenge)
func (authData *isAuthenticatedRequestedSend) encryptSecretIfPresent(publicKey *rsa.PublicKey) (*string, error) {
// no password value, pass it as is
secret, ok := authData.item.(*authd.IARequest_AuthenticationData_Challenge)
if !ok {
return nil, nil
}

ciphertext, err := rsa.EncryptOAEP(sha512.New(), rand.Reader, publicKey, []byte(challenge.Challenge), nil)
ciphertext, err := rsa.EncryptOAEP(sha512.New(), rand.Reader, publicKey, []byte(secret.Challenge), nil)
if err != nil {
return nil, err
}

// encrypt it to base64 and replace the challenge with it
// encrypt it to base64 and replace the password with it
base64Encoded := base64.StdEncoding.EncodeToString(ciphertext)
authData.item = &authd.IARequest_AuthenticationData_Challenge{Challenge: base64Encoded}
return &challenge.Challenge, nil
return &secret.Challenge, nil
}

// wait waits for the current authentication to be completed.
Expand Down
2 changes: 1 addition & 1 deletion pam/internal/adapter/brokerselection.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
"github.com/ubuntu/authd/pam/internal/proto"
)

// brokerSelectionModel is the model list selection layout to allow authenticating and return a challenge.
// brokerSelectionModel is the model list selection layout to allow authenticating and return a password.
type brokerSelectionModel struct {
list.Model
focused bool
Expand Down
2 changes: 1 addition & 1 deletion pam/internal/adapter/formmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/ubuntu/authd/log"
)

// formModel is the form layout type to allow authentication and return a challenge.
// formModel is the form layout type to allow authentication and return a password.
type formModel struct {
label string

Expand Down
2 changes: 1 addition & 1 deletion pam/internal/adapter/gdmmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (m *gdmModel) pollGdm() tea.Cmd {
log.Infof(context.TODO(), "GDM Stage changed to %s", res.StageChanged.Stage)

if m.waitingAuth && res.StageChanged.Stage != proto.Stage_challenge {
// Maybe this can be sent only if we ever hit the challenge phase.
// Maybe this can be sent only if we ever hit the password phase.
commands = append(commands, sendEvent(isAuthenticatedCancelled{}))
}
commands = append(commands, sendEvent(ChangeStage{res.StageChanged.Stage}))
Expand Down
Loading

0 comments on commit ff194c4

Please sign in to comment.