Skip to content

Commit

Permalink
Hardening code (#1057)
Browse files Browse the repository at this point in the history
* Introduce MaxBufferSize, and accept PrivateKeys where PublicKeys are expected

* be more pedantic

* appease linter

* use older atomic functions

* tweak for bazel
  • Loading branch information
lestrrat authored Jan 10, 2024
1 parent c1b9ed1 commit 3d934dc
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 30 deletions.
11 changes: 11 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ Changes
v2 has many incompatibilities with v1. To see the full list of differences between
v1 and v2, please read the Changes-v2.md file (https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes-v2.md)

v2.0.20 UNRELEASED
[New Features]
* [jwe] Added jwe.Settings(WithMaxBufferSize(int64)) to set the maximum size of
internal buffers. The default value is 256MB. Most users do not need to change
this value.

[Miscellaneous]
* Internal key conversions should now allow private keys to be used in place of
public keys. This would allow you to pass private keys where public keys are
expected.

v2.0.19 09 Jan 2024
[New Features]
* [jws] Added jws.IsVerificationError to check if the error returned by `jws.Verify`
Expand Down
55 changes: 35 additions & 20 deletions internal/keyconv/keyconv.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func RSAPrivateKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw rsa.PrivateKey
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce rsa.PrivateKey from %T: %w`, src, err)
return fmt.Errorf(`keyconv: failed to produce rsa.PrivateKey from %T: %w`, src, err)
}
src = &raw
}
Expand All @@ -30,7 +30,7 @@ func RSAPrivateKey(dst, src interface{}) error {
case *rsa.PrivateKey:
ptr = src
default:
return fmt.Errorf(`expected rsa.PrivateKey or *rsa.PrivateKey, got %T`, src)
return fmt.Errorf(`keyconv: expected rsa.PrivateKey or *rsa.PrivateKey, got %T`, src)
}

return blackmagic.AssignIfCompatible(dst, ptr)
Expand All @@ -41,21 +41,25 @@ func RSAPrivateKey(dst, src interface{}) error {
// `src` may be rsa.PublicKey, *rsa.PublicKey, or a jwk.Key
func RSAPublicKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw rsa.PublicKey
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce rsa.PublicKey from %T: %w`, src, err)
pk, err := jwk.PublicRawKeyOf(jwkKey)
if err != nil {
return fmt.Errorf(`keyconv: failed to produce public key from %T: %w`, src, err)
}
src = &raw
src = pk
}

var ptr *rsa.PublicKey
switch src := src.(type) {
case rsa.PrivateKey:
ptr = &src.PublicKey
case *rsa.PrivateKey:
ptr = &src.PublicKey
case rsa.PublicKey:
ptr = &src
case *rsa.PublicKey:
ptr = src
default:
return fmt.Errorf(`expected rsa.PublicKey or *rsa.PublicKey, got %T`, src)
return fmt.Errorf(`keyconv: expected rsa.PublicKey/rsa.PrivateKey or *rsa.PublicKey/*rsa.PrivateKey, got %T`, src)
}

return blackmagic.AssignIfCompatible(dst, ptr)
Expand All @@ -67,7 +71,7 @@ func ECDSAPrivateKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw ecdsa.PrivateKey
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce ecdsa.PrivateKey from %T: %w`, src, err)
return fmt.Errorf(`keyconv: failed to produce ecdsa.PrivateKey from %T: %w`, src, err)
}
src = &raw
}
Expand All @@ -79,7 +83,7 @@ func ECDSAPrivateKey(dst, src interface{}) error {
case *ecdsa.PrivateKey:
ptr = src
default:
return fmt.Errorf(`expected ecdsa.PrivateKey or *ecdsa.PrivateKey, got %T`, src)
return fmt.Errorf(`keyconv: expected ecdsa.PrivateKey or *ecdsa.PrivateKey, got %T`, src)
}
return blackmagic.AssignIfCompatible(dst, ptr)
}
Expand All @@ -88,21 +92,25 @@ func ECDSAPrivateKey(dst, src interface{}) error {
// non-pointer to a pointer
func ECDSAPublicKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw ecdsa.PublicKey
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce ecdsa.PublicKey from %T: %w`, src, err)
pk, err := jwk.PublicRawKeyOf(jwkKey)
if err != nil {
return fmt.Errorf(`keyconv: failed to produce public key from %T: %w`, src, err)
}
src = &raw
src = pk
}

