Skip to content

Commit

Permalink
add ErrDecrypt error (#130)
Browse files Browse the repository at this point in the history
This commit adds a `ErrDecrypt` that is returned by
the KES server when it fails to decrypt an encrypted
ciphertext.

It allows clients to reliably distinguish decryption
errors from other errors - like invalid ciphertext format
or internal server errors.

Signed-off-by: Andreas Auernhammer <hi@aead.dev>

Co-authored-by: Harshavardhana <harsha@minio.io>
  • Loading branch information
aead and harshavardhana committed Jun 26, 2021
1 parent f1f9507 commit f594e05
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
5 changes: 5 additions & 0 deletions error.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ var (
// ErrPolicyNotFound represents a KES server response returned when a client
// tries to access a policy which does not exist.
ErrPolicyNotFound Error = NewError(http.StatusNotFound, "policy does not exist")

// ErrDecrypt represents a KES server response returned when the server fails
// to decrypt an encrypted ciphertext. It may occur when a client uses the
// the wrong key or the ciphertext has been (maliciously) modified.
ErrDecrypt Error = NewError(http.StatusBadRequest, "decryption failed: ciphertext is not authentic")
)

// Error is the type of client-server API errors.
Expand Down
9 changes: 4 additions & 5 deletions internal/secret/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"encoding/json"
"errors"
"net/http"
"strconv"
"strings"

"github.com/minio/kes"
Expand Down Expand Up @@ -142,10 +141,10 @@ func (s Secret) Unwrap(ciphertext []byte, associatedData []byte) ([]byte, error)
}
var sealedSecret SealedSecret
if err := json.Unmarshal(ciphertext, &sealedSecret); err != nil {
return nil, err
return nil, kes.NewError(http.StatusBadRequest, "invalid ciphertext")
}
if n := len(sealedSecret.IV); n != 16 {
return nil, kes.NewError(http.StatusBadRequest, "invalid iv size "+strconv.Itoa(n))
return nil, kes.NewError(http.StatusBadRequest, "invalid iv size")
}

var aead cipher.AEAD
Expand Down Expand Up @@ -177,11 +176,11 @@ func (s Secret) Unwrap(ciphertext []byte, associatedData []byte) ([]byte, error)
}

if n := len(sealedSecret.Nonce); n != aead.NonceSize() {
return nil, kes.NewError(http.StatusBadRequest, "invalid nonce size "+strconv.Itoa(n))
return nil, kes.NewError(http.StatusBadRequest, "invalid nonce size")
}
plaintext, err := aead.Open(nil, sealedSecret.Nonce, sealedSecret.Bytes, associatedData)
if err != nil {
return nil, kes.NewError(http.StatusBadRequest, "ciphertext is not authentic")
return nil, kes.ErrDecrypt
}
return plaintext, nil
}
13 changes: 13 additions & 0 deletions internal/secret/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import (
"bytes"
"encoding/hex"
"fmt"
"net/http"
"testing"

"github.com/minio/kes"
"github.com/secure-io/sio-go/sioutil"
)

