Rework certificate authentication client api (#29931)

* Rework certificate authentication api

 - Use the passed in Vault api client to perform the connection
    - This provides namespace support, retry behaviors and uses
      the existing secret parsing logic instead of re-implementing it
 - Change the cert auth role to be an optional argument
 - Allow users to use a different cert auth mount point

* Clean up test name

Co-authored-by: Amir Aslamov <[email protected]>

---------

Co-authored-by: Amir Aslamov <[email protected]>

Author: stevendpclark

CODEOWNERS

@@ -67,6 +67,7 @@
 /enos/     @hashicorp/quality-team
 
 # Cryptosec
+/api/auth/cert/                                      @hashicorp/vault-crypto
 /builtin/logical/pki/                                @hashicorp/vault-crypto
 /builtin/logical/pkiext/                             @hashicorp/vault-crypto
 /website/content/docs/secrets/pki/                   @hashicorp/vault-crypto @hashicorp/vault-education-approvers

api/auth/cert/cert.go

@@ -4,49 +4,32 @@
 package cert
 
 import (
-	"bytes"
 	"context"
-	"crypto/tls"
-	"encoding/json"
+	"errors"
 	"fmt"
-	"io"
-	"net/http"
 
-	"github.com/hashicorp/go-rootcerts"
 	"github.com/hashicorp/vault/api"
 )
 
+const defaultMountPath = "cert"
+
 type CertAuth struct {
-	role               string
-	caCert             string
-	caCertBytes        []byte
-	clientCert         string
-	clientKey          string
-	insecureSkipVerify bool
+	role      string
+	mountPath string
 }
 
 var _ api.AuthMethod = (*CertAuth)(nil)
 
 type LoginOption func(a *CertAuth) error
 
 // NewCertAuth initializes a new Cert auth method interface to be
-// passed as a parameter to the client.Auth().Login method.
+// passed as a parameter to the client.Auth().Login method. The client and other
+// TLS configuration should be set up on the passed in client, you can use
+// the NewCertAuthClient function to set up the client with the proper TLS client attributes.
 //
-// Supported options: WithCACert, WithCACertBytes, WithInsecure
-func NewCertAuth(roleName, clientCert, clientKey string, opts ...LoginOption) (*CertAuth, error) {
-	if roleName == "" {
-		return nil, fmt.Errorf("no role name provided for login")
-	}
-
-	if clientCert == "" || clientKey == "" {
-		return nil, fmt.Errorf("client certificate and key must be provided")
-	}
-
-	a := &CertAuth{
-		role:       roleName,
-		clientCert: clientCert,
-		clientKey:  clientKey,
-	}
+// Supported options: WithRole, WithMountPath
+func NewCertAuth(opts ...LoginOption) (*CertAuth, error) {
+	a := &CertAuth{}
 
 	// Loop through each option
 	for _, opt := range opts {
@@ -58,111 +41,112 @@ func NewCertAuth(roleName, clientCert, clientKey string, opts ...LoginOption) (*
 		}
 	}
 
+	if a.mountPath == "" {
+		a.mountPath = defaultMountPath
+	}
+
 	// return the modified auth struct instance
 	return a, nil
 }
 
 // Login sets up the required request body for the Cert auth method's /login
-// endpoint, and performs a write to it.
-// It adds the client cert and key to the request.
+// endpoint, and performs a write to it. We assume the passed in client has the
+// proper TLS client certificates set up.
 func (a *CertAuth) Login(ctx context.Context, client *api.Client) (*api.Secret, error) {
 	if ctx == nil {
 		ctx = context.Background()
 	}
 
-	c, err := a.httpClient()
-	if err != nil {
-		return nil, err
+	if client == nil {
+		return nil, fmt.Errorf("client is required for login with the associated client certs initialized")
 	}
 
-	reqBody, err := json.Marshal(map[string]interface{}{
-		"name": a.role,
-	})
-	if err != nil {
-		return nil, fmt.Errorf("unable to marshal login data: %w", err)
+	loginData := make(map[string]interface{})
+	if a.role != "" {
+		loginData["name"] = a.role
 	}
 
-	url := fmt.Sprintf("%s/v1/auth/cert/login", client.Address())
-	req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(reqBody))
-	if err != nil {
-		return nil, fmt.Errorf("unable to create request: %w", err)
-	}
-
-	resp, err := c.Do(req)
+	certAuthPath := fmt.Sprintf("auth/%s/login", a.mountPath)
+	resp, err := client.Logical().WriteWithContext(ctx, certAuthPath, loginData)
 	if err != nil {
 		return nil, fmt.Errorf("unable to log in with cert auth: %w", err)
 	}
 
-	defer resp.Body.Close()
+	return resp, nil
+}
 
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, fmt.Errorf("unable to read response body: %w", err)
+// WithRole specifies the role to use for the login request.
+func WithRole(roleName string) LoginOption {
+	return func(a *CertAuth) error {
+		a.role = roleName
+		return nil
 	}
+}
 
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("unable to log in with cert auth, response code: %d. response body: %s", resp.StatusCode, string(body))
+// WithMountPath specifies the mount path to use for the login request.
+func WithMountPath(mountPath string) LoginOption {
+	return func(a *CertAuth) error {
+		a.mountPath = mountPath
+		return nil
 	}
+}
 
-	var secret api.Secret
-	if err := json.Unmarshal(body, &secret); err != nil {
-		return nil, fmt.Errorf("unable to unmarshal response body: %w", err)
+// NewDefaultCertAuthClient initializes a new client with a default configuration
+// with the provided address and TLS configuration. The TLSConfig must have the ClientCert and ClientKey
+// fields set.
+func NewDefaultCertAuthClient(address string, tlsConfig *api.TLSConfig) (*api.Client, error) {
+	if tlsConfig == nil {
+		return nil, errors.New("tls config is required for cert auth client")
 	}
 
-	return &secret, nil
-}
+	if tlsConfig.ClientCert == "" || tlsConfig.ClientKey == "" {
+		return nil, errors.New("client cert and key are required for cert auth client")
+	}
 
-func (a *CertAuth) httpClient() (*http.Client, error) {
-	cert, err := tls.LoadX509KeyPair(a.clientCert, a.clientKey)
-	if err != nil {
-		return nil, fmt.Errorf("unable to load cert: %w", err)
+	if len(address) == 0 {
+		return nil, errors.New("address is required for cert auth client")
 	}
 
-	tlsConfig := &tls.Config{
-		InsecureSkipVerify: a.insecureSkipVerify,
-		Certificates:       []tls.Certificate{cert},
+	cfg := api.DefaultConfig()
+	cfg.Address = address
+	err := cfg.ConfigureTLS(tlsConfig)
+	if err != nil {
+		return nil, fmt.Errorf("failed configuring TLS on client config: %w", err)
 	}
 
-	if a.caCert != "" || len(a.caCertBytes) > 0 {
-		err = rootcerts.ConfigureTLS(tlsConfig, &rootcerts.Config{
-			CAPath:        a.caCert,
-			CACertificate: a.caCertBytes,
-		})
-		if err != nil {
-			return nil, fmt.Errorf("unable to configure TLS: %w", err)
-		}
+	client, err := api.NewClient(cfg)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create client: %v", err)
 	}
 
-	return &http.Client{
-		Transport: &http.Transport{
-			TLSClientConfig: tlsConfig,
-		},
-	}, nil
+	return client, nil
 }
 
-// WithCACert sets the CA cert to be used for the login request.
-// caCert is the path to the CA cert file.
-func WithCACert(caCert string) LoginOption {
-	return func(a *CertAuth) error {
-		a.caCert = caCert
-		return nil
+// NewCertAuthClient initializes a new client based on the passed in client
+// with the provided TLS configuration. The TLSConfig must have the ClientCert and ClientKey
+// fields set.
+func NewCertAuthClient(c *api.Client, config *api.TLSConfig) (*api.Client, error) {
+	if c == nil {
+		return nil, errors.New("base client is required for cert auth client")
+	}
+	if config == nil {
+		return nil, errors.New("tls config is required for cert auth client")
 	}
-}
 
-// WithCACertBytes sets the CA cert to be used for the login request.
-// caCertBytes is the bytes of the CA cert.
-// caCertBytes takes precedence over caCert.
-func WithCACertBytes(caCertBytes []byte) LoginOption {
-	return func(a *CertAuth) error {
-		a.caCertBytes = caCertBytes
-		return nil
+	if config.ClientCert == "" || config.ClientKey == "" {
+		return nil, errors.New("client cert and key are required for cert auth client")
 	}
-}
 
-// WithInsecure skips the verification of the server's certificate chain and host name.
-func WithInsecure() LoginOption {
-	return func(a *CertAuth) error {
-		a.insecureSkipVerify = true
-		return nil
+	conf := c.CloneConfig()
+	err := conf.ConfigureTLS(config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to configure TLS on client: %w", err)
 	}
+
+	client, err := api.NewClient(conf)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create client: %v", err)
+	}
+
+	return client, nil
 }