Incorperate feedback

Signed-off-by: Kevin Fox <Kevin.Fox@pnnl.gov>

pkg/agent/plugin/nodeattestor/httpchallenge/httpchallenge_test.go

@@ -156,7 +156,7 @@ func TestAidAttestationSucceeds(t *testing.T) {
 			serverStream: func(attestationData *common_httpchallenge.AttestationData, challenge []byte, expectPayload []byte, challengeobj *common_httpchallenge.Challenge, port int) nodeattestor.ServerStream {
 				return streamBuilder.IgnoreThenChallenge(challenge).
 					Handle(func(challengeResponse []byte) ([]byte, error) {
-						err := common_httpchallenge.VerifyChallenge(http.DefaultClient, attestationData, challengeobj)
+						err := common_httpchallenge.VerifyChallenge(context.Background(), http.DefaultClient, attestationData, challengeobj)
 						return nil, err
 					}).Build()
 			},
@@ -172,7 +172,7 @@ func TestAidAttestationSucceeds(t *testing.T) {
 			serverStream: func(attestationData *common_httpchallenge.AttestationData, challenge []byte, expectPayload []byte, challengeobj *common_httpchallenge.Challenge, port int) nodeattestor.ServerStream {
 				return streamBuilder.ExpectThenChallenge(expectPayload, challenge).
 					Handle(func(challengeResponse []byte) ([]byte, error) {
-						err := common_httpchallenge.VerifyChallenge(http.DefaultClient, attestationData, challengeobj)
+						err := common_httpchallenge.VerifyChallenge(context.Background(), http.DefaultClient, attestationData, challengeobj)
 						return nil, err
 					}).Build()
 			},

pkg/common/plugin/httpchallenge/httpchallenge.go

@@ -11,7 +11,6 @@ import (
 	"net/url"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
 	"github.com/spiffe/spire/pkg/common/idutil"
@@ -53,7 +52,7 @@ func CalculateResponse(_ *Challenge) (*Response, error) {
 	return &Response{}, nil
 }
 
-func VerifyChallenge(client *http.Client, attestationData *AttestationData, challenge *Challenge) error {
+func VerifyChallenge(ctx context.Context, client *http.Client, attestationData *AttestationData, challenge *Challenge) error {
 	if attestationData.HostName == "" {
 		return fmt.Errorf("hostname must be set")
 	}
@@ -78,9 +77,6 @@ func VerifyChallenge(client *http.Client, attestationData *AttestationData, chal
 		Path:   fmt.Sprintf("/.well-known/spiffe/nodeattestor/http_challenge/%s/challenge", attestationData.AgentName),
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-
 	req, err := http.NewRequestWithContext(ctx, "GET", turl.String(), nil)
 	if err != nil {
 		return err

pkg/common/plugin/httpchallenge/httpchallenge_test.go

@@ -79,15 +79,16 @@ func TestValidateChallenge(t *testing.T) {
 			}))
 			defer func() { testServer.Close() }()
 
-			http.DefaultTransport.(*http.Transport).DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
+			transport := http.DefaultTransport.(*http.Transport).Clone()
+			transport.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
 				if addr == "foo.bar:80" {
 					addr = strings.TrimPrefix(testServer.URL, "http://")
 				}
 				dialer := &net.Dialer{}
 				return dialer.DialContext(ctx, network, addr)
 			}
 
-			err := VerifyChallenge(http.DefaultClient, ad, c)
+			err := VerifyChallenge(context.Background(), &http.Client{Transport: transport}, ad, c)
 			if tt.expectErr != "" {
 				require.Error(t, err)
 				require.Contains(t, err.Error(), tt.expectErr)

pkg/server/plugin/nodeattestor/httpchallenge/httpchallenge.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"regexp"
 	"sync"
+	"time"
 
 	"github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/hcl"
@@ -23,6 +24,10 @@ const (
 	pluginName = "http_challenge"
 )
 
+var (
+	agentNamePattern = regexp.MustCompile("^[a-zA-z]+[a-zA-Z0-9-]$")
+)
+
 func BuiltIn() catalog.BuiltIn {
 	return builtin(New())
 }
@@ -46,7 +51,6 @@ type configuration struct {
 	requiredPort      *int
 	allowNonRootPorts bool
 	dnsPatterns       []*regexp.Regexp
-	agentNamePattern  *regexp.Regexp
 	tofu              bool
 }
 
@@ -105,7 +109,7 @@ func (p *Plugin) Attest(stream nodeattestorv1.NodeAttestor_AttestServer) error {
 		return status.Errorf(codes.InvalidArgument, "port %d is not allowed to be >= 1024", attestationData.Port)
 	}
 
-	if err = validateAgentName(attestationData.AgentName, config.agentNamePattern); err != nil {
+	if err = validateAgentName(attestationData.AgentName); err != nil {
 		return err
 	}
 
@@ -138,7 +142,11 @@ func (p *Plugin) Attest(stream nodeattestorv1.NodeAttestor_AttestServer) error {
 	}
 
 	p.log.Debug("Verifying challenge")
-	if err := httpchallenge.VerifyChallenge(p.client, attestationData, challenge); err != nil {
+
+	timeoutctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	if err := httpchallenge.VerifyChallenge(timeoutctx, p.client, attestationData, challenge); err != nil {
 		return status.Errorf(codes.PermissionDenied, "challenge verification failed: %v", err)
 	}
 
@@ -189,8 +197,6 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*
 		dnsPatterns = append(dnsPatterns, re)
 	}
 
-	agentNamePattern := regexp.MustCompile("^[a-zA-z]+[a-zA-Z0-9-]$")
-
 	allowNonRootPorts := true
 	if hclConfig.AllowNonRootPorts != nil {
 		allowNonRootPorts = *hclConfig.AllowNonRootPorts
@@ -223,7 +229,6 @@ func (p *Plugin) Configure(_ context.Context, req *configv1.ConfigureRequest) (*
 		dnsPatterns:       dnsPatterns,
 		requiredPort:      hclConfig.RequiredPort,
 		allowNonRootPorts: allowNonRootPorts,
-		agentNamePattern:  agentNamePattern,
 		tofu:              tofu,
 	})
 
@@ -258,7 +263,7 @@ func buildSelectorValues(hostName string) []string {
 	return selectorValues
 }
 
-func validateAgentName(agentName string, agentNamePattern *regexp.Regexp) error {
+func validateAgentName(agentName string) error {
 	l := agentNamePattern.FindAllStringSubmatch(agentName, -1)
 	if len(l) != 1 || len(l[0]) == 0 || len(l[0]) > 32 {
 		return status.Error(codes.InvalidArgument, "agent name is not valid")