var ptr *ecdsa.PublicKey
switch src := src.(type) {
case ecdsa.PrivateKey:
ptr = &src.PublicKey
case *ecdsa.PrivateKey:
ptr = &src.PublicKey
case ecdsa.PublicKey:
ptr = &src
case *ecdsa.PublicKey:
ptr = src
default:
return fmt.Errorf(`expected ecdsa.PublicKey or *ecdsa.PublicKey, got %T`, src)
return fmt.Errorf(`keyconv: expected ecdsa.PublicKey/ecdsa.PrivateKey or *ecdsa.PublicKey/*ecdsa.PrivateKey, got %T`, src)
}
return blackmagic.AssignIfCompatible(dst, ptr)
}
Expand All @@ -111,13 +119,13 @@ func ByteSliceKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw []byte
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce []byte from %T: %w`, src, err)
return fmt.Errorf(`keyconv: failed to produce []byte from %T: %w`, src, err)
}
src = raw
}

if _, ok := src.([]byte); !ok {
return fmt.Errorf(`expected []byte, got %T`, src)
return fmt.Errorf(`keyconv: expected []byte, got %T`, src)
}
return blackmagic.AssignIfCompatible(dst, src)
}
Expand Down Expand Up @@ -145,11 +153,18 @@ func Ed25519PrivateKey(dst, src interface{}) error {

func Ed25519PublicKey(dst, src interface{}) error {
if jwkKey, ok := src.(jwk.Key); ok {
var raw ed25519.PublicKey
if err := jwkKey.Raw(&raw); err != nil {
return fmt.Errorf(`failed to produce ed25519.PublicKey from %T: %w`, src, err)
pk, err := jwk.PublicRawKeyOf(jwkKey)
if err != nil {
return fmt.Errorf(`keyconv: failed to produce public key from %T: %w`, src, err)
}
src = &raw
src = pk
}

switch key := src.(type) {
case ed25519.PrivateKey:
src = key.Public()
case *ed25519.PrivateKey:
src = key.Public()
}

var ptr *ed25519.PublicKey
Expand Down
1 change: 1 addition & 0 deletions jwe/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ go_library(
"//internal/keyconv",
"//internal/pool",
"//jwa",
"//jwe/internal/aescbc",
"//jwe/internal/cipher",
"//jwe/internal/content_crypt",
"//jwe/internal/keyenc",
Expand Down
29 changes: 29 additions & 0 deletions jwe/internal/aescbc/aescbc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,42 @@ import (
"errors"
"fmt"
"hash"
"sync/atomic"
)

const (
NonceSize = 16
)

const defaultBufSize int64 = 256 * 1024 * 1024

// Grr, we would like to use atomic.Int64, but that's only available
// from Go 1.19. Yes, we will cut support for Go 1.19 at some point,
// but not today (probably going to up the minimum required Go version
// some time after 1.22 is released)
var maxBufSize int64

func init() {
atomic.StoreInt64(&maxBufSize, defaultBufSize)
}

func SetMaxBufferSize(siz int64) {
if siz <= 0 {
siz = defaultBufSize
}
atomic.StoreInt64(&maxBufSize, siz)
}

func pad(buf []byte, n int) []byte {
rem := n - len(buf)%n
if rem == 0 {
return buf
}

mbs := atomic.LoadInt64(&maxBufSize)
if int64(len(buf)+rem) > mbs {
panic(fmt.Errorf("failed to allocate buffer"))
}
newbuf := make([]byte, len(buf)+rem)
copy(newbuf, buf)

Expand Down Expand Up @@ -174,6 +198,11 @@ func ensureSize(dst []byte, n int) []byte {
// Seal fulfills the crypto.AEAD interface
func (c Hmac) Seal(dst, nonce, plaintext, data []byte) []byte {
ctlen := len(plaintext)
bufsiz := ctlen + c.Overhead()
mbs := atomic.LoadInt64(&maxBufSize)
if int64(bufsiz) > mbs {
panic(fmt.Errorf("failed to allocate buffer"))
}
ciphertext := make([]byte, ctlen+c.Overhead())[:ctlen]
copy(ciphertext, plaintext)
ciphertext = pad(ciphertext, c.blockCipher.BlockSize())
Expand Down
3 changes: 3 additions & 0 deletions jwe/jwe.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/lestrrat-go/jwx/v2/jwk"

"github.com/lestrrat-go/jwx/v2/jwa"
"github.com/lestrrat-go/jwx/v2/jwe/internal/aescbc"
"github.com/lestrrat-go/jwx/v2/jwe/internal/content_crypt"
"github.com/lestrrat-go/jwx/v2/jwe/internal/keyenc"
"github.com/lestrrat-go/jwx/v2/jwe/internal/keygen"
Expand All @@ -36,6 +37,8 @@ func Settings(options ...GlobalOption) {
switch option.Ident() {
case identMaxPBES2Count{}:
maxPBES2Count = option.Value().(int)
case identMaxBufferSize{}:
aescbc.SetMaxBufferSize(option.Value().(int64))
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions jwe/jwe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -959,3 +959,15 @@ func TestGHSA_7f9x_gw85_8grf(t *testing.T) {
}
}
}

func TestMaxBufferSize(t *testing.T) {
// NOTE: This has GLOBAL EFFECT
jwe.Settings(jwe.WithMaxBufferSize(1))
defer jwe.Settings(jwe.WithMaxBufferSize(0))

key, err := jwxtest.GenerateRsaJwk()
require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`)

