From 5c2af3f7928281e37e1bd11257e3966575ba381f Mon Sep 17 00:00:00 2001 From: poornas Date: Mon, 30 Sep 2019 14:05:19 -0700 Subject: [PATCH] Add more context to error messages in STS handlers(#8304) --- cmd/sts-errors.go | 15 ++++++- cmd/sts-handlers.go | 105 ++++++++++++++++---------------------------- 2 files changed, 51 insertions(+), 69 deletions(-) diff --git a/cmd/sts-errors.go b/cmd/sts-errors.go index 1e605b26a..c35d2560c 100644 --- a/cmd/sts-errors.go +++ b/cmd/sts-errors.go @@ -17,16 +17,27 @@ package cmd import ( + "context" "encoding/xml" + "fmt" "net/http" xhttp "github.com/minio/minio/cmd/http" + "github.com/minio/minio/cmd/logger" ) // writeSTSErrorRespone writes error headers -func writeSTSErrorResponse(w http.ResponseWriter, err STSError) { +func writeSTSErrorResponse(ctx context.Context, w http.ResponseWriter, errCode STSErrorCode, errCtxt error) { + err := stsErrCodes.ToSTSErr(errCode) // Generate error response. - stsErrorResponse := getSTSErrorResponse(err, w.Header().Get(xhttp.AmzRequestID)) + stsErrorResponse := STSErrorResponse{} + stsErrorResponse.Error.Code = err.Code + stsErrorResponse.RequestID = w.Header().Get(xhttp.AmzRequestID) + stsErrorResponse.Error.Message = err.Description + if errCtxt != nil { + stsErrorResponse.Error.Message = fmt.Sprintf("%v", errCtxt) + } + logger.LogIf(ctx, errCtxt) encodedErrorResponse := encodeResponse(stsErrorResponse) writeResponse(w, err.HTTPStatusCode, encodedErrorResponse, mimeXML) } diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index fe65e7c20..57d2db9c0 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -132,19 +132,16 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { user, stsErr := checkAssumeRoleAuth(ctx, r) if stsErr != ErrSTSNone { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(stsErr)) + writeSTSErrorResponse(ctx, w, stsErr, nil) return } - if err := r.ParseForm(); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } if r.Form.Get("Version") != stsAPIVersion { - logger.LogIf(ctx, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSMissingParameter)) + writeSTSErrorResponse(ctx, w, ErrSTSMissingParameter, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) return } @@ -152,8 +149,7 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { switch action { case assumeRole: default: - logger.LogIf(ctx, fmt.Errorf("Unsupported action %s", action)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Unsupported action %s", action)) return } @@ -165,20 +161,20 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { // The plain text that you use for both inline and managed session // policies shouldn't exceed 2048 characters. if len(sessionPolicyStr) > 2048 { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Session policy shouldn't exceed 2048 characters")) return } if len(sessionPolicyStr) > 0 { sessionPolicy, err := iampolicy.ParseConfig(bytes.NewReader([]byte(sessionPolicyStr))) if err != nil { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } // Version in policy must not be empty if sessionPolicy.Version == "" { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Version cannot be empty expecting '2012-10-17'")) return } } @@ -187,20 +183,13 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { m := make(map[string]interface{}) m["exp"], err = validator.GetDefaultExpiration(r.Form.Get("DurationSeconds")) if err != nil { - switch err { - case validator.ErrInvalidDuration: - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) - default: - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) - } + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } policies, err := globalIAMSys.PolicyDBGet(user.AccessKey, false) if err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } @@ -221,15 +210,13 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { secret := globalServerConfig.GetCredential().SecretKey cred, err := auth.GetNewCredentialsWithMetadata(m, secret) if err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return } // Set the newly generated credentials. if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return } @@ -256,14 +243,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ // Parse the incoming form data. if err := r.ParseForm(); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } if r.Form.Get("Version") != stsAPIVersion { - logger.LogIf(ctx, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSMissingParameter)) + writeSTSErrorResponse(ctx, w, ErrSTSMissingParameter, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) return } @@ -271,8 +256,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ switch action { case clientGrants, webIdentity: default: - logger.LogIf(ctx, fmt.Errorf("Unsupported action %s", action)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Unsupported action %s", action)) return } @@ -280,14 +264,13 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ defer logger.AuditLog(w, r, action, nil) if globalIAMValidators == nil { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSNotInitialized)) + writeSTSErrorResponse(ctx, w, ErrSTSNotInitialized, errServerNotInitialized) return } v, err := globalIAMValidators.Get("jwt") if err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } @@ -302,17 +285,16 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ case validator.ErrTokenExpired: switch action { case clientGrants: - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSClientGrantsExpiredToken)) + writeSTSErrorResponse(ctx, w, ErrSTSClientGrantsExpiredToken, err) case webIdentity: - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSWebIdentityExpiredToken)) + writeSTSErrorResponse(ctx, w, ErrSTSWebIdentityExpiredToken, err) } return case validator.ErrInvalidDuration: - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } @@ -321,20 +303,20 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ // The plain text that you use for both inline and managed session // policies shouldn't exceed 2048 characters. if len(sessionPolicyStr) > 2048 { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Session policy should not exceed 2048 characters")) return } if len(sessionPolicyStr) > 0 { sessionPolicy, err := iampolicy.ParseConfig(bytes.NewReader([]byte(sessionPolicyStr))) if err != nil { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } // Version in policy must not be empty if sessionPolicy.Version == "" { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Invalid session policy version")) return } } @@ -346,8 +328,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ secret := globalServerConfig.GetCredential().SecretKey cred, err := auth.GetNewCredentialsWithMetadata(m, secret) if err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return } @@ -367,8 +348,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithJWT(w http.ResponseWriter, r *http.Requ // Set the newly generated credentials. if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return } @@ -430,14 +410,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * // Parse the incoming form data. if err := r.ParseForm(); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } if r.Form.Get("Version") != stsAPIVersion { - logger.LogIf(ctx, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSMissingParameter)) + writeSTSErrorResponse(ctx, w, ErrSTSMissingParameter, fmt.Errorf("Invalid STS API version %s, expecting %s", r.Form.Get("Version"), stsAPIVersion)) return } @@ -445,8 +423,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * switch action { case ldapIdentity: default: - logger.LogIf(ctx, fmt.Errorf("Unsupported action %s", action)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Unsupported action %s", action)) return } @@ -457,7 +434,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * ldapPassword := r.Form.Get("LDAPPassword") if ldapUsername == "" || ldapPassword == "" { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSMissingParameter)) + writeSTSErrorResponse(ctx, w, ErrSTSMissingParameter, fmt.Errorf("LDAPUsername and LDAPPassword cannot be empty")) return } @@ -466,33 +443,31 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * // The plain text that you use for both inline and managed session // policies shouldn't exceed 2048 characters. if len(sessionPolicyStr) > 2048 { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Session policy should not exceed 2048 characters")) return } if len(sessionPolicyStr) > 0 { sessionPolicy, err := iampolicy.ParseConfig(bytes.NewReader([]byte(sessionPolicyStr))) if err != nil { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } // Version in policy must not be empty if sessionPolicy.Version == "" { - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("Version needs to be specified in session policy")) return } } ldapConn, err := globalServerConfig.LDAPServerConfig.Connect() if err != nil { - logger.LogIf(ctx, fmt.Errorf("LDAP server connection failure: %v", err)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("LDAP server connection failure: %v", err)) return } if ldapConn == nil { - logger.LogIf(ctx, fmt.Errorf("LDAP server not configured: %v", err)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("LDAP server not configured: %v", err)) return } @@ -503,8 +478,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * // Bind with user credentials to validate the password err = ldapConn.Bind(usernameDN, ldapPassword) if err != nil { - logger.LogIf(ctx, fmt.Errorf("LDAP authentication failure: %v", err)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("LDAP authentication failure: %v", err)) return } @@ -530,8 +504,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * sr, err := ldapConn.Search(searchRequest) if err != nil { - logger.LogIf(ctx, fmt.Errorf("LDAP search failure: %v", err)) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInvalidParameterValue)) + writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("LDAP search failure: %v", err)) return } for _, entry := range sr.Entries { @@ -554,16 +527,14 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * secret := globalServerConfig.GetCredential().SecretKey cred, err := auth.GetNewCredentialsWithMetadata(m, secret) if err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return } policyName := "" // Set the newly generated credentials. if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil { - logger.LogIf(ctx, err) - writeSTSErrorResponse(w, stsErrCodes.ToSTSErr(ErrSTSInternalError)) + writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err) return }