From 0c418c84c44fe08ed89b1fcba7b9ab9eda959a02 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Thu, 25 May 2023 06:50:45 +0200 Subject: [PATCH 1/8] Add the ability to customise the details of the CA --- command/tls_ca_create.go | 67 +++++++++++++++++-- helper/tlsutil/generate.go | 58 ++++++++++++++-- helper/tlsutil/generate_test.go | 38 +++++++++++ .../content/docs/commands/tls/ca-create.mdx | 16 ++++- 4 files changed, 166 insertions(+), 13 deletions(-) diff --git a/command/tls_ca_create.go b/command/tls_ca_create.go index 3c946698681..7f81a89eb72 100644 --- a/command/tls_ca_create.go +++ b/command/tls_ca_create.go @@ -34,6 +34,27 @@ type TLSCACreateCommand struct { // additionalDomain provides a list of restricted domains to the CA which // will then reject any domains other than these. additionalDomain flags.StringFlag + + // country is used to set a country code for the CA + country string + + // postalCode is used to set a postal code for the CA + postalCode string + + // province is used to set a province for the CA + province string + + // locality is used to set a locality for the CA + locality string + + // streetAddress is used to set a street address for the CA + streetAddress string + + // organization is used to set an organization for the CA + organization string + + // organizationalUnit is used to set an organizational unit for the CA + organizationalUnit string } func (c *TLSCACreateCommand) Help() string { @@ -53,6 +74,9 @@ CA Create Options: -common-name Common Name of CA. Defaults to "Nomad Agent CA". + -country + Country of the CA. Defaults to "US". + -days Provide number of days the CA is valid for from now on. Defaults to 5 years or 1825 days. @@ -61,12 +85,31 @@ CA Create Options: Domain of Nomad cluster. Only used in combination with -name-constraint. Defaults to "nomad". + -locality + Locality of the CA. Defaults to "San Francisco". + -name-constraint Enables the DNS name restriction functionality to the CA. Results in the CA rejecting certificates for any other DNS zone. If enabled, localhost and the value of -domain will be added to the allowed DNS zones field. If the UI is going to be served over HTTPS its hostname must be added with -additional-domain. Defaults to false. + + -organization + Organization of the CA. Defaults to "HashiCorp Inc.". + + -organizational-unit + Organizational Unit of the CA. Defaults to "Nomad". + + -postal-code + Postal Code of the CA. Defaults to "94105". + + -province + Province of the CA. Defaults to "CA". + + -street-address + Street Address of the CA. Defaults to "101 Second Street". + ` return strings.TrimSpace(helpText) } @@ -74,11 +117,18 @@ CA Create Options: func (c *TLSCACreateCommand) AutocompleteFlags() complete.Flags { return mergeAutocompleteFlags(c.Meta.AutocompleteFlags(FlagSetClient), complete.Flags{ - "-additional-domain": complete.PredictAnything, - "-common-name": complete.PredictAnything, - "-days": complete.PredictAnything, - "-domain": complete.PredictAnything, - "-name-constraint": complete.PredictAnything, + "-additional-domain": complete.PredictAnything, + "-common-name": complete.PredictAnything, + "-days": complete.PredictAnything, + "-country": complete.PredictAnything, + "-domain": complete.PredictAnything, + "-locality": complete.PredictAnything, + "-name-constraint": complete.PredictAnything, + "-organization": complete.PredictAnything, + "-organizational-unit": complete.PredictAnything, + "-postcode": complete.PredictAnything, + "-province": complete.PredictAnything, + "-street-address": complete.PredictAnything, }) } @@ -101,6 +151,13 @@ func (c *TLSCACreateCommand) Run(args []string) int { flagSet.BoolVar(&c.constraint, "name-constraint", false, "") flagSet.StringVar(&c.domain, "domain", "nomad", "") flagSet.StringVar(&c.commonName, "common-name", "", "") + flagSet.StringVar(&c.country, "country", "", "") + flagSet.StringVar(&c.postalCode, "post-code", "", "") + flagSet.StringVar(&c.province, "province", "", "") + flagSet.StringVar(&c.locality, "locality", "", "") + flagSet.StringVar(&c.streetAddress, "street-address", "", "") + flagSet.StringVar(&c.organization, "organization", "", "") + flagSet.StringVar(&c.organizationalUnit, "organizational-unit", "", "") if err := flagSet.Parse(args); err != nil { return 1 } diff --git a/helper/tlsutil/generate.go b/helper/tlsutil/generate.go index 75a1a68ad99..596ec33314c 100644 --- a/helper/tlsutil/generate.go +++ b/helper/tlsutil/generate.go @@ -67,6 +67,13 @@ type CAOpts struct { Days int PermittedDNSDomains []string Domain string + Country string + PostalCode string + Province string + Locality string + StreetAddress string + Organization string + OrganizationalUnit string Name string } @@ -106,6 +113,7 @@ func GenerateCA(opts CAOpts) (string, string, error) { return "", "", err } } + name := opts.Name if name == "" { name = fmt.Sprintf("Nomad Agent CA %d", sn) @@ -116,17 +124,53 @@ func GenerateCA(opts CAOpts) (string, string, error) { days = 365 } + country := opts.Country + if country == "" { + country = ("US") + } + + postalCode := opts.PostalCode + if postalCode == "" { + postalCode = ("94105") + } + + province := opts.Province + if province == "" { + province = ("CA") + } + + locality := opts.Locality + if locality == "" { + locality = ("San Francisco") + } + + streetAddress := opts.StreetAddress + if streetAddress == "" { + streetAddress = ("101 Second Street") + } + + organization := opts.Organization + if organization == "" { + organization = ("HashiCorp Inc.") + } + + organizationalUnit := opts.OrganizationalUnit + if organizationalUnit == "" { + organizationalUnit = ("Nomad") + } + // Create the CA cert template := x509.Certificate{ SerialNumber: sn, Subject: pkix.Name{ - Country: []string{"US"}, - PostalCode: []string{"94105"}, - Province: []string{"CA"}, - Locality: []string{"San Francisco"}, - StreetAddress: []string{"101 Second Street"}, - Organization: []string{"HashiCorp Inc."}, - CommonName: name, + Country: []string{country}, + PostalCode: []string{postalCode}, + Province: []string{province}, + Locality: []string{locality}, + StreetAddress: []string{streetAddress}, + Organization: []string{organization}, + OrganizationalUnit: []string{organizationalUnit}, + CommonName: name, }, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature, diff --git a/helper/tlsutil/generate_test.go b/helper/tlsutil/generate_test.go index 31671558caa..8349513ca56 100644 --- a/helper/tlsutil/generate_test.go +++ b/helper/tlsutil/generate_test.go @@ -116,6 +116,44 @@ func TestGenerateCA(t *testing.T) { require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) }) + + t.Run("Custom CA", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 6, + PermittedDNSDomains: []string{"domain1.com"}, + Domain: "custdomain.com", + Country: "ZZ", + PostalCode: "0000", + Province: "CustProvince", + Locality: "CustLocality", + StreetAddress: "CustStreet", + Organization: "CustOrg", + OrganizationalUnit: "CustUnit", + Name: "Custom CA", + }) + require.NoError(t, err) + require.NotEmpty(t, ca) + require.NotEmpty(t, pk) + + cert, err := parseCert(ca) + require.NoError(t, err) + require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Custom CA")) + require.True(t, strings.Contains(cert.PermittedDNSDomains[0], "domain1.com")) + require.True(t, strings.Contains(cert.Subject.Country[0], "ZZ")) + require.True(t, strings.Contains(cert.Subject.PostalCode[0], "0000")) + require.True(t, strings.Contains(cert.Subject.Province[0], "CustProvince")) + require.True(t, strings.Contains(cert.Subject.Locality[0], "CustLocality")) + require.True(t, strings.Contains(cert.Subject.StreetAddress[0], "CustStreet")) + require.True(t, strings.Contains(cert.Subject.Organization[0], "CustOrg")) + require.True(t, strings.Contains(cert.Subject.OrganizationalUnit[0], "CustUnit")) + require.Equal(t, true, cert.IsCA) + require.Equal(t, true, cert.BasicConstraintsValid) + + require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 6), time.Minute) + + require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) + }) } func TestGenerateCert(t *testing.T) { diff --git a/website/content/docs/commands/tls/ca-create.mdx b/website/content/docs/commands/tls/ca-create.mdx index 01deb51ec0f..3e90a45c4f8 100644 --- a/website/content/docs/commands/tls/ca-create.mdx +++ b/website/content/docs/commands/tls/ca-create.mdx @@ -24,7 +24,9 @@ nomad tls ca create [options] `-additional-domain`. Can be used multiple times. This option can only used in combination with `-domain` and `-name-constraint`. -- `common-name`: Common Name of CA. Defaults to Nomad Agent CA. +- `-common-name`: Common Name of CA. Defaults to Nomad Agent CA. + +- `-country`: Country of the CA. Defaults to "US". - `-days=`: Provide number of days the CA is valid for from now on, defaults to 5 years. @@ -32,6 +34,8 @@ nomad tls ca create [options] - `-domain=`: Domain of nomad cluster. Only used in combination with `-name-constraint`. Defaults to `nomad`. +- `-locality`: Locality of the CA. Defaults to "San Francisco". + - `-name-constraint`: Add name constraints for the CA. Results in rejecting certificates for other DNS than specified. If set to true, "localhost" and `-domain` will be added to the allowed DNS. Defaults to false. @@ -40,6 +44,16 @@ nomad tls ca create [options] Nomad web UI over HTTPS its DNS must be added with `additional-domain`. It is not possible to add that after the fact. +- `-organization`: Organization of the CA. Defaults to "HashiCorp Inc.". + +- `-organizational-unit`: Organizational Unit of the CA. Defaults to "Nomad". + +- `-postal-code`: Postal Code of the CA. Defaults to "94105". + +- `-province`: Province of the CA. Defaults to "CA". + +- `-street-address`: Street Address of the CA. Defaults to "101 Second Street". + ## Example Create CA: From 36ef94a9029bacd2e2f104957528792375d541b3 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Thu, 25 May 2023 07:38:13 +0200 Subject: [PATCH 2/8] Create 17309.txt Add changelog entry --- .changelog/17309.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/17309.txt diff --git a/.changelog/17309.txt b/.changelog/17309.txt new file mode 100644 index 00000000000..6ca157629f1 --- /dev/null +++ b/.changelog/17309.txt @@ -0,0 +1,3 @@ +```release-note:improvement +tls: Add the ability to customize the details of the CA +``` \ No newline at end of file From 3e59966189e116e8e9b50bfa1afcc32c5a9e96bc Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Fri, 26 May 2023 14:11:29 +0200 Subject: [PATCH 3/8] Update .changelog/17309.txt Co-authored-by: James Rasell --- .changelog/17309.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/17309.txt b/.changelog/17309.txt index 6ca157629f1..8159f6293e7 100644 --- a/.changelog/17309.txt +++ b/.changelog/17309.txt @@ -1,3 +1,3 @@ ```release-note:improvement -tls: Add the ability to customize the details of the CA +cli: Add the ability to customize the details of the CA when running `nomad tls ca create` ``` \ No newline at end of file From 96e921cacb7e73ae3f6881546a3cc6519be14f11 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Mon, 5 Jun 2023 21:13:29 +0200 Subject: [PATCH 4/8] Add the Ability to customize the CA Added the ability to provide custom data to create CAs improved testing. --- command/tls_ca_create.go | 20 +++- command/tls_ca_create_test.go | 20 ---- helper/tlsutil/generate.go | 161 +++++++++++++++++++------------- helper/tlsutil/generate_test.go | 71 +++++++++++++- nomad/rpc_test.go | 9 +- 5 files changed, 185 insertions(+), 96 deletions(-) diff --git a/command/tls_ca_create.go b/command/tls_ca_create.go index 7f81a89eb72..5259aa6c1d6 100644 --- a/command/tls_ca_create.go +++ b/command/tls_ca_create.go @@ -126,7 +126,7 @@ func (c *TLSCACreateCommand) AutocompleteFlags() complete.Flags { "-name-constraint": complete.PredictAnything, "-organization": complete.PredictAnything, "-organizational-unit": complete.PredictAnything, - "-postcode": complete.PredictAnything, + "-postal-code": complete.PredictAnything, "-province": complete.PredictAnything, "-street-address": complete.PredictAnything, }) @@ -147,12 +147,12 @@ func (c *TLSCACreateCommand) Run(args []string) int { flagSet := c.Meta.FlagSet(c.Name(), FlagSetClient) flagSet.Usage = func() { c.Ui.Output(c.Help()) } flagSet.Var(&c.additionalDomain, "additional-domain", "") - flagSet.IntVar(&c.days, "days", 1825, "") + flagSet.IntVar(&c.days, "days", 0, "") flagSet.BoolVar(&c.constraint, "name-constraint", false, "") flagSet.StringVar(&c.domain, "domain", "nomad", "") flagSet.StringVar(&c.commonName, "common-name", "", "") flagSet.StringVar(&c.country, "country", "", "") - flagSet.StringVar(&c.postalCode, "post-code", "", "") + flagSet.StringVar(&c.postalCode, "postal-code", "", "") flagSet.StringVar(&c.province, "province", "", "") flagSet.StringVar(&c.locality, "locality", "", "") flagSet.StringVar(&c.streetAddress, "street-address", "", "") @@ -200,7 +200,19 @@ func (c *TLSCACreateCommand) Run(args []string) int { constraints = append(constraints, c.additionalDomain...) } - ca, pk, err := tlsutil.GenerateCA(tlsutil.CAOpts{Name: c.commonName, Days: c.days, Domain: c.domain, PermittedDNSDomains: constraints}) + ca, pk, err := tlsutil.GenerateCA(tlsutil.CAOpts{ + Name: c.commonName, + Days: c.days, + Domain: c.domain, + PermittedDNSDomains: constraints, + Country: c.country, + PostalCode: c.postalCode, + Province: c.province, + Locality: c.locality, + StreetAddress: c.streetAddress, + Organization: c.organization, + OrganizationalUnit: c.organizationalUnit, + }) if err != nil { c.Ui.Error(err.Error()) return 1 diff --git a/command/tls_ca_create_test.go b/command/tls_ca_create_test.go index d4da1cbce4f..2c13f8ec627 100644 --- a/command/tls_ca_create_test.go +++ b/command/tls_ca_create_test.go @@ -6,7 +6,6 @@ package command import ( "crypto/x509" "os" - "strings" "testing" "time" @@ -57,24 +56,6 @@ func TestCACreateCommand(t *testing.T) { require.ElementsMatch(t, cert.PermittedDNSDomains, []string{"nomad", "foo", "localhost", "bar"}) }, }, - {"with common-name", - []string{ - "-common-name=foo", - }, - "nomad-agent-ca.pem", - "nomad-agent-ca-key.pem", - func(t *testing.T, cert *x509.Certificate) { - require.Equal(t, cert.Subject.CommonName, "foo") - }, - }, - {"without common-name", - []string{}, - "nomad-agent-ca.pem", - "nomad-agent-ca-key.pem", - func(t *testing.T, cert *x509.Certificate) { - require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Nomad Agent CA")) - }, - }, } for _, tc := range cases { tc := tc @@ -97,5 +78,4 @@ func TestCACreateCommand(t *testing.T) { require.NoError(t, os.Remove(tc.keyPath)) }) } - } diff --git a/helper/tlsutil/generate.go b/helper/tlsutil/generate.go index 596ec33314c..1c893fc5e52 100644 --- a/helper/tlsutil/generate.go +++ b/helper/tlsutil/generate.go @@ -14,6 +14,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "errors" "fmt" "math/big" "net" @@ -88,94 +89,122 @@ type CertOpts struct { ExtKeyUsage []x509.ExtKeyUsage } +// Check if any of CAOpts the variables are populated +func AreCARequiredFieldsEmpty(caOpts CAOpts) bool { + return caOpts.Days == 0 && + caOpts.Country == "" && + caOpts.PostalCode == "" && + caOpts.Province == "" && + caOpts.Locality == "" && + caOpts.StreetAddress == "" && + caOpts.Organization == "" && + caOpts.OrganizationalUnit == "" && + caOpts.Name == "" +} + // GenerateCA generates a new CA for agent TLS (not to be confused with Connect TLS) func GenerateCA(opts CAOpts) (string, string, error) { - signer := opts.Signer - var pk string - if signer == nil { - var err error - signer, pk, err = GeneratePrivateKey() + var ( + id []byte + pk string + err error + signer = opts.Signer + sn = opts.Serial + ) + + // Check if opts required fields are empty + isEmpty := AreCARequiredFieldsEmpty(opts) + if isEmpty { + + if signer == nil { + var err error + signer, pk, err = GeneratePrivateKey() + if err != nil { + return "", "", err + } + } + + certKeyId, err := keyID(signer.Public()) if err != nil { return "", "", err } - } - - id, err := keyID(signer.Public()) - if err != nil { - return "", "", err - } + id = certKeyId + + if sn == nil { + var err error + sn, err = GenerateSerialNumber() + if err != nil { + return "", "", err + } + } + opts.Name = fmt.Sprintf("Nomad Agent CA %d", sn) + opts.Days = 1825 + opts.Country = "US" + opts.PostalCode = "94105" + opts.Province = "CA" + opts.Locality = "San Francisco" + opts.StreetAddress = "101 Second Street" + opts.Organization = "HashiCorp Inc." + opts.OrganizationalUnit = "Nomad" + + } else { + if signer == nil { + var err error + signer, pk, err = GeneratePrivateKey() + if err != nil { + return "", "", err + } + } - sn := opts.Serial - if sn == nil { - var err error - sn, err = GenerateSerialNumber() + certKeyId, err := keyID(signer.Public()) if err != nil { return "", "", err } - } - - name := opts.Name - if name == "" { - name = fmt.Sprintf("Nomad Agent CA %d", sn) - } - - days := opts.Days - if opts.Days == 0 { - days = 365 - } - - country := opts.Country - if country == "" { - country = ("US") - } - - postalCode := opts.PostalCode - if postalCode == "" { - postalCode = ("94105") - } - - province := opts.Province - if province == "" { - province = ("CA") - } - - locality := opts.Locality - if locality == "" { - locality = ("San Francisco") - } + id = certKeyId + + if sn == nil { + var err error + sn, err = GenerateSerialNumber() + if err != nil { + return "", "", err + } + } - streetAddress := opts.StreetAddress - if streetAddress == "" { - streetAddress = ("101 Second Street") - } + if opts.Name == "" { + return "", "", errors.New("please provide the -common-name flag when customizing the CA") + } else { + opts.Name = fmt.Sprintf("%s %d", opts.Name, sn) + } + if opts.Country == "" { + return "", "", errors.New("please provide the -country flag when customizing the CA") + } - organization := opts.Organization - if organization == "" { - organization = ("HashiCorp Inc.") - } + if opts.Organization == "" { + return "", "", errors.New("please provide the -organization flag when customizing the CA") + } - organizationalUnit := opts.OrganizationalUnit - if organizationalUnit == "" { - organizationalUnit = ("Nomad") + if opts.OrganizationalUnit == "" { + return "", "", errors.New("please provide the -organizational-unit flag when customizing the CA") + } } // Create the CA cert template := x509.Certificate{ SerialNumber: sn, Subject: pkix.Name{ - Country: []string{country}, - PostalCode: []string{postalCode}, - Province: []string{province}, - Locality: []string{locality}, - StreetAddress: []string{streetAddress}, - Organization: []string{organization}, - OrganizationalUnit: []string{organizationalUnit}, - CommonName: name, + Country: []string{opts.Country}, + PostalCode: []string{opts.PostalCode}, + Province: []string{opts.Province}, + Locality: []string{opts.Locality}, + StreetAddress: []string{opts.StreetAddress}, + Organization: []string{opts.Organization}, + OrganizationalUnit: []string{opts.OrganizationalUnit}, + CommonName: opts.Name, }, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature, IsCA: true, - NotAfter: time.Now().AddDate(0, 0, days), + NotAfter: time.Now().AddDate(0, 0, opts.Days), NotBefore: time.Now(), AuthorityKeyId: id, SubjectKeyId: id, diff --git a/helper/tlsutil/generate_test.go b/helper/tlsutil/generate_test.go index 8349513ca56..8cc00a19895 100644 --- a/helper/tlsutil/generate_test.go +++ b/helper/tlsutil/generate_test.go @@ -94,7 +94,7 @@ func TestGenerateCA(t *testing.T) { require.Equal(t, true, cert.BasicConstraintsValid) require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) - require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 1825), time.Minute) require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) }) @@ -112,7 +112,7 @@ func TestGenerateCA(t *testing.T) { require.Equal(t, true, cert.BasicConstraintsValid) require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) - require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 1825), time.Minute) require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) }) @@ -154,6 +154,61 @@ func TestGenerateCA(t *testing.T) { require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) }) + + t.Run("Custom CA No CN", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 6, + PermittedDNSDomains: []string{"domain1.com"}, + Domain: "custdomain.com", + Locality: "CustLocality", + }) + require.ErrorContains(t, err, "-common-name") + require.Empty(t, ca) + require.Empty(t, pk) + }) + + t.Run("Custom CA No Country", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 6, + PermittedDNSDomains: []string{"domain1.com"}, + Domain: "custdomain.com", + Name: "Custom CA", + Locality: "CustLocality", + }) + require.ErrorContains(t, err, "-country") + require.Empty(t, ca) + require.Empty(t, pk) + }) + + t.Run("Custom CA No Organization", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 6, + PermittedDNSDomains: []string{"domain1.com"}, + Domain: "custdomain.com", + Name: "Custom CA", + Country: "ZZ", + Locality: "CustLocality", + }) + require.ErrorContains(t, err, "-organization") + // require.NoError(t, err) + require.Empty(t, ca) + require.Empty(t, pk) + }) + + t.Run("Custom CA No Organizational Unit", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 6, + PermittedDNSDomains: []string{"domain1.com"}, + Domain: "custdomain.com", + Name: "Custom CA", + Country: "ZZ", + Locality: "CustLocality", + Organization: "CustOrg", + }) + require.ErrorContains(t, err, "-organizational-unit") + require.Empty(t, ca) + require.Empty(t, pk) + }) } func TestGenerateCert(t *testing.T) { @@ -161,10 +216,16 @@ func TestGenerateCert(t *testing.T) { signer, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.Nil(t, err) - ca, _, err := GenerateCA(CAOpts{Signer: signer}) + ca, _, err := GenerateCA(CAOpts{ + Name: "Custom CA", + Country: "ZZ", + Organization: "CustOrg", + OrganizationalUnit: "CustOrgUnit", + Signer: signer}, + ) require.Nil(t, err) - DNSNames := []string{"server.dc1.consul"} + DNSNames := []string{"server.dc1.nomad"} IPAddresses := []net.IP{net.ParseIP("123.234.243.213")} extKeyUsage := []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth} name := "Cert Name" @@ -188,7 +249,7 @@ func TestGenerateCert(t *testing.T) { caID, err := keyID(signer.Public()) require.Nil(t, err) require.Equal(t, caID, cert.AuthorityKeyId) - require.Contains(t, cert.Issuer.CommonName, "Nomad Agent CA") + require.Contains(t, cert.Issuer.CommonName, "Custom CA") require.Equal(t, false, cert.IsCA) require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) diff --git a/nomad/rpc_test.go b/nomad/rpc_test.go index 0bebfd57f86..d49063d0073 100644 --- a/nomad/rpc_test.go +++ b/nomad/rpc_test.go @@ -1370,7 +1370,14 @@ func newTLSTestHelper(t *testing.T) tlsTestHelper { } // Generate CA certificate and write it to disk. - h.caPEM, h.pk, err = tlsutil.GenerateCA(tlsutil.CAOpts{Days: 5, Domain: "nomad"}) + h.caPEM, h.pk, err = tlsutil.GenerateCA(tlsutil.CAOpts{ + Name: "Nomad CA", + Country: "ZZ", + Days: 5, + Domain: "nomad", + Organization: "CustOrgUnit", + OrganizationalUnit: "CustOrgUnit", + }) must.NoError(t, err) err = os.WriteFile(filepath.Join(h.dir, "ca.pem"), []byte(h.caPEM), 0600) From 4da2936ee0dbafe07032fad8e3a4efdc511e1943 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Tue, 6 Jun 2023 09:40:47 +0200 Subject: [PATCH 5/8] Update tls_ca_create_test.go Update Tests --- command/tls_ca_create_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/command/tls_ca_create_test.go b/command/tls_ca_create_test.go index 2c13f8ec627..577db92a7d1 100644 --- a/command/tls_ca_create_test.go +++ b/command/tls_ca_create_test.go @@ -46,6 +46,10 @@ func TestCACreateCommand(t *testing.T) { "-name-constraint=true", "-domain=foo", "-additional-domain=bar", + "-common-name=CustomCA", + "-country=ZZ", + "-organization=CustOrg", + "-organizational-unit=CustOrgUnit", }, "foo-agent-ca.pem", "foo-agent-ca-key.pem", @@ -54,6 +58,10 @@ func TestCACreateCommand(t *testing.T) { require.True(t, cert.PermittedDNSDomainsCritical) require.Len(t, cert.PermittedDNSDomains, 4) require.ElementsMatch(t, cert.PermittedDNSDomains, []string{"nomad", "foo", "localhost", "bar"}) + require.Equal(t, cert.Issuer.Organization, []string{"CustOrg"}) + require.Equal(t, cert.Issuer.OrganizationalUnit, []string{"CustOrgUnit"}) + require.Equal(t, cert.Issuer.Country, []string{"ZZ"}) + require.Contains(t, cert.Issuer.CommonName, "CustomCA") }, }, } From 8cf9c473c15c2411ed6a26dae75eb136db20d6de Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Thu, 8 Jun 2023 10:36:56 +0200 Subject: [PATCH 6/8] Update generate.go Apply suggestion by @jrasell to compose the function off the struct. --- helper/tlsutil/generate.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/helper/tlsutil/generate.go b/helper/tlsutil/generate.go index 1c893fc5e52..db816767f7d 100644 --- a/helper/tlsutil/generate.go +++ b/helper/tlsutil/generate.go @@ -89,17 +89,18 @@ type CertOpts struct { ExtKeyUsage []x509.ExtKeyUsage } -// Check if any of CAOpts the variables are populated -func AreCARequiredFieldsEmpty(caOpts CAOpts) bool { - return caOpts.Days == 0 && - caOpts.Country == "" && - caOpts.PostalCode == "" && - caOpts.Province == "" && - caOpts.Locality == "" && - caOpts.StreetAddress == "" && - caOpts.Organization == "" && - caOpts.OrganizationalUnit == "" && - caOpts.Name == "" +// IsEmpty checks whether any of CAOpts parameters have been populated with +// non-default values. +func (c CAOpts) IsEmpty() bool { + return c.Days == 0 && + c.Country == "" && + c.PostalCode == "" && + c.Province == "" && + c.Locality == "" && + c.StreetAddress == "" && + c.Organization == "" && + c.OrganizationalUnit == "" && + c.Name == "" } // GenerateCA generates a new CA for agent TLS (not to be confused with Connect TLS) @@ -112,9 +113,7 @@ func GenerateCA(opts CAOpts) (string, string, error) { sn = opts.Serial ) - // Check if opts required fields are empty - isEmpty := AreCARequiredFieldsEmpty(opts) - if isEmpty { + if opts.IsEmpty() { if signer == nil { var err error From ff6e6bf07ac0b1ef816cad733d36cd0df6016ee5 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Thu, 15 Jun 2023 20:04:00 +0200 Subject: [PATCH 7/8] Update ca create code to be more generic for later usage. --- command/tls_ca_create.go | 45 ++++++++++++++++++- command/tls_ca_create_test.go | 10 +++++ helper/tlsutil/generate.go | 80 +++++++++++++-------------------- helper/tlsutil/generate_test.go | 25 +++++++---- nomad/rpc_test.go | 1 - 5 files changed, 100 insertions(+), 61 deletions(-) diff --git a/command/tls_ca_create.go b/command/tls_ca_create.go index 5259aa6c1d6..f3b545c6aa6 100644 --- a/command/tls_ca_create.go +++ b/command/tls_ca_create.go @@ -149,7 +149,7 @@ func (c *TLSCACreateCommand) Run(args []string) int { flagSet.Var(&c.additionalDomain, "additional-domain", "") flagSet.IntVar(&c.days, "days", 0, "") flagSet.BoolVar(&c.constraint, "name-constraint", false, "") - flagSet.StringVar(&c.domain, "domain", "nomad", "") + flagSet.StringVar(&c.domain, "domain", "", "") flagSet.StringVar(&c.commonName, "common-name", "", "") flagSet.StringVar(&c.country, "country", "", "") flagSet.StringVar(&c.postalCode, "postal-code", "", "") @@ -169,6 +169,34 @@ func (c *TLSCACreateCommand) Run(args []string) int { c.Ui.Error(commandErrorText(c)) return 1 } + if c.IsEmpty() { + c.domain = "nomad" + } else if c.IsEmpty() && c.days != 0 { + c.domain = "nomad" + } else { + if c.commonName == "" { + c.Ui.Error("please provide the -common-name flag when customizing the CA") + c.Ui.Error(commandErrorText(c)) + return 1 + } + if c.country == "" { + c.Ui.Error("please provide the -country flag when customizing the CA") + c.Ui.Error(commandErrorText(c)) + return 1 + } + + if c.organization == "" { + c.Ui.Error("please provide the -organization flag when customizing the CA") + c.Ui.Error(commandErrorText(c)) + return 1 + } + + if c.organizationalUnit == "" { + c.Ui.Error("please provide the -organizational-unit flag when customizing the CA") + c.Ui.Error(commandErrorText(c)) + return 1 + } + } if c.domain != "" && c.domain != "nomad" && !c.constraint { c.Ui.Error("Please provide the -name-constraint flag to use a custom domain constraint") return 1 @@ -203,7 +231,6 @@ func (c *TLSCACreateCommand) Run(args []string) int { ca, pk, err := tlsutil.GenerateCA(tlsutil.CAOpts{ Name: c.commonName, Days: c.days, - Domain: c.domain, PermittedDNSDomains: constraints, Country: c.country, PostalCode: c.postalCode, @@ -232,3 +259,17 @@ func (c *TLSCACreateCommand) Run(args []string) int { return 0 } + +// IsEmpty checks whether any of TLSCACreateCommand parameters have been populated with +// non-default values. +func (c TLSCACreateCommand) IsEmpty() bool { + return c.commonName == "" && + c.country == "" && + c.postalCode == "" && + c.province == "" && + c.locality == "" && + c.streetAddress == "" && + c.organization == "" && + c.organizationalUnit == "" + +} diff --git a/command/tls_ca_create_test.go b/command/tls_ca_create_test.go index 577db92a7d1..2755f079dc4 100644 --- a/command/tls_ca_create_test.go +++ b/command/tls_ca_create_test.go @@ -64,6 +64,16 @@ func TestCACreateCommand(t *testing.T) { require.Contains(t, cert.Issuer.CommonName, "CustomCA") }, }, + {"ca custom date", + []string{ + "-days=365", + }, + "nomad-agent-ca.pem", + "nomad-agent-ca-key.pem", + func(t *testing.T, cert *x509.Certificate) { + require.Equal(t, 365*24*time.Hour, time.Until(cert.NotAfter).Round(24*time.Hour)) + }, + }, } for _, tc := range cases { tc := tc diff --git a/helper/tlsutil/generate.go b/helper/tlsutil/generate.go index db816767f7d..acb344f3bc4 100644 --- a/helper/tlsutil/generate.go +++ b/helper/tlsutil/generate.go @@ -67,7 +67,6 @@ type CAOpts struct { Serial *big.Int Days int PermittedDNSDomains []string - Domain string Country string PostalCode string Province string @@ -92,8 +91,7 @@ type CertOpts struct { // IsEmpty checks whether any of CAOpts parameters have been populated with // non-default values. func (c CAOpts) IsEmpty() bool { - return c.Days == 0 && - c.Country == "" && + return c.Country == "" && c.PostalCode == "" && c.Province == "" && c.Locality == "" && @@ -112,30 +110,28 @@ func GenerateCA(opts CAOpts) (string, string, error) { signer = opts.Signer sn = opts.Serial ) - - if opts.IsEmpty() { - - if signer == nil { - var err error - signer, pk, err = GeneratePrivateKey() - if err != nil { - return "", "", err - } + if signer == nil { + var err error + signer, pk, err = GeneratePrivateKey() + if err != nil { + return "", "", err } + } + + id, err = keyID(signer.Public()) + if err != nil { + return "", "", err + } - certKeyId, err := keyID(signer.Public()) + if sn == nil { + var err error + sn, err = GenerateSerialNumber() if err != nil { return "", "", err } - id = certKeyId - - if sn == nil { - var err error - sn, err = GenerateSerialNumber() - if err != nil { - return "", "", err - } - } + } + + if opts.IsEmpty() && opts.Days == 0 { opts.Name = fmt.Sprintf("Nomad Agent CA %d", sn) opts.Days = 1825 opts.Country = "US" @@ -145,45 +141,31 @@ func GenerateCA(opts CAOpts) (string, string, error) { opts.StreetAddress = "101 Second Street" opts.Organization = "HashiCorp Inc." opts.OrganizationalUnit = "Nomad" - + } else if opts.IsEmpty() && opts.Days != 0 { + opts.Name = fmt.Sprintf("Nomad Agent CA %d", sn) + opts.Country = "US" + opts.PostalCode = "94105" + opts.Province = "CA" + opts.Locality = "San Francisco" + opts.StreetAddress = "101 Second Street" + opts.Organization = "HashiCorp Inc." + opts.OrganizationalUnit = "Nomad" } else { - if signer == nil { - var err error - signer, pk, err = GeneratePrivateKey() - if err != nil { - return "", "", err - } - } - - certKeyId, err := keyID(signer.Public()) - if err != nil { - return "", "", err - } - id = certKeyId - - if sn == nil { - var err error - sn, err = GenerateSerialNumber() - if err != nil { - return "", "", err - } - } - if opts.Name == "" { - return "", "", errors.New("please provide the -common-name flag when customizing the CA") + return "", "", errors.New("common name value not provided") } else { opts.Name = fmt.Sprintf("%s %d", opts.Name, sn) } if opts.Country == "" { - return "", "", errors.New("please provide the -country flag when customizing the CA") + return "", "", errors.New("country value not provided") } if opts.Organization == "" { - return "", "", errors.New("please provide the -organization flag when customizing the CA") + return "", "", errors.New("organization value not provided") } if opts.OrganizationalUnit == "" { - return "", "", errors.New("please provide the -organizational-unit flag when customizing the CA") + return "", "", errors.New("organizational unit value not provided") } } diff --git a/helper/tlsutil/generate_test.go b/helper/tlsutil/generate_test.go index 8cc00a19895..55431b543da 100644 --- a/helper/tlsutil/generate_test.go +++ b/helper/tlsutil/generate_test.go @@ -121,7 +121,6 @@ func TestGenerateCA(t *testing.T) { ca, pk, err := GenerateCA(CAOpts{ Days: 6, PermittedDNSDomains: []string{"domain1.com"}, - Domain: "custdomain.com", Country: "ZZ", PostalCode: "0000", Province: "CustProvince", @@ -155,14 +154,25 @@ func TestGenerateCA(t *testing.T) { require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) }) + t.Run("Custom CA Custom Date", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{ + Days: 365, + }) + require.NoError(t, err) + require.NotEmpty(t, ca) + require.NotEmpty(t, pk) + + cert, err := parseCert(ca) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) + }) + t.Run("Custom CA No CN", func(t *testing.T) { ca, pk, err := GenerateCA(CAOpts{ Days: 6, PermittedDNSDomains: []string{"domain1.com"}, - Domain: "custdomain.com", Locality: "CustLocality", }) - require.ErrorContains(t, err, "-common-name") + require.ErrorContains(t, err, "common name value not provided") require.Empty(t, ca) require.Empty(t, pk) }) @@ -171,11 +181,10 @@ func TestGenerateCA(t *testing.T) { ca, pk, err := GenerateCA(CAOpts{ Days: 6, PermittedDNSDomains: []string{"domain1.com"}, - Domain: "custdomain.com", Name: "Custom CA", Locality: "CustLocality", }) - require.ErrorContains(t, err, "-country") + require.ErrorContains(t, err, "country value not provided") require.Empty(t, ca) require.Empty(t, pk) }) @@ -184,12 +193,11 @@ func TestGenerateCA(t *testing.T) { ca, pk, err := GenerateCA(CAOpts{ Days: 6, PermittedDNSDomains: []string{"domain1.com"}, - Domain: "custdomain.com", Name: "Custom CA", Country: "ZZ", Locality: "CustLocality", }) - require.ErrorContains(t, err, "-organization") + require.ErrorContains(t, err, "organization value not provided") // require.NoError(t, err) require.Empty(t, ca) require.Empty(t, pk) @@ -199,13 +207,12 @@ func TestGenerateCA(t *testing.T) { ca, pk, err := GenerateCA(CAOpts{ Days: 6, PermittedDNSDomains: []string{"domain1.com"}, - Domain: "custdomain.com", Name: "Custom CA", Country: "ZZ", Locality: "CustLocality", Organization: "CustOrg", }) - require.ErrorContains(t, err, "-organizational-unit") + require.ErrorContains(t, err, "organizational unit value not provided") require.Empty(t, ca) require.Empty(t, pk) }) diff --git a/nomad/rpc_test.go b/nomad/rpc_test.go index d49063d0073..4ac6ffb4c70 100644 --- a/nomad/rpc_test.go +++ b/nomad/rpc_test.go @@ -1374,7 +1374,6 @@ func newTLSTestHelper(t *testing.T) tlsTestHelper { Name: "Nomad CA", Country: "ZZ", Days: 5, - Domain: "nomad", Organization: "CustOrgUnit", OrganizationalUnit: "CustOrgUnit", }) From 4bc9d81bfc479017602f3f2556dbc9a91c23cdb4 Mon Sep 17 00:00:00 2001 From: Lance Haig Date: Fri, 23 Jun 2023 16:14:33 +0200 Subject: [PATCH 8/8] Update with suggestions from @jrasell --- command/tls_ca_create.go | 16 +++++++--------- helper/tlsutil/generate.go | 19 ++++++------------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/command/tls_ca_create.go b/command/tls_ca_create.go index f3b545c6aa6..198edae87e9 100644 --- a/command/tls_ca_create.go +++ b/command/tls_ca_create.go @@ -169,30 +169,28 @@ func (c *TLSCACreateCommand) Run(args []string) int { c.Ui.Error(commandErrorText(c)) return 1 } - if c.IsEmpty() { - c.domain = "nomad" - } else if c.IsEmpty() && c.days != 0 { + if c.IsCustom() && c.days != 0 || c.IsCustom() { c.domain = "nomad" } else { if c.commonName == "" { - c.Ui.Error("please provide the -common-name flag when customizing the CA") + c.Ui.Error("Please provide the -common-name flag when customizing the CA") c.Ui.Error(commandErrorText(c)) return 1 } if c.country == "" { - c.Ui.Error("please provide the -country flag when customizing the CA") + c.Ui.Error("Please provide the -country flag when customizing the CA") c.Ui.Error(commandErrorText(c)) return 1 } if c.organization == "" { - c.Ui.Error("please provide the -organization flag when customizing the CA") + c.Ui.Error("Please provide the -organization flag when customizing the CA") c.Ui.Error(commandErrorText(c)) return 1 } if c.organizationalUnit == "" { - c.Ui.Error("please provide the -organizational-unit flag when customizing the CA") + c.Ui.Error("Please provide the -organizational-unit flag when customizing the CA") c.Ui.Error(commandErrorText(c)) return 1 } @@ -260,9 +258,9 @@ func (c *TLSCACreateCommand) Run(args []string) int { return 0 } -// IsEmpty checks whether any of TLSCACreateCommand parameters have been populated with +// IsCustom checks whether any of TLSCACreateCommand parameters have been populated with // non-default values. -func (c TLSCACreateCommand) IsEmpty() bool { +func (c *TLSCACreateCommand) IsCustom() bool { return c.commonName == "" && c.country == "" && c.postalCode == "" && diff --git a/helper/tlsutil/generate.go b/helper/tlsutil/generate.go index acb344f3bc4..1db753fec28 100644 --- a/helper/tlsutil/generate.go +++ b/helper/tlsutil/generate.go @@ -88,9 +88,9 @@ type CertOpts struct { ExtKeyUsage []x509.ExtKeyUsage } -// IsEmpty checks whether any of CAOpts parameters have been populated with +// IsCustom checks whether any of CAOpts parameters have been populated with // non-default values. -func (c CAOpts) IsEmpty() bool { +func (c *CAOpts) IsCustom() bool { return c.Country == "" && c.PostalCode == "" && c.Province == "" && @@ -131,18 +131,11 @@ func GenerateCA(opts CAOpts) (string, string, error) { } } - if opts.IsEmpty() && opts.Days == 0 { - opts.Name = fmt.Sprintf("Nomad Agent CA %d", sn) - opts.Days = 1825 - opts.Country = "US" - opts.PostalCode = "94105" - opts.Province = "CA" - opts.Locality = "San Francisco" - opts.StreetAddress = "101 Second Street" - opts.Organization = "HashiCorp Inc." - opts.OrganizationalUnit = "Nomad" - } else if opts.IsEmpty() && opts.Days != 0 { + if opts.IsCustom() { opts.Name = fmt.Sprintf("Nomad Agent CA %d", sn) + if opts.Days == 0 { + opts.Days = 1825 + } opts.Country = "US" opts.PostalCode = "94105" opts.Province = "CA"