From 46f9049fb48fd9ae2010b54b1f1b5cca74865082 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 16 Mar 2023 11:59:42 -0700 Subject: [PATCH] simplify error responses for KMS (#16793) --- cmd/api-errors.go | 7 +++++ internal/kms/errors.go | 29 ++++++++++++++++++ internal/kms/single-key.go | 61 +++++++++++++++++++++++++++++++------- internal/logger/logonce.go | 46 ++++++++++++++++++---------- 4 files changed, 117 insertions(+), 26 deletions(-) create mode 100644 internal/kms/errors.go diff --git a/cmd/api-errors.go b/cmd/api-errors.go index c77c9ca2b..a14a20cc1 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -38,6 +38,7 @@ import ( "github.com/minio/minio/internal/bucket/replication" "github.com/minio/minio/internal/config/dns" "github.com/minio/minio/internal/crypto" + "github.com/minio/minio/internal/kms" "github.com/minio/minio/internal/logger" objectlock "github.com/minio/minio/internal/bucket/object/lock" @@ -2317,6 +2318,12 @@ func toAPIError(ctx context.Context, err error) APIError { // any underlying errors if possible depending on // their internal error types. switch e := err.(type) { + case kms.Error: + apiErr = APIError{ + Description: e.Err.Error(), + Code: e.APICode, + HTTPStatusCode: e.HTTPStatusCode, + } case batchReplicationJobError: apiErr = APIError(e) case InvalidArgument: diff --git a/internal/kms/errors.go b/internal/kms/errors.go new file mode 100644 index 000000000..7900c0a23 --- /dev/null +++ b/internal/kms/errors.go @@ -0,0 +1,29 @@ +// Copyright (c) 2015-2023 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package kms + +// Error encapsulates S3 API error response fields. +type Error struct { + Err error + APICode string + HTTPStatusCode int +} + +func (e Error) Error() string { + return e.Err.Error() +} diff --git a/internal/kms/single-key.go b/internal/kms/single-key.go index b95db0d50..abe773e71 100644 --- a/internal/kms/single-key.go +++ b/internal/kms/single-key.go @@ -25,6 +25,7 @@ import ( "encoding/base64" "errors" "fmt" + "net/http" "strconv" "strings" @@ -107,14 +108,22 @@ func (kms secretKey) List() []kes.KeyInfo { } func (secretKey) Metrics(ctx context.Context) (kes.Metric, error) { - return kes.Metric{}, errors.New("kms: metrics are not supported") + return kes.Metric{}, Error{ + HTTPStatusCode: http.StatusNotImplemented, + APICode: "KMS.NotImplemented", + Err: errors.New("metrics are not supported"), + } } func (kms secretKey) CreateKey(_ context.Context, keyID string) error { if keyID == kms.keyID { return nil } - return fmt.Errorf("kms: creating custom key %q is not supported", keyID) + return Error{ + HTTPStatusCode: http.StatusNotImplemented, + APICode: "KMS.NotImplemented", + Err: fmt.Errorf("creating custom key %q is not supported", keyID), + } } func (kms secretKey) GenerateKey(_ context.Context, keyID string, context Context) (DEK, error) { @@ -122,7 +131,11 @@ func (kms secretKey) GenerateKey(_ context.Context, keyID string, context Contex keyID = kms.keyID } if keyID != kms.keyID { - return DEK{}, fmt.Errorf("kms: key %q does not exist", keyID) + return DEK{}, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.NotFoundException", + Err: fmt.Errorf("key %q does not exist", keyID), + } } iv, err := sioutil.Random(16) if err != nil { @@ -163,7 +176,11 @@ func (kms secretKey) GenerateKey(_ context.Context, keyID string, context Contex return DEK{}, err } default: - return DEK{}, errors.New("invalid algorithm: " + algorithm) + return DEK{}, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: errors.New("invalid algorithm: " + algorithm), + } } nonce, err := sioutil.Random(aead.NonceSize()) @@ -197,17 +214,29 @@ func (kms secretKey) GenerateKey(_ context.Context, keyID string, context Contex func (kms secretKey) DecryptKey(keyID string, ciphertext []byte, context Context) ([]byte, error) { if keyID != kms.keyID { - return nil, fmt.Errorf("kms: key %q does not exist", keyID) + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.NotFoundException", + Err: fmt.Errorf("key %q does not exist", keyID), + } } var encryptedKey encryptedKey json := jsoniter.ConfigCompatibleWithStandardLibrary if err := json.Unmarshal(ciphertext, &encryptedKey); err != nil { - return nil, err + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: err, + } } if n := len(encryptedKey.IV); n != 16 { - return nil, fmt.Errorf("kms: invalid iv size") + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: fmt.Errorf("invalid iv size: %d", n), + } } var aead cipher.AEAD @@ -235,17 +264,29 @@ func (kms secretKey) DecryptKey(keyID string, ciphertext []byte, context Context return nil, err } default: - return nil, fmt.Errorf("kms: invalid algorithm: %q", encryptedKey.Algorithm) + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: fmt.Errorf("invalid algorithm: %q", encryptedKey.Algorithm), + } } if n := len(encryptedKey.Nonce); n != aead.NonceSize() { - return nil, fmt.Errorf("kms: invalid nonce size %d", n) + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: fmt.Errorf("invalid nonce size %d", n), + } } associatedData, _ := context.MarshalText() plaintext, err := aead.Open(nil, encryptedKey.Nonce, encryptedKey.Bytes, associatedData) if err != nil { - return nil, fmt.Errorf("kms: encrypted key is not authentic") + return nil, Error{ + HTTPStatusCode: http.StatusBadRequest, + APICode: "KMS.InternalException", + Err: fmt.Errorf("encrypted key is not authentic"), + } } return plaintext, nil } diff --git a/internal/logger/logonce.go b/internal/logger/logonce.go index 63a3ec7c8..f982265e7 100644 --- a/internal/logger/logonce.go +++ b/internal/logger/logonce.go @@ -27,9 +27,14 @@ import ( // LogOnce provides the function type for logger.LogOnceIf() function type LogOnce func(ctx context.Context, err error, id string, errKind ...interface{}) +type onceErr struct { + Err error + Count int +} + // Holds a map of recently logged errors. type logOnceType struct { - IDMap map[string]error + IDMap map[string]onceErr sync.Mutex } @@ -41,12 +46,17 @@ func (l *logOnceType) logOnceConsoleIf(ctx context.Context, err error, id string nerr := unwrapErrs(err) l.Lock() shouldLog := true - prevErr, ok := l.IDMap[id] + prev, ok := l.IDMap[id] if !ok { - l.IDMap[id] = nerr - } else { + l.IDMap[id] = onceErr{ + Err: nerr, + Count: 1, + } + } else if prev.Err.Error() == nerr.Error() { // if errors are equal do not log. - shouldLog = prevErr.Error() != nerr.Error() + prev.Count++ + l.IDMap[id] = prev + shouldLog = false } l.Unlock() @@ -88,37 +98,41 @@ func (l *logOnceType) logOnceIf(ctx context.Context, err error, id string, errKi } nerr := unwrapErrs(err) - l.Lock() shouldLog := true - prevErr, ok := l.IDMap[id] + prev, ok := l.IDMap[id] if !ok { - l.IDMap[id] = nerr - } else { + l.IDMap[id] = onceErr{ + Err: nerr, + Count: 1, + } + } else if prev.Err.Error() == nerr.Error() { // if errors are equal do not log. - shouldLog = prevErr.Error() != nerr.Error() + prev.Count++ + l.IDMap[id] = prev + shouldLog = false } l.Unlock() if shouldLog { - LogIf(ctx, err, errKind...) + logIf(ctx, err, errKind...) } } // Cleanup the map every 30 minutes so that the log message is printed again for the user to notice. func (l *logOnceType) cleanupRoutine() { for { - l.Lock() - l.IDMap = make(map[string]error) - l.Unlock() + time.Sleep(time.Hour) - time.Sleep(30 * time.Minute) + l.Lock() + l.IDMap = make(map[string]onceErr) + l.Unlock() } } // Returns logOnceType func newLogOnceType() *logOnceType { - l := &logOnceType{IDMap: make(map[string]error)} + l := &logOnceType{IDMap: make(map[string]onceErr)} go l.cleanupRoutine() return l }