VAULT-6727 Adjust cert and approle role resolution, add more tests (#16341)

* VAULT-6727 Adjust cert and approle role resolution, add more tests * VAULT-6727 Add new test
2025-10-29 17:52:32 +00:00 · 2022-07-20 09:24:06 -04:00
parent 3ca6036a4a
commit a5f6b1cc70
4 changed files with 239 additions and 26 deletions
--- a/builtin/credential/approle/path_login.go
+++ b/builtin/credential/approle/path_login.go
@@ -75,6 +75,18 @@ func (b *backend) pathLoginResolveRole(ctx context.Context, req *logical.Request

 	roleName := roleIDIndex.Name

+	roleLock := b.roleLock(roleName)
+	roleLock.RLock()
+
+	role, err := b.roleEntry(ctx, req.Storage, roleName)
+	roleLock.RUnlock()
+	if err != nil {
+		return nil, err
+	}
+	if role == nil {
+		return logical.ErrorResponse("invalid role ID"), nil
+	}
+
 	return logical.ResolveRoleResponse(roleName)
 }

--- a/builtin/credential/approle/path_login_test.go
+++ b/builtin/credential/approle/path_login_test.go
@@ -354,3 +354,39 @@ func TestAppRole_RoleResolve(t *testing.T) {
 		t.Fatalf("Role was not as expected. Expected %s, received %s", role, resp.Data["role"])
 	}
 }