_, err = jwe.Encrypt([]byte("Lorem Ipsum"), jwe.WithContentEncryption(jwa.A128CBC_HS256), jwe.WithKey(jwa.RSA_OAEP, key))
require.Error(t, err, `jwe.Encrypt should fail`)
}
13 changes: 12 additions & 1 deletion jwe/options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,15 @@ options:
comment: |
WithMaxPBES2Count specifies the maximum number of PBES2 iterations
to use when decrypting a message. If not specified, the default
value of 10,000 is used.
value of 10,000 is used.
This option has a global effect.
- ident: MaxBufferSize
interface: GlobalOption
argument_type: int64
comment: |
WithMaxBufferSize specifies the maximum buffer size for internal
calculations, such as when AES-CBC is performed. The default value is 256MB.
If set to an invalid value, the default value is used.
This option has a global effect.
16 changes: 16 additions & 0 deletions jwe/options_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions jwe/options_gen_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 23 additions & 9 deletions jwt/openid/birthdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"fmt"
"io"
"math"
"regexp"
"strconv"

Expand Down Expand Up @@ -57,9 +58,22 @@ func (b *BirthdateClaim) UnmarshalJSON(data []byte) error {
return nil
}

func tointptr(v int64) *int {
i := int(v)
return &i
var intSize int

func init() {
switch math.MaxInt {
case math.MaxInt16:
intSize = 16
case math.MaxInt32:
intSize = 32
case math.MaxInt64:
intSize = 64
}
}

func parseBirthdayInt(s string) int {
i, _ := strconv.ParseInt(s, 10, intSize)
return int(i)
}

var birthdateRx = regexp.MustCompile(`^(\d{4})-(\d{2})-(\d{2})$`)
Expand Down Expand Up @@ -100,23 +114,23 @@ func (b *BirthdateClaim) Accept(v interface{}) error {
// we can assume that strconv.ParseInt always succeeds.
// strconv.ParseInt (and strconv.ParseUint that it uses internally)
// only returns range errors, so we should be safe.
year, _ := strconv.ParseInt(v[indices[2]:indices[3]], 10, 64)
year := parseBirthdayInt(v[indices[2]:indices[3]])
if year <= 0 {
return fmt.Errorf(`failed to parse birthdate year`)
}
tmp.year = tointptr(year)
tmp.year = &year

month, _ := strconv.ParseInt(v[indices[4]:indices[5]], 10, 64)
month := parseBirthdayInt(v[indices[4]:indices[5]])
if month <= 0 {
return fmt.Errorf(`failed to parse birthdate month`)
}
tmp.month = tointptr(month)
tmp.month = &month

day, _ := strconv.ParseInt(v[indices[6]:indices[7]], 10, 64)
day := parseBirthdayInt(v[indices[6]:indices[7]])
if day <= 0 {
return fmt.Errorf(`failed to parse birthdate day`)
}
tmp.day = tointptr(day)
tmp.day = &day

*b = tmp
return nil
Expand Down

0 comments on commit 3d934dc

Please sign in to comment.