From f79a4ef4d0dc3e6562cad0d1d1db674bc8c75531 Mon Sep 17 00:00:00 2001 From: Anis Eleuch Date: Tue, 28 May 2024 18:19:04 +0100 Subject: [PATCH] policy: More defensive code validating svc:DurationSeconds (#19820) This does not fix any current issue, but merging https://github.com/minio/madmin-go/pull/282 can lose the validation of the service account expiration time. Add more defensive code for now. In the future, we should avoid doing validation in another library. --- cmd/admin-handlers-users.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 8a88094fd..8488a95ba 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -830,7 +830,11 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re } condValues := getConditionValues(r, "", cred) - addExpirationToCondValues(updateReq.NewExpiration, condValues) + err = addExpirationToCondValues(updateReq.NewExpiration, condValues) + if err != nil { + writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) + return + } // Permission checks: // @@ -2459,11 +2463,16 @@ func (a adminAPIHandlers) ImportIAM(w http.ResponseWriter, r *http.Request) { } } -func addExpirationToCondValues(exp *time.Time, condValues map[string][]string) { - if exp == nil { - return +func addExpirationToCondValues(exp *time.Time, condValues map[string][]string) error { + if exp == nil || exp.IsZero() || exp.Equal(timeSentinel) { + return nil } - condValues["DurationSeconds"] = []string{strconv.FormatInt(int64(exp.Sub(time.Now()).Seconds()), 10)} + dur := exp.Sub(time.Now()) + if dur <= 0 { + return errors.New("unsupported expiration time") + } + condValues["DurationSeconds"] = []string{strconv.FormatInt(int64(dur.Seconds()), 10)} + return nil } func commonAddServiceAccount(r *http.Request) (context.Context, auth.Credentials, newServiceAccountOpts, madmin.AddServiceAccountReq, string, APIError) { @@ -2528,7 +2537,10 @@ func commonAddServiceAccount(r *http.Request) (context.Context, auth.Credentials } condValues := getConditionValues(r, "", cred) - addExpirationToCondValues(createReq.Expiration, condValues) + err = addExpirationToCondValues(createReq.Expiration, condValues) + if err != nil { + return ctx, auth.Credentials{}, newServiceAccountOpts{}, madmin.AddServiceAccountReq{}, "", toAdminAPIErr(ctx, err) + } // Check if action is allowed if creating access key for another user // Check if action is explicitly denied if for self