Expand Down Expand Up @@ -99,6 +101,7 @@ var secretUnwrapTests = []struct {
Ciphertext string
AssociatedData []byte
ShouldFail bool
Err error
}{
{ // 0
Ciphertext: `{"aead":"AES-256-GCM-HMAC-SHA-256","iv":"xLxIN3tSCkg2xMafuvwUwg==","nonce":"gu0mGwUkwcvMEoi5","bytes":"WVgRjeIJm3w50C/l+y7y2i6mbNg5NCAqN1zvOYWZKmc="}`,
Expand All @@ -112,36 +115,43 @@ var secretUnwrapTests = []struct {
Ciphertext: `{"aead":"AES-256-GCM","iv":"xLxIN3tSCkg2xMafuvwUwg==","nonce":"gu0mGwUkwcvMEoi5","bytes":"WVgRjeIJm3w50C/l+y7y2i6mbNg5NCAqN1zvOYWZKmc="}`,
AssociatedData: nil,
ShouldFail: true, // Invalid algorithm
Err: kes.NewError(http.StatusUnprocessableEntity, "unsupported cryptographic algorithm"),
},
{ // 3
Ciphertext: `{"aead":"AES-256-GCM-HMAC-SHA-256","iv":"EjOY4JKqjIrPmQ5z1KSR8zlhggY=","nonce":"gu0mGwUkwcvMEoi5","bytes":"WVgRjeIJm3w50C/l+y7y2i6mbNg5NCAqN1zvOYWZKmc="}`,
AssociatedData: nil,
ShouldFail: true, // invalid IV length
Err: kes.NewError(http.StatusBadRequest, "invalid iv size"),
},
{ // 4
Ciphertext: `{"aead":"ChaCha20Poly1305","iv":"s3fSZ6vk5m+DfQA8yZWeUg==","nonce":"SXAbms731/c=","bytes":"cw22HjLq/4cx8507SW4hhSrYbDiMuRao4b5+GE+XfbE="}`,
AssociatedData: nil,
ShouldFail: true, // invalid nonce length
Err: kes.NewError(http.StatusBadRequest, "invalid nonce size"),
},
{ // 5
Ciphertext: `{"aead":"AES-256-GCM-HMAC-SHA-256","iv":"xLxIN3tSCkg2xMafuvwUwg==","nonce":"efY+4kYF9n8=","bytes":"WVgRjeIJm3w50C/l+y7y2i6mbNg5NCAqN1zvOYWZKmc="}`,
AssociatedData: nil,
ShouldFail: true, // invalid nonce length
Err: kes.NewError(http.StatusBadRequest, "invalid nonce size"),
},
{ // 6
Ciphertext: `{"aead":"AES-256-GCM-HMAC-SHA-256","iv":"xLxIN3tSCkg2xMafuvwUwg==","nonce":"gu0mGwUkwcvMEoi5","bytes":"QTza1g5oX3f9cGJMbY1xJwWPj1F7R2VnNl6XpFKYQy0="}`,
AssociatedData: nil,
ShouldFail: true, // ciphertext not authentic
Err: kes.ErrDecrypt,
},
{ // 7
Ciphertext: `{"aead":"ChaCha20Poly1305","iv":"s3fSZ6vk5m+DfQA8yZWeUg==","nonce":"8/kHMnCMs3h9NZ2a","bytes":"TTi8pkO+Jh1JWAHvPxZeUk/iVoBPUCE4ZSVGBy3fW2s="}`,
AssociatedData: nil,
ShouldFail: true, // ciphertext not authentic
Err: kes.ErrDecrypt,
},
{ // 8
Ciphertext: `{"aead":"AES-256-GCM-HMAC-SHA-256" "iv":"xLxIN3tSCkg2xMafuvwUwg==","nonce":"gu0mGwUkwcvMEoi5","bytes":"WVgRjeIJm3w50C/l+y7y2i6mbNg5NCAqN1zvOYWZKmc="}`,
AssociatedData: nil,
ShouldFail: true, // invalid JSON
Err: kes.NewError(http.StatusBadRequest, "invalid ciphertext"),
},
}

Expand All @@ -156,6 +166,9 @@ func TestSecrectUnwrap(t *testing.T) {
if err == nil && test.ShouldFail {
t.Fatalf("Test %d: Expected to fail but succeeded", i)
}
if test.ShouldFail && err != test.Err {
t.Fatalf("Test %d: Invalid error response: got %v - want %v", i, err, test.Err)
}
if !test.ShouldFail && !bytes.Equal(plaintext, Plaintext) {
t.Fatalf("Test %d: Plaintext mismatch: got %x - want %x", i, plaintext, Plaintext)
}
Expand Down

0 comments on commit f594e05

Please sign in to comment.