Merge pull request #1136 from smallstep/herman/ignore-empty-acme-meta

2 years ago · c169defc73
parent 80cbcb652b 920c4f02c5
commit c169defc73
6 changed files with 176 additions and 8 deletions
--- a/acme/api/handler.go
+++ b/acme/api/handler.go
@ -205,7 +205,7 @@ type Directory struct {
 	NewOrder   string `json:"newOrder"`
 	RevokeCert string `json:"revokeCert"`
 	KeyChange  string `json:"keyChange"`
-	Meta       Meta   `json:"meta"`
+	Meta       *Meta  `json:"meta,omitempty"`
 }

 // ToLog enables response logging for the Directory type.
@ -228,18 +228,49 @@ func GetDirectory(w http.ResponseWriter, r *http.Request) {
 	}

 	linker := acme.MustLinkerFromContext(ctx)
+
 	render.JSON(w, &Directory{
 		NewNonce:   linker.GetLink(ctx, acme.NewNonceLinkType),
 		NewAccount: linker.GetLink(ctx, acme.NewAccountLinkType),
 		NewOrder:   linker.GetLink(ctx, acme.NewOrderLinkType),
 		RevokeCert: linker.GetLink(ctx, acme.RevokeCertLinkType),
 		KeyChange:  linker.GetLink(ctx, acme.KeyChangeLinkType),
-		Meta: Meta{
-			ExternalAccountRequired: acmeProv.RequireEAB,
-		},
+		Meta:       createMetaObject(acmeProv),
 	})
 }

+// createMetaObject creates a Meta object if the ACME provisioner
+// has one or more properties that are written in the ACME directory output.
+// It returns nil if none of the properties are set.
+func createMetaObject(p *provisioner.ACME) *Meta {
+	if shouldAddMetaObject(p) {
+		return &Meta{
+			TermsOfService:          p.TermsOfService,
+			Website:                 p.Website,
+			CaaIdentities:           p.CaaIdentities,
+			ExternalAccountRequired: p.RequireEAB,
+		}
+	}
+	return nil
+}
+
+// shouldAddMetaObject returns whether or not the ACME provisioner
+// has properties configured that must be added to the ACME directory object.
+func shouldAddMetaObject(p *provisioner.ACME) bool {
+	switch {
+	case p.TermsOfService != "":
+		return true
+	case p.Website != "":
+		return true
+	case len(p.CaaIdentities) > 0:
+		return true
+	case p.RequireEAB:
+		return true
+	default:
+		return false
+	}
+}
+
 // NotImplemented returns a 501 and is generally a placeholder for functionality which
 // MAY be added at some point in the future but is not in any way a guarantee of such.
 func NotImplemented(w http.ResponseWriter, r *http.Request) {
--- a/acme/api/handler_test.go
+++ b/acme/api/handler_test.go
@ -18,10 +18,13 @@ import (
 	"github.com/go-chi/chi"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
-	"github.com/smallstep/assert"
-	"github.com/smallstep/certificates/acme"
+
 	"go.step.sm/crypto/jose"
 	"go.step.sm/crypto/pemutil"
+
+	"github.com/smallstep/assert"
+	"github.com/smallstep/certificates/acme"
+	"github.com/smallstep/certificates/authority/provisioner"
 )

 type mockClient struct {
@ -129,7 +132,35 @@ func TestHandler_GetDirectory(t *testing.T) {
 				NewOrder:   fmt.Sprintf("%s/acme/%s/new-order", baseURL.String(), provName),
 				RevokeCert: fmt.Sprintf("%s/acme/%s/revoke-cert", baseURL.String(), provName),
 				KeyChange:  fmt.Sprintf("%s/acme/%s/key-change", baseURL.String(), provName),
-				Meta: Meta{
+				Meta: &Meta{
+					ExternalAccountRequired: true,
+				},
+			}
+			return test{
+				ctx:        ctx,
+				dir:        expDir,
+				statusCode: 200,
+			}
+		},
+		"ok/full-meta": func(t *testing.T) test {
+			prov := newACMEProv(t)
+			prov.TermsOfService = "https://terms.ca.local/"
+			prov.Website = "https://ca.local/"
+			prov.CaaIdentities = []string{"ca.local"}
+			prov.RequireEAB = true
+			provName := url.PathEscape(prov.GetName())
+			baseURL := &url.URL{Scheme: "https", Host: "test.ca.smallstep.com"}
+			ctx := acme.NewProvisionerContext(context.Background(), prov)
+			expDir := Directory{
+				NewNonce:   fmt.Sprintf("%s/acme/%s/new-nonce", baseURL.String(), provName),
+				NewAccount: fmt.Sprintf("%s/acme/%s/new-account", baseURL.String(), provName),
+				NewOrder:   fmt.Sprintf("%s/acme/%s/new-order", baseURL.String(), provName),
+				RevokeCert: fmt.Sprintf("%s/acme/%s/revoke-cert", baseURL.String(), provName),
+				KeyChange:  fmt.Sprintf("%s/acme/%s/key-change", baseURL.String(), provName),
+				Meta: &Meta{
+					TermsOfService:          "https://terms.ca.local/",
+					Website:                 "https://ca.local/",
+					CaaIdentities:           []string{"ca.local"},
 					ExternalAccountRequired: true,
 				},
 			}
@ -751,3 +782,89 @@ func TestHandler_GetChallenge(t *testing.T) {
 		})
 	}
 }
