Merge pull request #9883 from joelegasse/jwt

auth: Support all JWT algorithms
etcd-io · Jun 27, 2018 · 54e2511 · 54e2511
2 parents eb0a10d + 67488a7
commit 54e2511
Show file tree

Hide file tree

Showing 10 changed files with 469 additions and 161 deletions.
diff --git a/CHANGELOG-3.4.md b/CHANGELOG-3.4.md
@@ -41,6 +41,7 @@ See [code changes](https://github.com/coreos/etcd/compare/v3.3.0...v3.4.0) and [
 - Add [`snapshot`](https://github.com/coreos/etcd/pull/9118) package for easier snapshot workflow (see [`godoc.org/github.com/etcd/clientv3/snapshot`](https://godoc.org/github.com/coreos/etcd/clientv3/snapshot) for more).
 - Improve [functional tester](https://github.com/coreos/etcd/tree/master/functional) coverage: [proxy layer to run network fault tests in CI](https://github.com/coreos/etcd/pull/9081), [TLS is enabled both for server and client](https://github.com/coreos/etcd/pull/9534), [liveness mode](https://github.com/coreos/etcd/issues/9230), [shuffle test sequence](https://github.com/coreos/etcd/issues/9381), [membership reconfiguration failure cases](https://github.com/coreos/etcd/pull/9564), [disastrous quorum loss and snapshot recover from a seed member](https://github.com/coreos/etcd/pull/9565), [embedded etcd](https://github.com/coreos/etcd/pull/9572).
 - Improve [index compaction blocking](https://github.com/coreos/etcd/pull/9511) by using a copy on write clone to avoid holding the lock for the traversal of the entire index.
+- Update [JWT methods](https://github.com/coreos/etcd/pull/9883) to allow for use of any supported signature method/algorithm.
 
 ### Breaking Changes
 

diff --git a/Documentation/op-guide/configuration.md b/Documentation/op-guide/configuration.md
@@ -372,6 +372,7 @@ Follow the instructions when using these flags.
 
 ### --auth-token
 + Specify a token type and token specific options, especially for JWT. Its format is "type,var1=val1,var2=val2,...". Possible type is 'simple' or 'jwt'. Possible variables are 'sign-method' for specifying a sign method of jwt (its possible values are 'ES256', 'ES384', 'ES512', 'HS256', 'HS384', 'HS512', 'RS256', 'RS384', 'RS512', 'PS256', 'PS384', or 'PS512'), 'pub-key' for specifying a path to a public key for verifying jwt, 'priv-key' for specifying a path to a private key for signing jwt, and 'ttl' for specifying TTL of jwt tokens.
++ For asymmetric algorithms ('RS', 'PS', 'ES'), the public key is optional, as the private key contains enough information to both sign and verify tokens.
 + Example option of JWT: '--auth-token jwt,pub-key=app.rsa.pub,priv-key=app.rsa,sign-method=RS512,ttl=10m'
 + default: "simple"
 

diff --git a/auth/jwt.go b/auth/jwt.go
@@ -16,8 +16,9 @@ package auth
 
 import (
 	"context"
+	"crypto/ecdsa"
 	"crypto/rsa"
-	"io/ioutil"
+	"errors"
 	"time"
 
 	jwt "github.com/dgrijalva/jwt-go"
@@ -26,10 +27,10 @@ import (
 
 type tokenJWT struct {
 	lg         *zap.Logger
-	signMethod string
-	signKey    *rsa.PrivateKey
-	verifyKey  *rsa.PublicKey
+	signMethod jwt.SigningMethod
+	key        interface{}
 	ttl        time.Duration
+	verifyOnly bool
 }
 
 func (t *tokenJWT) enable()                         {}
@@ -45,25 +46,20 @@ func (t *tokenJWT) info(ctx context.Context, token string, rev uint64) (*AuthInf
 	)
 
 	parsed, err := jwt.Parse(token, func(token *jwt.Token) (interface{}, error) {
-		return t.verifyKey, nil
-	})
-
-	switch err.(type) {
-	case nil:
-		if !parsed.Valid {
-			if t.lg != nil {
-				t.lg.Warn("invalid JWT token", zap.String("token", token))
-			} else {
-				plog.Warningf("invalid jwt token: %s", token)
-			}
-			return nil, false
+		if token.Method.Alg() != t.signMethod.Alg() {
+			return nil, errors.New("invalid signing method")
 		}
+		switch k := t.key.(type) {
+		case *rsa.PrivateKey:
+			return &k.PublicKey, nil
+		case *ecdsa.PrivateKey:
+			return &k.PublicKey, nil
+		default:
+			return t.key, nil
+		}
+	})
 
-		claims := parsed.Claims.(jwt.MapClaims)
-
-		username = claims["username"].(string)
-		revision = uint64(claims["revision"].(float64))
-	default:
+	if err != nil {
 		if t.lg != nil {
 			t.lg.Warn(
 				"failed to parse a JWT token",
@@ -76,20 +72,37 @@ func (t *tokenJWT) info(ctx context.Context, token string, rev uint64) (*AuthInf
 		return nil, false
 	}
 
+	claims, ok := parsed.Claims.(jwt.MapClaims)
+	if !parsed.Valid || !ok {
+		if t.lg != nil {
+			t.lg.Warn("invalid JWT token", zap.String("token", token))
+		} else {
+			plog.Warningf("invalid jwt token: %s", token)
+		}
+		return nil, false
+	}
+
+	username = claims["username"].(string)
+	revision = uint64(claims["revision"].(float64))
+
 	return &AuthInfo{Username: username, Revision: revision}, true
 }
 
 func (t *tokenJWT) assign(ctx context.Context, username string, revision uint64) (string, error) {
+	if t.verifyOnly {
+		return "", ErrVerifyOnly
+	}
+
 	// Future work: let a jwt token include permission information would be useful for
 	// permission checking in proxy side.
-	tk := jwt.NewWithClaims(jwt.GetSigningMethod(t.signMethod),
+	tk := jwt.NewWithClaims(t.signMethod,
 		jwt.MapClaims{
 			"username": username,
 			"revision": revision,
 			"exp":      time.Now().Add(t.ttl).Unix(),
 		})
 
-	token, err := tk.SignedString(t.signKey)
+	token, err := tk.SignedString(t.key)
 	if err != nil {
 		if t.lg != nil {
 			t.lg.Warn(
@@ -117,113 +130,54 @@ func (t *tokenJWT) assign(ctx context.Context, username string, revision uint64)
 	return token, err
 }
 
-func prepareOpts(lg *zap.Logger, opts map[string]string) (jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath string, ttl time.Duration, err error) {
-	for k, v := range opts {
-		switch k {
-		case "sign-method":
-			jwtSignMethod = v
-		case "pub-key":
-			jwtPubKeyPath = v
-		case "priv-key":
-			jwtPrivKeyPath = v
-		case "ttl":
-			ttl, err = time.ParseDuration(v)
-			if err != nil {
-				if lg != nil {
-					lg.Warn(
-						"failed to parse JWT TTL option",
-						zap.String("ttl-value", v),
-						zap.Error(err),
-					)
-				} else {
-					plog.Errorf("failed to parse ttl option (%s)", err)
-				}
-				return "", "", "", 0, ErrInvalidAuthOpts
-			}
-		default:
-			if lg != nil {
-				lg.Warn("unknown JWT token option", zap.String("option", k))
-			} else {
-				plog.Errorf("unknown token specific option: %s", k)
-			}
-			return "", "", "", 0, ErrInvalidAuthOpts
-		}
-	}
-	if len(jwtSignMethod) == 0 {
-		return "", "", "", 0, ErrInvalidAuthOpts
-	}
-	return jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath, ttl, nil
-}
-
-func newTokenProviderJWT(lg *zap.Logger, opts map[string]string) (*tokenJWT, error) {
-	jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath, ttl, err := prepareOpts(lg, opts)
+func newTokenProviderJWT(lg *zap.Logger, optMap map[string]string) (*tokenJWT, error) {
+	var err error
+	var opts jwtOptions
+	err = opts.ParseWithDefaults(optMap)
 	if err != nil {
+		if lg != nil {
+			lg.Warn("problem loading JWT options", zap.Error(err))
+		} else {
+			plog.Errorf("problem loading JWT options: %s", err)
+		}
 		return nil, ErrInvalidAuthOpts
 	}
 
-	if ttl == 0 {
-		ttl = 5 * time.Minute
-	}
-
-	t := &tokenJWT{
-		lg:  lg,
-		ttl: ttl,
-	}
-
-	t.signMethod = jwtSignMethod
-
-	verifyBytes, err := ioutil.ReadFile(jwtPubKeyPath)
-	if err != nil {
-		if lg != nil {
-			lg.Warn(
-				"failed to read JWT public key",
-				zap.String("public-key-path", jwtPubKeyPath),
-				zap.Error(err),
-			)
-		} else {
-			plog.Errorf("failed to read public key (%s) for jwt: %s", jwtPubKeyPath, err)
+	var keys = make([]string, 0, len(optMap))
+	for k := range optMap {
+		if !knownOptions[k] {
+			keys = append(keys, k)
 		}
-		return nil, err
 	}
-	t.verifyKey, err = jwt.ParseRSAPublicKeyFromPEM(verifyBytes)
-	if err != nil {
+	if len(keys) > 0 {
 		if lg != nil {
-			lg.Warn(
-				"failed to parse JWT public key",
-				zap.String("public-key-path", jwtPubKeyPath),
-				zap.Error(err),
-			)
+			lg.Warn("unknown JWT options", zap.Strings("keys", keys))
 		} else {
-			plog.Errorf("failed to parse public key (%s): %s", jwtPubKeyPath, err)
+			plog.Warningf("unknown JWT options: %v", keys)
 		}
-		return nil, err
 	}
 
-	signBytes, err := ioutil.ReadFile(jwtPrivKeyPath)
+	key, err := opts.Key()
 	if err != nil {
-		if lg != nil {
-			lg.Warn(
-				"failed to read JWT private key",
-				zap.String("private-key-path", jwtPrivKeyPath),
-				zap.Error(err),
-			)
-		} else {
-			plog.Errorf("failed to read private key (%s) for jwt: %s", jwtPrivKeyPath, err)
-		}
 		return nil, err
 	}
-	t.signKey, err = jwt.ParseRSAPrivateKeyFromPEM(signBytes)
-	if err != nil {
-		if lg != nil {
-			lg.Warn(
-				"failed to parse JWT private key",
-				zap.String("private-key-path", jwtPrivKeyPath),
-				zap.Error(err),
-			)
-		} else {
-			plog.Errorf("failed to parse private key (%s): %s", jwtPrivKeyPath, err)
+
+	t := &tokenJWT{
+		lg:         lg,
+		ttl:        opts.TTL,
+		signMethod: opts.SignMethod,
+		key:        key,
+	}
+
+	switch t.signMethod.(type) {
+	case *jwt.SigningMethodECDSA:
+		if _, ok := t.key.(*ecdsa.PublicKey); ok {
+			t.verifyOnly = true
+		}
+	case *jwt.SigningMethodRSA, *jwt.SigningMethodRSAPSS:
+		if _, ok := t.key.(*rsa.PublicKey); ok {
+			t.verifyOnly = true
 		}
-		return nil, err
 	}
 
 	return t, nil