From 6a6df7c1ea8556cbb21841228d2a4aa53ce344d6 Mon Sep 17 00:00:00 2001 From: cviecco Date: Thu, 1 Aug 2024 09:32:27 -0700 Subject: [PATCH] Fix client retry on good token (#240) * adding new path channel to disconnect on bad response * more debug * missed commit * fixing typos * addressed nit --- cmd/keymaster/main.go | 1 + cmd/keymasterd/2fa_okta.go | 12 ++++++++++ lib/authenticators/okta/impl.go | 7 +++--- lib/client/twofa/pushtoken/pushtoken.go | 32 +++++++++++++++++++++++-- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/cmd/keymaster/main.go b/cmd/keymaster/main.go index 90a598fe..8ed8620a 100644 --- a/cmd/keymaster/main.go +++ b/cmd/keymaster/main.go @@ -374,6 +374,7 @@ func setupCerts( } } + logger.Debugf(1, "SetupCerts: authentication Complete") if err := signers.Wait(); err != nil { return err } diff --git a/cmd/keymasterd/2fa_okta.go b/cmd/keymasterd/2fa_okta.go index dea30ea9..1101e989 100644 --- a/cmd/keymasterd/2fa_okta.go +++ b/cmd/keymasterd/2fa_okta.go @@ -95,12 +95,24 @@ func (state *RuntimeState) oktaPushStartHandler(w http.ResponseWriter, r *http.R state.writeFailureResponse(w, r, http.StatusInternalServerError, "Apperent Misconfiguration") return } + userResponse, err := oktaAuth.GetValidUserResponse(authData.Username) + if err != nil { + logger.Debugf(2, "oktaPushStartHandler: ") + } + if len(userResponse.Embedded.Factor) < 1 { + logger.Printf("oktaPushStartHandler: user %s does not have valid authenticators", authData.Username) + logger.Debugf(2, "oktaPushStartHandler: userdata for broken user%s is :%s", authData.Username, userResponse) + state.writeFailureResponse(w, r, http.StatusPreconditionFailed, "No valid MFA authenticators available") + return + } + pushResponse, err := oktaAuth.ValidateUserPush(authData.Username) if err != nil { logger.Println(err) state.writeFailureResponse(w, r, http.StatusInternalServerError, "Failure when validating OKTA push") return } + logger.Debugf(2, "oktaPushStartHandler: after validating push response=%+v", pushResponse) switch pushResponse { case okta.PushResponseWaiting: w.WriteHeader(http.StatusOK) diff --git a/lib/authenticators/okta/impl.go b/lib/authenticators/okta/impl.go index e7bba181..6dcf1df3 100644 --- a/lib/authenticators/okta/impl.go +++ b/lib/authenticators/okta/impl.go @@ -122,7 +122,7 @@ func (pa *PasswordAuthenticator) passwordAuthenticate(username string, } } -func (pa *PasswordAuthenticator) getValidUserResponse(username string) (*OktaApiPrimaryResponseType, error) { +func (pa *PasswordAuthenticator) GetValidUserResponse(username string) (*OktaApiPrimaryResponseType, error) { pa.mutex.Lock() userData, ok := pa.recentAuth[username] defer pa.mutex.Unlock() @@ -138,7 +138,7 @@ func (pa *PasswordAuthenticator) getValidUserResponse(username string) (*OktaApi } func (pa *PasswordAuthenticator) validateUserOTP(username string, otpValue int) (bool, error) { - userResponse, err := pa.getValidUserResponse(username) + userResponse, err := pa.GetValidUserResponse(username) if err != nil { return false, err } @@ -195,13 +195,14 @@ func (pa *PasswordAuthenticator) validateUserOTP(username string, otpValue int) } func (pa *PasswordAuthenticator) validateUserPush(username string) (PushResponse, error) { - userResponse, err := pa.getValidUserResponse(username) + userResponse, err := pa.GetValidUserResponse(username) if err != nil { return PushResponseRejected, err } if userResponse == nil { return PushResponseRejected, nil } + pa.logger.Debugf(2, "oktaAuthenticator: validateUserPush: after getting userResponse=%+v", userResponse) rvalue := PushResponseRejected for _, factor := range userResponse.Embedded.Factor { if !(factor.FactorType == "push" && factor.VendorName == "OKTA") { diff --git a/lib/client/twofa/pushtoken/pushtoken.go b/lib/client/twofa/pushtoken/pushtoken.go index 477cc99e..ab67e2af 100644 --- a/lib/client/twofa/pushtoken/pushtoken.go +++ b/lib/client/twofa/pushtoken/pushtoken.go @@ -2,6 +2,7 @@ package pushtoken import ( "bufio" + "crypto/x509" "encoding/json" "errors" "fmt" @@ -20,6 +21,11 @@ import ( const vipCheckTimeoutSecs = 180 +func debugLogCert(messageSuffix string, cert *x509.Certificate, logger log.DebugLogger) { + logger.Debugf(2, "%s.issuer=%+v", messageSuffix, cert.Issuer) + logger.Debugf(2, "%s.subject=%+v", messageSuffix, cert.Subject) +} + func startGenericPush(client *http.Client, baseURL string, pushType string, @@ -42,6 +48,14 @@ func startGenericPush(client *http.Client, return err } defer pushStartResp.Body.Close() + + if pushStartResp.TLS != nil { + debugLogCert("startGenericPush peeerCerts[0]", pushStartResp.TLS.PeerCertificates[0], logger) + if pushStartResp.TLS.VerifiedChains != nil { + debugLogCert("startGenericPush verifiedcerts[0]", pushStartResp.TLS.VerifiedChains[0][0], logger) + } + } + // since we dont care about content we just consume it all. io.Copy(ioutil.Discard, pushStartResp.Body) if pushStartResp.StatusCode != 200 { @@ -94,6 +108,7 @@ func doGenericPushCheck(client *http.Client, baseURL string, pushType string, userAgentString string, + codeIsDone <-chan bool, logger log.DebugLogger, errorReturnDuration time.Duration) error { @@ -118,7 +133,15 @@ func doGenericPushCheck(client *http.Client, logger.Printf("") //To do a CR return nil } - time.Sleep(2 * time.Second) + select { + case codeSuccess := <-codeIsDone: + if codeSuccess { + return nil + } + continue + case <-time.After(2 * time.Second): + logger.Debugf(1, "doGenericPushCheck: timeout on checkGenericPollStatus loop") + } } err = errors.New("Vip Push Checked timeout out") @@ -178,7 +201,7 @@ func genericAuthenticateWithToken( defer loginResp.Body.Close() if loginResp.StatusCode != 200 { logger.Printf("got error from login call %s", loginResp.Status) - return err + return fmt.Errorf("Failed to authenticate with token") } loginJSONResponse := proto.LoginResponse{} @@ -203,14 +226,19 @@ func doGenericTokenPushAuthenticate( timeout := time.Duration(time.Duration(vipCheckTimeoutSecs) * time.Second) ch := make(chan error, 1) + doneCh := make(chan bool, 1) go func() { err := genericAuthenticateWithToken(client, baseURL, pushType, userAgentString, logger) + if err == nil { + doneCh <- true + } ch <- err }() go func() { err := doGenericPushCheck(client, baseURL, pushType, userAgentString, + doneCh, logger, timeout) ch <- err