+
+func Test_createMetaObject(t *testing.T) {
+	tests := []struct {
+		name string
+		p    *provisioner.ACME
+		want *Meta
+	}{
+		{
+			name: "no-meta",
+			p: &provisioner.ACME{
+				Type: "ACME",
+				Name: "acme",
+			},
+			want: nil,
+		},
+		{
+			name: "terms-of-service",
+			p: &provisioner.ACME{
+				Type:           "ACME",
+				Name:           "acme",
+				TermsOfService: "https://terms.ca.local",
+			},
+			want: &Meta{
+				TermsOfService: "https://terms.ca.local",
+			},
+		},
+		{
+			name: "website",
+			p: &provisioner.ACME{
+				Type:    "ACME",
+				Name:    "acme",
+				Website: "https://ca.local",
+			},
+			want: &Meta{
+				Website: "https://ca.local",
+			},
+		},
+		{
+			name: "caa",
+			p: &provisioner.ACME{
+				Type:          "ACME",
+				Name:          "acme",
+				CaaIdentities: []string{"ca.local", "ca.remote"},
+			},
+			want: &Meta{
+				CaaIdentities: []string{"ca.local", "ca.remote"},
+			},
+		},
+		{
+			name: "require-eab",
+			p: &provisioner.ACME{
+				Type:       "ACME",
+				Name:       "acme",
+				RequireEAB: true,
+			},
+			want: &Meta{
+				ExternalAccountRequired: true,
+			},
+		},
+		{
+			name: "full-meta",
+			p: &provisioner.ACME{
+				Type:           "ACME",
+				Name:           "acme",
+				TermsOfService: "https://terms.ca.local",
+				Website:        "https://ca.local",
+				CaaIdentities:  []string{"ca.local", "ca.remote"},
+				RequireEAB:     true,
+			},
+			want: &Meta{
+				TermsOfService:          "https://terms.ca.local",
+				Website:                 "https://ca.local",
+				CaaIdentities:           []string{"ca.local", "ca.remote"},
+				ExternalAccountRequired: true,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := createMetaObject(tt.p)
+			if !cmp.Equal(tt.want, got) {
+				t.Errorf("createMetaObject() diff =\n%s", cmp.Diff(tt.want, got))
+			}
+		})
+	}
+}
--- a/authority/provisioner/acme.go
+++ b/authority/provisioner/acme.go
@ -84,6 +84,17 @@ type ACME struct {
 	Type    string `json:"type"`
 	Name    string `json:"name"`
 	ForceCN bool   `json:"forceCN,omitempty"`
+	// TermsOfService contains a URL pointing to the ACME server's
+	// terms of service. Defaults to empty.
+	TermsOfService string `json:"termsOfService,omitempty"`
+	// Website contains an URL pointing to more information about
+	// the ACME server. Defaults to empty.
+	Website string `json:"website,omitempty"`
+	// CaaIdentities is an array of hostnames that the ACME server
+	// identifies itself with. These hostnames can be used by ACME
+	// clients to determine the correct issuer domain name to use
+	// when configuring CAA records. Defaults to empty array.
+	CaaIdentities []string `json:"caaIdentities,omitempty"`
 	// RequireEAB makes the provisioner require ACME EAB to be provided
 	// by clients when creating a new Account. If set to true, the provided
 	// EAB will be verified. If set to false and an EAB is provided, it is
--- a/authority/provisioners.go
+++ b/authority/provisioners.go
@ -880,6 +880,9 @@ func ProvisionerToCertificates(p *linkedca.Provisioner) (provisioner.Interface,
 			Type:               p.Type.String(),
 			Name:               p.Name,
 			ForceCN:            cfg.ForceCn,
+			TermsOfService:     cfg.TermsOfService,
+			Website:            cfg.Website,
+			CaaIdentities:      cfg.CaaIdentities,
 			RequireEAB:         cfg.RequireEab,
 			Challenges:         challengesToCertificates(cfg.Challenges),
 			AttestationFormats: attestationFormatsToCertificates(cfg.AttestationFormats),
@ -1138,6 +1141,10 @@ func ProvisionerToLinkedca(p provisioner.Interface) (*linkedca.Provisioner, erro
 				Data: &linkedca.ProvisionerDetails_ACME{
 					ACME: &linkedca.ACMEProvisioner{
 						ForceCn:            p.ForceCN,
+						TermsOfService:     p.TermsOfService,
+						Website:            p.Website,
+						CaaIdentities:      p.CaaIdentities,
+						RequireEab:         p.RequireEAB,
 						Challenges:         challengesToLinkedca(p.Challenges),
 						AttestationFormats: attestationFormatsToLinkedca(p.AttestationFormats),
 						AttestationRoots:   provisionerPEMToLinkedca(p.AttestationRoots),
--- a/go.mod
+++ b/go.mod
@ -44,7 +44,7 @@ require (
 	go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352
 	go.step.sm/cli-utils v0.7.5
 	go.step.sm/crypto v0.23.0
-	go.step.sm/linkedca v0.19.0-rc.3
+	go.step.sm/linkedca v0.19.0-rc.4
 	golang.org/x/crypto v0.0.0-20221005025214-4161e89ecf1b
 	golang.org/x/net v0.0.0-20221014081412-f15817d10f9b
 	golang.org/x/sys v0.0.0-20221006211917-84dc82d7e875 // indirect
--- a/go.sum
+++ b/go.sum
@ -688,6 +688,8 @@ go.step.sm/crypto v0.23.0 h1:pkkAlQxeDs+7qZ0mWSnN25qbtDm/AH6u0hYlwcmRWng=
 go.step.sm/crypto v0.23.0/go.mod h1:sK4iH/xyQDbffE1jCgj5hraVrbdKY9CTs0Lnjskxnk4=
 go.step.sm/linkedca v0.19.0-rc.3 h1:3Uu8j187wm7mby+/pz/aQ0wHKRm7w/2AsVPpvcAn4v8=
 go.step.sm/linkedca v0.19.0-rc.3/go.mod h1:MCZmPIdzElEofZbiw4eyUHayXgFTwa94cNAV34aJ5ew=
+go.step.sm/linkedca v0.19.0-rc.4 h1:kaBW+xHkRRgMNDa4gWiIj7gBq5yjbJKGlTWYYo5z2KQ=
+go.step.sm/linkedca v0.19.0-rc.4/go.mod h1:b7vWPrHfYLEOTSUZitFEcztVCpTc+ileIN85CwEAluM=
 go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
 go.uber.org/atomic v1.5.0/go.mod h1:sABNBOSYdrvTF6hTgEIbc7YasKWGhgEQZyfxyTvoXHQ=