Skip to content

Commit

Permalink
simplify error responses for KMS (minio#16793)
Browse files Browse the repository at this point in the history
  • Loading branch information
harshavardhana committed Mar 16, 2023
1 parent 58266c9 commit 46f9049
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 25 deletions.
7 changes: 7 additions & 0 deletions cmd/api-errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down
29 changes: 29 additions & 0 deletions internal/kms/errors.go
Original file line number Diff line number Diff line change
@@ -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 <http://www.gnu.org/licenses/>.

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()
}
61 changes: 51 additions & 10 deletions internal/kms/single-key.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"encoding/base64"
"errors"
"fmt"
"net/http"
"strconv"
"strings"

Expand Down Expand Up @@ -107,22 +108,34 @@ 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) {
if keyID == "" {
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 {
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
44 changes: 29 additions & 15 deletions internal/logger/logonce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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()

Expand Down Expand Up @@ -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 {
time.Sleep(time.Hour)

l.Lock()
l.IDMap = make(map[string]error)
l.IDMap = make(map[string]onceErr)
l.Unlock()

time.Sleep(30 * time.Minute)
}
}

// Returns logOnceType
func newLogOnceType() *logOnceType {
l := &logOnceType{IDMap: make(map[string]error)}
l := &logOnceType{IDMap: make(map[string]onceErr)}
go l.cleanupRoutine()
return l
}
Expand Down

0 comments on commit 46f9049

Please sign in to comment.