In ACME responses only return Type, Value fields (#20480)

- Do not serialize the entire internal object, instead return
   just the Type and Value fields back to the caller.
 - Also within authorization responses, return the base domain
   on wildcard queries, dropping the *. as the RFC requests.
 - Update tests to reflect/test this logic.

builtin/logical/pki/acme_authorizations.go

@@ -56,6 +56,17 @@ func (ai *ACMEIdentifier) MaybeParseWildcard() (bool, string, error) {
 	return true, reducedName, nil
 }
 
+func (ai *ACMEIdentifier) NetworkMarshal(useOriginalValue bool) map[string]interface{} {
+	value := ai.OriginalValue
+	if !useOriginalValue {
+		value = ai.Value
+	}
+	return map[string]interface{}{
+		"type":  ai.Type,
+		"value": value,
+	}
+}
+
 type ACMEAuthorizationStatusType string
 
 const (
@@ -155,7 +166,7 @@ func (aa *ACMEAuthorization) GetExpires() (time.Time, error) {
 
 func (aa *ACMEAuthorization) NetworkMarshal(acmeCtx *acmeContext) map[string]interface{} {
 	resp := map[string]interface{}{
-		"identifier": aa.Identifier,
+		"identifier": aa.Identifier.NetworkMarshal( /* use value, not original value */ false),
 		"status":     aa.Status,
 		"wildcard":   aa.Wildcard,
 	}

builtin/logical/pki/path_acme_order.go

@@ -685,11 +685,16 @@ func formatOrderResponse(acmeCtx *acmeContext, order *acmeOrder) *logical.Respon
 		authorizationUrls = append(authorizationUrls, buildAuthorizationUrl(acmeCtx, authId))
 	}
 
+	var identifiers []map[string]interface{}
+	for _, identifier := range order.Identifiers {
+		identifiers = append(identifiers, identifier.NetworkMarshal( /* use original value */ true))
+	}
+
 	resp := &logical.Response{
 		Data: map[string]interface{}{
 			"status":         order.Status,
 			"expires":        order.Expires.Format(time.RFC3339),
-			"identifiers":    order.Identifiers,
+			"identifiers":    identifiers,
 			"authorizations": authorizationUrls,
 			"finalize":       baseOrderUrl + "/finalize",
 		},

builtin/logical/pki/path_acme_test.go

@@ -126,7 +126,7 @@ func TestAcmeBasicWorkflow(t *testing.T) {
 
 			// Create an order
 			t.Logf("Testing Authorize Order on %s", baseAcmeURL)
-			identifiers := []string{"localhost", "*.localhost"}
+			identifiers := []string{"localhost.localdomain", "*.localdomain"}
 			createOrder, err := acmeClient.AuthorizeOrder(testCtx, []acme.AuthzID{
 				{Type: "dns", Value: identifiers[0]},
 				{Type: "dns", Value: identifiers[1]},
@@ -146,27 +146,68 @@ func TestAcmeBasicWorkflow(t *testing.T) {
 				t.Fatalf("Differences exist between create and get order: \n%v", strings.Join(diffs, "\n"))
 			}
 
-			// Load authorization
+			// Make sure the identifiers returned in the order contain the original values
-			auth, err := acmeClient.GetAuthorization(testCtx, getOrder.AuthzURLs[0])
+			var ids []string
-			require.NoError(t, err, "failed fetching authorization")
+			for _, id := range getOrder.Identifiers {
-			require.Equal(t, acme.StatusPending, auth.Status)
+				require.Equal(t, "dns", id.Type)
-			require.Equal(t, "dns", auth.Identifier.Type)
+				ids = append(ids, id.Value)
-			require.Equal(t, "localhost", auth.Identifier.Value)
+			}
-			require.False(t, auth.Wildcard, "should not be a wildcard")
+			require.ElementsMatch(t, identifiers, ids, "order responses should have all original identifiers")
-			require.True(t, auth.Expires.IsZero(), "authorization should only have expiry set on valid status")
 
-			require.Len(t, auth.Challenges, 2, "expected two challenges")
+			// Load authorizations
-			require.Equal(t, acme.StatusPending, auth.Challenges[0].Status)
+			var authorizations []*acme.Authorization
-			require.True(t, auth.Challenges[0].Validated.IsZero(), "validated time should be 0 on challenge")
+			for _, authUrl := range getOrder.AuthzURLs {
-			require.Equal(t, "http-01", auth.Challenges[0].Type)
+				auth, err := acmeClient.GetAuthorization(testCtx, authUrl)
-			require.NotEmpty(t, auth.Challenges[0].Token, "missing challenge token")
+				require.NoError(t, err, "failed fetching authorization: %s", authUrl)
-			require.Equal(t, acme.StatusPending, auth.Challenges[1].Status)
+
-			require.True(t, auth.Challenges[1].Validated.IsZero(), "validated time should be 0 on challenge")
+				authorizations = append(authorizations, auth)
-			require.Equal(t, "dns-01", auth.Challenges[1].Type)
+			}
-			require.NotEmpty(t, auth.Challenges[1].Token, "missing challenge token")
+
+			// We should have 2 separate auth challenges as we have two separate identifier
+			require.Len(t, authorizations, 2, "expected 2 authorizations in order")
+
+			var wildcardAuth *acme.Authorization
+			var domainAuth *acme.Authorization
+			for _, auth := range authorizations {
+				if auth.Wildcard {
+					wildcardAuth = auth
+				} else {
+					domainAuth = auth
+				}
+			}
+
+			// Test the values for the domain authentication
+			require.Equal(t, acme.StatusPending, domainAuth.Status)
+			require.Equal(t, "dns", domainAuth.Identifier.Type)
+			require.Equal(t, "localhost.localdomain", domainAuth.Identifier.Value)
+			require.False(t, domainAuth.Wildcard, "should not be a wildcard")
+			require.True(t, domainAuth.Expires.IsZero(), "authorization should only have expiry set on valid status")
+
+			require.Len(t, domainAuth.Challenges, 2, "expected two challenges")
+			require.Equal(t, acme.StatusPending, domainAuth.Challenges[0].Status)
+			require.True(t, domainAuth.Challenges[0].Validated.IsZero(), "validated time should be 0 on challenge")
+			require.Equal(t, "http-01", domainAuth.Challenges[0].Type)
+			require.NotEmpty(t, domainAuth.Challenges[0].Token, "missing challenge token")
+			require.Equal(t, acme.StatusPending, domainAuth.Challenges[1].Status)
+			require.True(t, domainAuth.Challenges[1].Validated.IsZero(), "validated time should be 0 on challenge")
+			require.Equal(t, "dns-01", domainAuth.Challenges[1].Type)
+			require.NotEmpty(t, domainAuth.Challenges[1].Token, "missing challenge token")
+
+			// Test the values for the wilcard authentication
+			require.Equal(t, acme.StatusPending, wildcardAuth.Status)
+			require.Equal(t, "dns", wildcardAuth.Identifier.Type)
+			require.Equal(t, "localdomain", wildcardAuth.Identifier.Value) // Make sure we strip the *. in auth responses
+			require.True(t, wildcardAuth.Wildcard, "should be a wildcard")
+			require.True(t, wildcardAuth.Expires.IsZero(), "authorization should only have expiry set on valid status")
+
+			require.Len(t, wildcardAuth.Challenges, 1, "expected two challenges")
+			require.Equal(t, acme.StatusPending, domainAuth.Challenges[0].Status)
+			require.True(t, wildcardAuth.Challenges[0].Validated.IsZero(), "validated time should be 0 on challenge")
+			require.Equal(t, "dns-01", wildcardAuth.Challenges[0].Type)
+			require.NotEmpty(t, domainAuth.Challenges[0].Token, "missing challenge token")
 
 			// Load a challenge directly; this triggers validation to start.
-			challenge, err := acmeClient.GetChallenge(testCtx, auth.Challenges[0].URI)
+			challenge, err := acmeClient.GetChallenge(testCtx, domainAuth.Challenges[0].URI)
 			require.NoError(t, err, "failed to load challenge")
 			require.Equal(t, acme.StatusProcessing, challenge.Status)
 			require.True(t, challenge.Validated.IsZero(), "validated time should be 0 on challenge")
@@ -496,9 +537,12 @@ func setupAcmeBackend(t *testing.T) (*vault.TestCluster, *api.Client, string) {
 	require.NoError(t, err, "failed updating default issuer")
 
 	_, err = client.Logical().Write("/pki/roles/test-role", map[string]interface{}{
-		"ttl_duration":     "365h",
+		"ttl_duration":                "365h",
-		"max_ttl_duration": "720h",
+		"max_ttl_duration":            "720h",
-		"key_type":         "any",
+		"key_type":                    "any",
+		"allowed_domains":             "localdomain",
+		"allow_subdomains":            "true",
+		"allow_wildcard_certificates": "true",
 	})
 	require.NoError(t, err, "failed creating role test-role")