Skip to content

Commit

Permalink
auth: psk fixup
Browse files Browse the repository at this point in the history
this commit no longer requires a base64 PSK in the config, decoding it
became an issue when interoping with Quay.

commit also simplifies the PSK check to look at multiple issuers. This
is mostly done for more accurate logging.

Signed-off-by: ldelossa <ldelossa@redhat.com>
  • Loading branch information
ldelossa authored and ldelossa committed Sep 22, 2020
1 parent 15f3755 commit f00698b
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 41 deletions.
6 changes: 3 additions & 3 deletions Documentation/reference/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -513,11 +513,11 @@ a string value
A shared key distributed between all parties signing and verifying JWTs.
```

#### &emsp;&emsp;issuer: ""
#### &emsp;&emsp;issuer: []string
```
a string value
a list of string value
The "Issuer" key is what the service expects to verify as the "issuer" claim.
A list of issuers to verify. An empty list will accept any issuer in a jwt claim.
```

### &emsp;keyserver: \<object\>
Expand Down
8 changes: 4 additions & 4 deletions config/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ func (a *AuthKeyserver) UnmarshalYAML(f func(interface{}) error) error {
//
// The "Issuer" key is what the service expects to verify as the "issuer" claim.
type AuthPSK struct {
Key []byte `yaml:"key" json:"key"`
Issuer string `yaml:"iss" json:"issuer"`
Key []byte `yaml:"key" json:"key"`
Issuer []string `yaml:"iss" json:"issuer"`
}

// UnmarshalYAML implements yaml.Unmarshaler.
func (a *AuthPSK) UnmarshalYAML(f func(interface{}) error) error {
var m struct {
Issuer string `yaml:"iss" json:"issuer"`
Key string `yaml:"key" json:"key"`
Issuer []string `yaml:"iss" json:"issuer"`
Key string `yaml:"key" json:"key"`
}
if err := f(&m); err != nil {
return nil
Expand Down
5 changes: 3 additions & 2 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,12 @@ func TestAuthUnmarshal(t *testing.T) {
In: `---
key: >-
ZGVhZGJlZWZkZWFkYmVlZg==
iss: iss
iss:
- iss
`,
Want: config.AuthPSK{
Key: []byte("deadbeefdeadbeef"),
Issuer: "iss",
Issuer: []string{"iss"},
},
},
}
Expand Down
14 changes: 7 additions & 7 deletions httptransport/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@ func authHandler(cfg *config.Config, next http.Handler) (http.Handler, error) {
}
checks = append(checks, ks)
if cfg.Intraservice != nil {
psk, err := auth.NewPSK(cfg.Intraservice, IntraserviceIssuer)
psk, err := auth.NewPSK(cfg.Intraservice, []string{IntraserviceIssuer})
if err != nil {
return nil, fmt.Errorf("failed to initialize quay keyserver: %w", err)
}
checks = append(checks, psk)
}
case cfg.Auth.PSK != nil:
cfg := cfg.Auth.PSK
intra, err := auth.NewPSK(cfg.Key, IntraserviceIssuer)
if err != nil {
return nil, err
}
psk, err := auth.NewPSK(cfg.Key, cfg.Issuer)
issuers := make([]string, 0, 1+len(cfg.Issuer))
issuers = append(issuers, IntraserviceIssuer)
issuers = append(issuers, cfg.Issuer...)

psk, err := auth.NewPSK(cfg.Key, issuers)
if err != nil {
return nil, err
}
checks = append(checks, intra, psk)
checks = append(checks, psk)
default:
return next, nil
}
Expand Down
6 changes: 3 additions & 3 deletions httptransport/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func TestAuth(t *testing.T) {
Config: config.Config{
Auth: config.Auth{
PSK: &config.AuthPSK{
Issuer: `sweet-bro`,
Issuer: []string{`sweet-bro`},
Key: fakeKey,
},
},
Expand All @@ -117,7 +117,7 @@ func TestAuth(t *testing.T) {
Config: config.Config{
Auth: config.Auth{
PSK: &config.AuthPSK{
Issuer: `sweet-bro`,
Issuer: []string{`sweet-bro`},
Key: fakeKey,
},
},
Expand All @@ -143,7 +143,7 @@ func TestAuth(t *testing.T) {
Config: config.Config{
Auth: config.Auth{
PSK: &config.AuthPSK{
Issuer: `sweet-bro`,
Issuer: []string{`sweet-bro`},
Key: fakeKey,
},
},
Expand Down
30 changes: 26 additions & 4 deletions middleware/auth/httpauth_psk.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"time"

"github.com/rs/zerolog"
"gopkg.in/square/go-jose.v2/jwt"
)

Expand All @@ -14,11 +15,11 @@ import (
// will be validated against a pre-shared-key.
type PSK struct {
key []byte
iss string
iss []string
}

// NewPSK returns an instance of a PSK
func NewPSK(key []byte, issuer string) (*PSK, error) {
func NewPSK(key []byte, issuer []string) (*PSK, error) {
return &PSK{
key: key,
iss: issuer,
Expand All @@ -27,23 +28,44 @@ func NewPSK(key []byte, issuer string) (*PSK, error) {

// Check implements AuthCheck
func (p *PSK) Check(_ context.Context, r *http.Request) bool {
ctx := r.Context()
log := zerolog.Ctx(ctx).With().
Str("component", "middleware/auth/PSK.Check").
Logger()
ctx = log.WithContext(ctx)

wt, ok := fromHeader(r)
if !ok {
log.Debug().Msg("failed to retrieve jwt from header")
return false
}
tok, err := jwt.ParseSigned(wt)
if err != nil {
log.Debug().Err(err).Msg("failed to parse jwt")
return false
}
cl := jwt.Claims{}
if err := tok.Claims(p.key, &cl); err != nil {
log.Debug().Err(err).Msg("failed to parse jwt")
return false
}

if err := cl.ValidateWithLeeway(jwt.Expected{
Issuer: p.iss,
Time: time.Now(),
Time: time.Now(),
}, 15*time.Second); err != nil {
log.Debug().Err(err).Str("iss", cl.Issuer).Msg("could not validate claims")
return false
}

for i, iss := range p.iss {
if iss == cl.Issuer {
break
}
if i == len(p.iss)-1 {
log.Debug().Err(err).Str("iss", cl.Issuer).Msg("could not verify issuer")
return false
}
}

return true
}
43 changes: 25 additions & 18 deletions middleware/auth/httpauth_psk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

type pskTestcase struct {
key []byte
issuer string
issuer []string
nonce string
alg jose.SignatureAlgorithm
}
Expand All @@ -35,33 +35,40 @@ var signAlgo = []jose.SignatureAlgorithm{
jose.HS512,
}

// implements the Generate interface from testing/quick package.
func (tc *pskTestcase) Generate(rand *rand.Rand, sz int) reflect.Value {
b := make([]byte, sz)
n := &pskTestcase{
key: make([]byte, sz),
alg: signAlgo[rand.Intn(len(signAlgo))],
t := &pskTestcase{
key: make([]byte, sz),
alg: signAlgo[rand.Intn(len(signAlgo))],
issuer: make([]string, 0),
}
switch n, err := rand.Read(n.key); {
switch n, err := rand.Read(t.key); {
case n != sz:
panic(fmt.Errorf("read %d, expected %d", n, sz))
case err != nil:
panic(err)
}

for _, t := range []*string{
&n.issuer,
&n.nonce,
} {
switch n, err := rand.Read(b); {
case n != sz:
panic(fmt.Errorf("read %d, expected %d", n, sz))
case err != nil:
panic(err)
}
*t = base64.StdEncoding.EncodeToString(b)
switch n, err := rand.Read(b); {
case n != sz:
panic(fmt.Errorf("read %d, expected %d", n, sz))
case err != nil:
panic(err)
default:
t.issuer = append(t.issuer, base64.StdEncoding.EncodeToString(b))
}

switch n, err := rand.Read(b); {
case n != sz:
panic(fmt.Errorf("read %d, expected %d", n, sz))
case err != nil:
panic(err)
default:
t.nonce = base64.StdEncoding.EncodeToString(b)
}

return reflect.ValueOf(n)
return reflect.ValueOf(t)
}

func (tc *pskTestcase) Handler(t *testing.T) http.Handler {
Expand Down Expand Up @@ -96,7 +103,7 @@ func roundtrips(t *testing.T) func(*pskTestcase) bool {

// Mint the jwt.
tok, err := jwt.Signed(s).Claims(&jwt.Claims{
Issuer: tc.issuer,
Issuer: tc.issuer[0],
Expiry: jwt.NewNumericDate(now.Add(time.Minute)),
IssuedAt: jwt.NewNumericDate(now),
NotBefore: jwt.NewNumericDate(now),
Expand Down

0 comments on commit f00698b

Please sign in to comment.