Disable /login path for roles with role_type oidc (#38)

Vault must not accept signed JWT tokens through /login path when role has role_type oidc, since there might be a situation when the client secret could be compromised, and thus the malicious might be able to illegitimately get a token with the right aud claim, which Vault would accept through the /login path.
hashicorp · Apr 4, 2019 · 7ca4cef · 7ca4cef
1 parent 7b19d4b
commit 7ca4cef
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 11 deletions.
diff --git a/path_login.go b/path_login.go
@@ -68,6 +68,10 @@ func (b *jwtAuthBackend) pathLogin(ctx context.Context, req *logical.Request, d
 		return logical.ErrorResponse("role %q could not be found", roleName), nil
 	}
 
+	if role.RoleType == "oidc" {
+		return logical.ErrorResponse("role with oidc role_type is not allowed"), nil
+	}
+
 	token := d.Get("jwt").(string)
 	if len(token) == 0 {
 		return logical.ErrorResponse("missing token"), nil
@@ -228,13 +232,9 @@ func (b *jwtAuthBackend) verifyOIDCToken(ctx context.Context, config *jwtConfig,
 
 	oidcConfig := &oidc.Config{
 		SupportedSigningAlgs: config.JWTSupportedAlgs,
+		SkipClientIDCheck:    true,
 	}
 
-	if role.RoleType == "oidc" {
-		oidcConfig.ClientID = config.OIDCClientID
-	} else {
-		oidcConfig.SkipClientIDCheck = true
-	}
 	verifier := provider.Verifier(oidcConfig)
 
 	idToken, err := verifier.Verify(ctx, rawToken)

diff --git a/path_login_test.go b/path_login_test.go
@@ -19,7 +19,7 @@ import (
 	"gopkg.in/square/go-jose.v2/jwt"
 )
 
-func setupBackend(t *testing.T, oidc, audience bool, boundClaims bool, boundCIDRs bool) (logical.Backend, logical.Storage) {
+func setupBackend(t *testing.T, oidc, role_type_oidc, audience bool, boundClaims bool, boundCIDRs bool) (logical.Backend, logical.Storage) {
 	b, storage := getBackend(t)
 
 	var data map[string]interface{}
@@ -62,6 +62,10 @@ func setupBackend(t *testing.T, oidc, audience bool, boundClaims bool, boundCIDR
 			"/org/primary": "primary_org",
 		},
 	}
+	if role_type_oidc {
+		data["role_type"] = "oidc"
+		data["allowed_redirect_uris"] = "http://127.0.0.1"
+	}
 	if audience {
 		data["bound_audiences"] = []string{"https://vault.plugin.auth.jwt.test", "another_audience"}
 	}
@@ -147,9 +151,56 @@ func getTestOIDC(t *testing.T) string {
 }
 
 func TestLogin_JWT(t *testing.T) {
+	// Test role_type oidc
+	{
+		b, storage := setupBackend(t, false, true, true, false, false)
+		cl := jwt.Claims{
+			Subject:   "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients",
+			Issuer:    "https://team-vault.auth0.com/",
+			NotBefore: jwt.NewNumericDate(time.Now().Add(-5 * time.Second)),
+			Audience:  jwt.Audience{"https://vault.plugin.auth.jwt.test"},
+		}
+
+		privateCl := struct {
+			User   string   `json:"https://vault/user"`
+			Groups []string `json:"https://vault/groups"`
+		}{
+			"jeff",
+			[]string{"foo", "bar"},
+		}
+
+		jwtData, _ := getTestJWT(t, ecdsaPrivKey, cl, privateCl)
+
+		data := map[string]interface{}{
+			"role": "plugin-test",
+			"jwt":  jwtData,
+		}
+
+		req := &logical.Request{
+			Operation: logical.UpdateOperation,
+			Path:      "login",
+			Storage:   storage,
+			Data:      data,
+		}
+
+		resp, err := b.HandleRequest(context.Background(), req)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if resp == nil {
+			t.Fatal("got nil response")
+		}
+		if !resp.IsError() {
+			t.Fatal("expected error")
+		}
+		if !strings.Contains(resp.Error().Error(), "role with oidc role_type is not allowed") {
+			t.Fatalf("unexpected error: %v", resp.Error())
+		}
+	}
+
 	// Test missing audience
 	{
-		b, storage := setupBackend(t, false, false, false, false)
+		b, storage := setupBackend(t, false, false, false, false, false)
 		cl := jwt.Claims{
 			Subject:   "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients",
 			Issuer:    "https://team-vault.auth0.com/",
@@ -198,7 +249,7 @@ func TestLogin_JWT(t *testing.T) {
 	{
 		// run test with and without bound_cidrs configured
 		for _, useBoundCIDRs := range []bool{false, true} {
-			b, storage := setupBackend(t, false, true, true, useBoundCIDRs)
+			b, storage := setupBackend(t, false, false, true, true, useBoundCIDRs)
 
 			cl := jwt.Claims{
 				Subject:   "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients",
@@ -287,7 +338,7 @@ func TestLogin_JWT(t *testing.T) {
 		}
 	}
 
-	b, storage := setupBackend(t, false, true, true, false)
+	b, storage := setupBackend(t, false, false, true, true, false)
 
 	// test invalid bound claim
 	{
@@ -678,7 +729,7 @@ func TestLogin_JWT(t *testing.T) {
 
 	// test invalid address
 	{
-		b, storage := setupBackend(t, false, false, false, true)
+		b, storage := setupBackend(t, false, false, false, false, true)
 
 		cl := jwt.Claims{
 			Subject:   "r3qXcK2bix9eFECzsU3Sbmh0K16fatW6@clients",
@@ -754,7 +805,7 @@ func TestLogin_JWT(t *testing.T) {
 }
 
 func TestLogin_OIDC(t *testing.T) {
-	b, storage := setupBackend(t, true, true, false, false)
+	b, storage := setupBackend(t, true, false, true, false, false)
 
 	jwtData := getTestOIDC(t)