Merge pull request #829 from ericchiang/fix-keys-expiry
server: fix expiry detection for verification keys
This commit is contained in:
commit
3e5480a859
2 changed files with 119 additions and 10 deletions
|
@ -22,8 +22,9 @@ type rotationStrategy struct {
|
||||||
// Time between rotations.
|
// Time between rotations.
|
||||||
rotationFrequency time.Duration
|
rotationFrequency time.Duration
|
||||||
|
|
||||||
// After being rotated how long can a key validate signatues?
|
// After being rotated how long should the key be kept around for validating
|
||||||
verifyFor time.Duration
|
// signatues?
|
||||||
|
idTokenValidFor time.Duration
|
||||||
|
|
||||||
// Keys are always RSA keys. Though cryptopasta recommends ECDSA keys, not every
|
// Keys are always RSA keys. Though cryptopasta recommends ECDSA keys, not every
|
||||||
// client may support these (e.g. github.com/coreos/go-oidc/oidc).
|
// client may support these (e.g. github.com/coreos/go-oidc/oidc).
|
||||||
|
@ -35,17 +36,17 @@ func staticRotationStrategy(key *rsa.PrivateKey) rotationStrategy {
|
||||||
return rotationStrategy{
|
return rotationStrategy{
|
||||||
// Setting these values to 100 years is easier than having a flag indicating no rotation.
|
// Setting these values to 100 years is easier than having a flag indicating no rotation.
|
||||||
rotationFrequency: time.Hour * 8760 * 100,
|
rotationFrequency: time.Hour * 8760 * 100,
|
||||||
verifyFor: time.Hour * 8760 * 100,
|
idTokenValidFor: time.Hour * 8760 * 100,
|
||||||
key: func() (*rsa.PrivateKey, error) { return key, nil },
|
key: func() (*rsa.PrivateKey, error) { return key, nil },
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// defaultRotationStrategy returns a strategy which rotates keys every provided period,
|
// defaultRotationStrategy returns a strategy which rotates keys every provided period,
|
||||||
// holding onto the public parts for some specified amount of time.
|
// holding onto the public parts for some specified amount of time.
|
||||||
func defaultRotationStrategy(rotationFrequency, verifyFor time.Duration) rotationStrategy {
|
func defaultRotationStrategy(rotationFrequency, idTokenValidFor time.Duration) rotationStrategy {
|
||||||
return rotationStrategy{
|
return rotationStrategy{
|
||||||
rotationFrequency: rotationFrequency,
|
rotationFrequency: rotationFrequency,
|
||||||
verifyFor: verifyFor,
|
idTokenValidFor: idTokenValidFor,
|
||||||
key: func() (*rsa.PrivateKey, error) {
|
key: func() (*rsa.PrivateKey, error) {
|
||||||
return rsa.GenerateKey(rand.Reader, 2048)
|
return rsa.GenerateKey(rand.Reader, 2048)
|
||||||
},
|
},
|
||||||
|
@ -128,11 +129,14 @@ func (k keyRotater) rotate() error {
|
||||||
return storage.Keys{}, errors.New("keys already rotated")
|
return storage.Keys{}, errors.New("keys already rotated")
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove expired verification keys.
|
expired := func(key storage.VerificationKey) bool {
|
||||||
i := 0
|
return tNow.After(key.Expiry)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Remove any verification keys that have expired.
|
||||||
|
i := 0
|
||||||
for _, key := range keys.VerificationKeys {
|
for _, key := range keys.VerificationKeys {
|
||||||
if !key.Expiry.After(tNow) {
|
if !expired(key) {
|
||||||
keys.VerificationKeys[i] = key
|
keys.VerificationKeys[i] = key
|
||||||
i++
|
i++
|
||||||
}
|
}
|
||||||
|
@ -140,10 +144,15 @@ func (k keyRotater) rotate() error {
|
||||||
keys.VerificationKeys = keys.VerificationKeys[:i]
|
keys.VerificationKeys = keys.VerificationKeys[:i]
|
||||||
|
|
||||||
if keys.SigningKeyPub != nil {
|
if keys.SigningKeyPub != nil {
|
||||||
// Move current signing key to a verification only key.
|
// Move current signing key to a verification only key, throwing
|
||||||
|
// away the private part.
|
||||||
verificationKey := storage.VerificationKey{
|
verificationKey := storage.VerificationKey{
|
||||||
PublicKey: keys.SigningKeyPub,
|
PublicKey: keys.SigningKeyPub,
|
||||||
Expiry: tNow.Add(k.strategy.verifyFor),
|
// After demoting the signing key, keep the token around for at least
|
||||||
|
// the amount of time an ID Token is valid for. This ensures the
|
||||||
|
// verification key won't expire until all ID Tokens it's signed
|
||||||
|
// expired as well.
|
||||||
|
Expiry: tNow.Add(k.strategy.idTokenValidFor),
|
||||||
}
|
}
|
||||||
keys.VerificationKeys = append(keys.VerificationKeys, verificationKey)
|
keys.VerificationKeys = append(keys.VerificationKeys, verificationKey)
|
||||||
}
|
}
|
||||||
|
|
|
@ -1 +1,101 @@
|
||||||
package server
|
package server
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"sort"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/Sirupsen/logrus"
|
||||||
|
"github.com/coreos/dex/storage"
|
||||||
|
"github.com/coreos/dex/storage/memory"
|
||||||
|
)
|
||||||
|
|
||||||
|
func signingKeyID(t *testing.T, s storage.Storage) string {
|
||||||
|
keys, err := s.GetKeys()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
return keys.SigningKey.KeyID
|
||||||
|
}
|
||||||
|
|
||||||
|
func verificationKeyIDs(t *testing.T, s storage.Storage) (ids []string) {
|
||||||
|
keys, err := s.GetKeys()
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
for _, key := range keys.VerificationKeys {
|
||||||
|
ids = append(ids, key.PublicKey.KeyID)
|
||||||
|
}
|
||||||
|
return ids
|
||||||
|
}
|
||||||
|
|
||||||
|
// slicesEq compare two string slices without modifying the ordering
|
||||||
|
// of the slices.
|
||||||
|
func slicesEq(s1, s2 []string) bool {
|
||||||
|
if len(s1) != len(s2) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
cp := func(s []string) []string {
|
||||||
|
c := make([]string, len(s))
|
||||||
|
copy(c, s)
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
cp1 := cp(s1)
|
||||||
|
cp2 := cp(s2)
|
||||||
|
sort.Strings(cp1)
|
||||||
|
sort.Strings(cp2)
|
||||||
|
|
||||||
|
for i, el := range cp1 {
|
||||||
|
if el != cp2[i] {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestKeyRotater(t *testing.T) {
|
||||||
|
now := time.Now()
|
||||||
|
|
||||||
|
delta := time.Millisecond
|
||||||
|
rotationFrequency := time.Second * 5
|
||||||
|
validFor := time.Second * 21
|
||||||
|
|
||||||
|
// Only the last 5 verification keys are expected to be kept around.
|
||||||
|
maxVerificationKeys := 5
|
||||||
|
|
||||||
|
l := &logrus.Logger{
|
||||||
|
Out: os.Stderr,
|
||||||
|
Formatter: &logrus.TextFormatter{DisableColors: true},
|
||||||
|
Level: logrus.DebugLevel,
|
||||||
|
}
|
||||||
|
|
||||||
|
r := &keyRotater{
|
||||||
|
Storage: memory.New(l),
|
||||||
|
strategy: defaultRotationStrategy(rotationFrequency, validFor),
|
||||||
|
now: func() time.Time { return now },
|
||||||
|
logger: l,
|
||||||
|
}
|
||||||
|
|
||||||
|
var expVerificationKeys []string
|
||||||
|
|
||||||
|
for i := 0; i < 10; i++ {
|
||||||
|
now = now.Add(rotationFrequency + delta)
|
||||||
|
if err := r.rotate(); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
got := verificationKeyIDs(t, r.Storage)
|
||||||
|
|
||||||
|
if !slicesEq(expVerificationKeys, got) {
|
||||||
|
t.Errorf("after %d rotation, expected varification keys %q, got %q", i+1, expVerificationKeys, got)
|
||||||
|
}
|
||||||
|
|
||||||
|
expVerificationKeys = append(expVerificationKeys, signingKeyID(t, r.Storage))
|
||||||
|
if n := len(expVerificationKeys); n > maxVerificationKeys {
|
||||||
|
expVerificationKeys = expVerificationKeys[n-maxVerificationKeys:]
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
Reference in a new issue