Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: fix tls ca create command with -domain #19892

Merged
merged 2 commits into from
Feb 7, 2024
Merged

cli: fix tls ca create command with -domain #19892

merged 2 commits into from
Feb 7, 2024

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Feb 6, 2024

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.

Closes #19836

@lgfa29 lgfa29 force-pushed the cli-fix-ca-create branch from bdd0214 to c89742a Compare February 7, 2024 00:31
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.
@lgfa29 lgfa29 force-pushed the cli-fix-ca-create branch from c89742a to 90f33cb Compare February 7, 2024 00:31
@jonashaag
Copy link

Does this also fix the issue that when you overwrite the organization details, the validity of the CA is set to 0 days?

@lgfa29
Copy link
Contributor Author

lgfa29 commented Feb 13, 2024

Hi @jonashaag 👋

I'm not sure, do you happen to have an issue that describes the problem?

You can also test these changes from our latest CI build (it's all the way down the page 😅):
https://github.com/hashicorp/nomad/actions/runs/7878378042

Just keep in mind that these are development builds and should be used for any production code.

@jonashaag
Copy link

Thanks, I just gave it a try. It seems to be fixed in that build. With v1.7.4:

$ nomad tls ca create -name-constraint -domain ... -additional-domain ... -country DE -common-name ... -organization ... -organizational-unit ...

$ openssl x509 -noout -text -in *ca.pem | grep Not
            Not Before: Feb 13 08:33:23 2024 GMT
            Not After : Feb 13 08:33:23 2024 GMT

@lgfa29
Copy link
Contributor Author

lgfa29 commented Feb 13, 2024

Nice, thanks for testing it. So we got a two-for-one deal on this PR 😄

The fix is available on Nomad 1.7.5 and 1.6.8 that were just released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.6.x backport to 1.6.x release line backport/1.7.x backport to 1.7.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TLS CA create command with name-constraint
3 participants