+
+func TestAppRole_RoleDoesNotExist(t *testing.T) {
+	var resp *logical.Response
+	var err error
+	b, storage := createBackendWithStorage(t)
+
+	roleID := "roleDoesNotExist"
+
+	loginData := map[string]interface{}{
+		"role_id":   roleID,
+		"secret_id": "secret",
+	}
+	loginReq := &logical.Request{
+		Operation: logical.ResolveRoleOperation,
+		Path:      "login",
+		Storage:   storage,
+		Data:      loginData,
+		Connection: &logical.Connection{
+			RemoteAddr: "127.0.0.1",
+		},
+	}
+
+	resp, err = b.HandleRequest(context.Background(), loginReq)
+	if resp == nil && !resp.IsError() {
+		t.Fatalf("Response was not an error: err:%v resp:%#v", err, resp)
+	}
+
+	errString, ok := resp.Data["error"].(string)
+	if !ok {
+		t.Fatal("Error not part of response.")
+	}
+
+	if !strings.Contains(errString, "invalid role ID") {
+		t.Fatalf("Error was not due to invalid role ID. Error: %s", errString)
+	}
+}
--- a/builtin/credential/cert/path_login.go
+++ b/builtin/credential/cert/path_login.go
@@ -45,11 +45,20 @@ func pathLogin(b *backend) *framework.Path {
 }

 func (b *backend) pathLoginResolveRole(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) {
-	certRole := data.Get("name").(string)
-	if certRole == "" {
-		return logical.ErrorResponse("Role not present in this request"), nil
+	var matched *ParsedCert
+	if verifyResp, resp, err := b.verifyCredentials(ctx, req, data); err != nil {
+		return nil, err
+	} else if resp != nil {
+		return resp, nil
+	} else {
+		matched = verifyResp
 	}
-	return logical.ResolveRoleResponse(certRole)
+
+	if matched == nil {
+		return logical.ErrorResponse("no certificate was matched by this request"), nil
+	}
+
+	return logical.ResolveRoleResponse(matched.Entry.Name)
 }

 func (b *backend) pathLoginAliasLookahead(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
--- a/builtin/credential/cert/path_login_test.go
+++ b/builtin/credential/cert/path_login_test.go
@@ -1,43 +1,199 @@
 package cert

 import (
-	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"io/ioutil"
+	"math/big"
+	mathrand "math/rand"
+	"net"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
+	"time"
+
+	logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"

 	"github.com/hashicorp/vault/sdk/logical"
 )

 func TestCert_RoleResolve(t *testing.T) {
-	config := logical.TestBackendConfig()
-	storage := &logical.InmemStorage{}
-	config.StorageView = storage
+	certTemplate := &x509.Certificate{
+		Subject: pkix.Name{
+			CommonName: "example.com",
+		},
+		DNSNames:    []string{"example.com"},
+		IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
+		ExtKeyUsage: []x509.ExtKeyUsage{
+			x509.ExtKeyUsageServerAuth,
+			x509.ExtKeyUsageClientAuth,
+		},
+		KeyUsage:     x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageKeyAgreement,
+		SerialNumber: big.NewInt(mathrand.Int63()),
+		NotBefore:    time.Now().Add(-30 * time.Second),
+		NotAfter:     time.Now().Add(262980 * time.Hour),
+	}

-	b, err := Factory(context.Background(), config)
+	tempDir, connState, err := generateTestCertAndConnState(t, certTemplate)
+	if tempDir != "" {
+		defer os.RemoveAll(tempDir)
+	}
 	if err != nil {
-		t.Fatal(err)
+		t.Fatalf("error testing connection state: %v", err)
+	}
+	ca, err := ioutil.ReadFile(filepath.Join(tempDir, "ca_cert.pem"))
+	if err != nil {
+		t.Fatalf("err: %v", err)
 	}

-	roleName := "roleName"
+	logicaltest.Test(t, logicaltest.TestCase{
+		CredentialBackend: testFactory(t),
+		Steps: []logicaltest.TestStep{
+			testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false),
+			testAccStepLoginWithName(t, connState, "web"),
+			testAccStepResolveRoleWithName(t, connState, "web"),
+		},
+	})
+}

-	loginData := map[string]interface{}{
-		"name": roleName,
-	}
-	loginReq := &logical.Request{
-		Operation: logical.ResolveRoleOperation,
-		Path:      "login",
-		Storage:   storage,
-		Data:      loginData,
-		Connection: &logical.Connection{
-			RemoteAddr: "127.0.0.1",
+func testAccStepResolveRoleWithName(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation:       logical.ResolveRoleOperation,
+		Path:            "login",
+		Unauthenticated: true,
+		ConnState:       &connState,
+		Check: func(resp *logical.Response) error {
+			if resp.Data["role"] != certName {
+				t.Fatalf("Role was not as expected. Expected %s, received %s", certName, resp.Data["role"])
+			}
+			return nil
+		},
+		Data: map[string]interface{}{
+			"name": certName,
 		},
 	}
+}

-	resp, err := b.HandleRequest(context.Background(), loginReq)
-	if err != nil || (resp != nil && resp.IsError()) {
-		t.Fatalf("err:%v resp:%#v", err, resp)
+func TestCert_RoleResolveWithoutProvidingCertName(t *testing.T) {
+	certTemplate := &x509.Certificate{
+		Subject: pkix.Name{
+			CommonName: "example.com",
+		},
+		DNSNames:    []string{"example.com"},
+		IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
+		ExtKeyUsage: []x509.ExtKeyUsage{
+			x509.ExtKeyUsageServerAuth,
+			x509.ExtKeyUsageClientAuth,
+		},
+		KeyUsage:     x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageKeyAgreement,
+		SerialNumber: big.NewInt(mathrand.Int63()),
+		NotBefore:    time.Now().Add(-30 * time.Second),
+		NotAfter:     time.Now().Add(262980 * time.Hour),
 	}

-	if resp.Data["role"] != roleName {
-		t.Fatalf("Role was not as expected. Expected %s, received %s", roleName, resp.Data["role"])
+	tempDir, connState, err := generateTestCertAndConnState(t, certTemplate)
+	if tempDir != "" {
+		defer os.RemoveAll(tempDir)
+	}
+	if err != nil {
+		t.Fatalf("error testing connection state: %v", err)
+	}
+	ca, err := ioutil.ReadFile(filepath.Join(tempDir, "ca_cert.pem"))
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	logicaltest.Test(t, logicaltest.TestCase{
+		CredentialBackend: testFactory(t),
+		Steps: []logicaltest.TestStep{
+			testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false),
+			testAccStepLoginWithName(t, connState, "web"),
+			testAccStepResolveRoleWithEmptyDataMap(t, connState, "web"),
+		},
+	})
+}
+
+func testAccStepResolveRoleWithEmptyDataMap(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation:       logical.ResolveRoleOperation,
+		Path:            "login",
+		Unauthenticated: true,
+		ConnState:       &connState,
+		Check: func(resp *logical.Response) error {
+			if resp.Data["role"] != certName {
+				t.Fatalf("Role was not as expected. Expected %s, received %s", certName, resp.Data["role"])
+			}
+			return nil
+		},
+		Data: map[string]interface{}{},
 	}
 }
+
+func testAccStepResolveRoleExpectRoleResolutionToFail(t *testing.T, connState tls.ConnectionState, certName string) logicaltest.TestStep {
+	return logicaltest.TestStep{
+		Operation:       logical.ResolveRoleOperation,
+		Path:            "login",
+		Unauthenticated: true,
+		ConnState:       &connState,
+		ErrorOk:         true,
+		Check: func(resp *logical.Response) error {
+			if resp == nil && !resp.IsError() {
+				t.Fatalf("Response was not an error: resp:%#v", resp)
+			}
+
+			errString, ok := resp.Data["error"].(string)
+			if !ok {
+				t.Fatal("Error not part of response.")
+			}
+
+			if !strings.Contains(errString, "invalid certificate") {
+				t.Fatalf("Error was not due to invalid role name. Error: %s", errString)
+			}
+			return nil
+		},
+		Data: map[string]interface{}{
+			"name": certName,
+		},
+	}
+}
+
+func TestCert_RoleResolve_RoleDoesNotExist(t *testing.T) {
+	certTemplate := &x509.Certificate{
+		Subject: pkix.Name{
+			CommonName: "example.com",
+		},
+		DNSNames:    []string{"example.com"},
+		IPAddresses: []net.IP{net.ParseIP("127.0.0.1")},
+		ExtKeyUsage: []x509.ExtKeyUsage{
+			x509.ExtKeyUsageServerAuth,
+			x509.ExtKeyUsageClientAuth,
+		},
+		KeyUsage:     x509.KeyUsageDigitalSignature | x509.KeyUsageKeyEncipherment | x509.KeyUsageKeyAgreement,
+		SerialNumber: big.NewInt(mathrand.Int63()),
+		NotBefore:    time.Now().Add(-30 * time.Second),
+		NotAfter:     time.Now().Add(262980 * time.Hour),
+	}
+
+	tempDir, connState, err := generateTestCertAndConnState(t, certTemplate)
+	if tempDir != "" {
+		defer os.RemoveAll(tempDir)
+	}
+	if err != nil {
+		t.Fatalf("error testing connection state: %v", err)
+	}
+	ca, err := ioutil.ReadFile(filepath.Join(tempDir, "ca_cert.pem"))
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+
+	logicaltest.Test(t, logicaltest.TestCase{
+		CredentialBackend: testFactory(t),
+		Steps: []logicaltest.TestStep{
+			testAccStepCert(t, "web", ca, "foo", allowed{dns: "example.com"}, false),
+			testAccStepLoginWithName(t, connState, "web"),
+			testAccStepResolveRoleExpectRoleResolutionToFail(t, connState, "notweb"),
+		},
+	})
+}