Skip to content

Commit

Permalink
cli: fix tls ca create command with -domain (#19892)
Browse files Browse the repository at this point in the history
The current implementation of the `nomad tls ca create` command
ovierrides the value of the `-domain` flag with `"nomad"` if no
additional customization is provided.

This results in a certificate for the wrong domain or an error if the
`-name-constraint` flag is also used.

THe logic for `IsCustom()` also seemed reversed. If all custom fields
are empty then the certificate is _not_ customized, so `IsCustom()`
should return false.
  • Loading branch information
lgfa29 authored Feb 7, 2024
1 parent 15b06e8 commit ce710d4
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 37 deletions.
3 changes: 3 additions & 0 deletions .changelog/19892.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: Fixed a bug where the `nomad tls ca create` command failed when the `-domain` was used without other values
```
25 changes: 11 additions & 14 deletions command/tls_ca_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ 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", 0, "")
flagSet.IntVar(&c.days, "days", 1825, "")
flagSet.BoolVar(&c.constraint, "name-constraint", false, "")
flagSet.StringVar(&c.domain, "domain", "", "")
flagSet.StringVar(&c.domain, "domain", "nomad", "")
flagSet.StringVar(&c.commonName, "common-name", "", "")
flagSet.StringVar(&c.country, "country", "", "")
flagSet.StringVar(&c.postalCode, "postal-code", "", "")
Expand All @@ -169,9 +169,7 @@ func (c *TLSCACreateCommand) Run(args []string) int {
c.Ui.Error(commandErrorText(c))
return 1
}
if c.IsCustom() && c.days != 0 || c.IsCustom() {
c.domain = "nomad"
} else {
if c.IsCustom() {
if c.commonName == "" {
c.Ui.Error("Please provide the -common-name flag when customizing the CA")
c.Ui.Error(commandErrorText(c))
Expand Down Expand Up @@ -261,13 +259,12 @@ func (c *TLSCACreateCommand) Run(args []string) int {
// IsCustom checks whether any of TLSCACreateCommand parameters have been populated with
// non-default values.
func (c *TLSCACreateCommand) IsCustom() bool {
return c.commonName == "" &&
c.country == "" &&
c.postalCode == "" &&
c.province == "" &&
c.locality == "" &&
c.streetAddress == "" &&
c.organization == "" &&
c.organizationalUnit == ""

return c.commonName != "" ||
c.country != "" ||
c.postalCode != "" ||
c.province != "" ||
c.locality != "" ||
c.streetAddress != "" ||
c.organization != "" ||
c.organizationalUnit != ""
}
11 changes: 11 additions & 0 deletions command/tls_ca_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,17 @@ func TestCACreateCommand(t *testing.T) {
require.Len(t, cert.PermittedDNSDomains, 0)
},
},
{"ca custom domain",
[]string{
"-name-constraint=true",
"-domain=foo.com",
},
"foo.com-agent-ca.pem",
"foo.com-agent-ca-key.pem",
func(t *testing.T, cert *x509.Certificate) {
require.ElementsMatch(t, cert.PermittedDNSDomains, []string{"nomad", "foo.com", "localhost"})
},
},
{"ca options",
[]string{
"-days=365",
Expand Down
47 changes: 24 additions & 23 deletions helper/tlsutil/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,17 @@ type CertOpts struct {
ExtKeyUsage []x509.ExtKeyUsage
}

// IsNotCustom 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) IsNotCustom() bool {
return c.Country == "" &&
c.PostalCode == "" &&
c.Province == "" &&
c.Locality == "" &&
c.StreetAddress == "" &&
c.Organization == "" &&
c.OrganizationalUnit == "" &&
c.Name == ""
func (c *CAOpts) IsCustom() bool {
return 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)
Expand Down Expand Up @@ -131,19 +131,11 @@ func GenerateCA(opts CAOpts) (string, string, error) {
}
}

if opts.IsNotCustom() {
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"
opts.Locality = "San Francisco"
opts.StreetAddress = "101 Second Street"
opts.Organization = "HashiCorp Inc."
opts.OrganizationalUnit = "Nomad"
} else {
if opts.Days == 0 {
opts.Days = 1825
}

if opts.IsCustom() {
if opts.Name == "" {
return "", "", errors.New("common name value not provided")
} else {
Expand All @@ -160,6 +152,15 @@ func GenerateCA(opts CAOpts) (string, string, error) {
if opts.OrganizationalUnit == "" {
return "", "", errors.New("organizational unit value not provided")
}
} else {
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"
}

// Create the CA cert
Expand Down

0 comments on commit ce710d4

Please sign in to comment.