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

New sub-command: account tls #660

Merged
merged 1 commit into from
Jan 9, 2023
Merged

New sub-command: account tls #660

merged 1 commit into from
Jan 9, 2023

Conversation

philpennock
Copy link
Member

Running nats server tls will report all the valid TLS chains verified by the client for the current TLS connection. Each chain shows the PEM-encoded form of each cert in the chain, preceded by some hopefully-useful data as comments.

We'll warn, and exit with an error, if we're within 1 week of the expiration of a certificate. This is adjustable with a flag. Exiting if a cert has expired is not adjustable (but shouldn't happen with verified chains).

There's logic for OCSP verification, taken from my smtpdane tool (I'm the copyright holder of that tool so this is fine, relicensing this copied 20 line snippet as part of natscli).

Warning: I'm unable to get OCSP stapling working with a server in my setup, so I haven't been able to properly test the OCSP output mode. The tool of mine I copied it from, works fine with this.

@ripienaar
Copy link
Collaborator

nats account info already shows basic tls info - that’s where we show connection related things where server * tend to be system user stuff.

So I think either extend nats account info with a flag to show tls details else a new nats account sub command might be most in line with current flow ?

@philpennock
Copy link
Member Author

I considered that, but to me, nats account is "about the current account" and had some inline data for basic debugging. This is about the current connection, as debugging, all of which is server-specific rather than account-specific.

It feels like putting this in account is stretching into tech debt misplacement, so I opted to put it in a place which made it clearer what the context is.

What I'm not sure about is if this should be part of nats server check instead?

@philpennock
Copy link
Member Author

Running this (which suggests that --no-pem might be useful?):

nats server tls | perl -ne 'print unless /^-----BEGIN CERTIFICATE-----$/../^-----END CERTIFICATE-----$/' 

yields output:

# TLS Verified Chains count: 2

# chain: 1
# chain=1 cert=1 isCA=false Subject="CN=*.ngs.global"
#   Expiration: 2023-02-22 07:08:47 +0000 UTC
#   SAN: DNS Names: [*.aws.cloud.ngs.global *.cloud.ngs.global *.eu.geo.ngs.global *.geo.ngs.global *.ngs.global *.us.geo.ngs.global]
#   Serial: 290543087657219866918809514819475917570247
#   Signed-with: SHA256-RSA
# chain=1 cert=2 isCA=true Subject="CN=R3,O=Let's Encrypt,C=US"
#   Expiration: 2025-09-15 16:00:00 +0000 UTC
#   Serial: 192961496339968674994309121183282847578
#   Signed-with: SHA256-RSA

# chain: 2
# chain=2 cert=1 isCA=false Subject="CN=*.ngs.global"
#   Expiration: 2023-02-22 07:08:47 +0000 UTC
#   SAN: DNS Names: [*.aws.cloud.ngs.global *.cloud.ngs.global *.eu.geo.ngs.global *.geo.ngs.global *.ngs.global *.us.geo.ngs.global]
#   Serial: 290543087657219866918809514819475917570247
#   Signed-with: SHA256-RSA
# chain=2 cert=2 isCA=true Subject="CN=R3,O=Let's Encrypt,C=US"
#   Expiration: 2025-09-15 16:00:00 +0000 UTC
#   Serial: 192961496339968674994309121183282847578
#   Signed-with: SHA256-RSA
# chain=2 cert=3 isCA=true Subject="CN=ISRG Root X1,O=Internet Security Research Group,C=US"
#   Expiration: 2035-06-04 11:04:38 +0000 UTC
#   Serial: 172886928669790476064670243504169061120
#   Signed-with: SHA256-RSA

@ripienaar
Copy link
Collaborator

well, perhaps nats account info is badly named - but it IS all about the connection, its real runtime properties, tls verification details etc, all shown there already.

And accessing this data isnt system specific or admin stuff - its for everyone, every connection - so despite the name perhaps being bad, nats account info is the right place to render this in considering current existing features.

@philpennock
Copy link
Member Author

Okay, will move it. And I think I'll add --no-pem :)

@ripienaar
Copy link
Collaborator

yeah I think it would need to be opt-in like --pem to not spam all nats account info output with this

@philpennock
Copy link
Member Author

I went with your "else a new nats account sub command" because this really is quite a bit of data which isn't directly useful for the normal flow, and also went with your request to make the PEM be opt-in.

Using octothorpe # comments for all lines of output in the default case is slightly strange but I'm not sure it's worth making the logic adapt to "showing PEM or not" and I do want the comment-form for the PEM output mode.

So, nats account tls it is.

@philpennock philpennock changed the title New sub-command: server tls New sub-command: account tls Dec 26, 2022
@ripienaar
Copy link
Collaborator

Cool, will review in the new year in depth

@philpennock
Copy link
Member Author

We might also consider nats connection as a top-level command, aka nats conn, with backwards-compatibility aliases; but ... it's probably too close to consumer and context .

@ripienaar
Copy link
Collaborator

Is this output format some kind of standard?

# chain: 1
# chain=1 cert=1 isCA=false Subject="CN=demo.nats.io"
#   Expiration: 2023-02-10 00:21:46 +0000 UTC
#   SAN: DNS Names: [demo.nats.io us-dallas1.nats-demo.equinix.synadia.net]
#   Serial: 383630615706475581505430846238902344717276
#   Signed-with: SHA256-RSA
# chain=1 cert=2 isCA=true Subject="CN=R3,O=Let's Encrypt,C=US"
#   Expiration: 2025-09-15 16:00:00 +0000 UTC
#   Serial: 192961496339968674994309121183282847578
#   Signed-with: SHA256-RSA
# chain=1 cert=3 isCA=true Subject="CN=ISRG Root X1,O=Internet Security Research Group,C=US"
#   Expiration: 2035-06-04 11:04:38 +0000 UTC
#   Serial: 172886928669790476064670243504169061120
#   Signed-with: SHA256-RSA

It's not super user friendly but if its a standard format then fine, otherwise maybe more in-line with the rest of the tool design as a table or somethihng?

@philpennock
Copy link
Member Author

Is this output format some kind of standard?

Not standard, but using comments is fine in PEM certificate files, they'll be ignored. It's generally a good idea to decorate the PEM certs in a file, to explain which cert it is you're looking at. Thus the model of using this combined with PEM emission. I think it would be more annoying to switch when PEM certs are included so am inclined to leave it in this format if that's fine.

@ripienaar
Copy link
Collaborator

Ah, now the use case makes sense, ok - well then it makes sense to always emit the PEM data anyway (I mainly didnt want PEM noise along with friendly ux stuff) but here it makes sense

@philpennock
Copy link
Member Author

Okay, switched to needing --no-pem and rebased.

Copy link
Collaborator

@ripienaar ripienaar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Running `nats account tls` will report all the valid TLS chains verified
by the client for the current TLS connection.  Each chain shows the PEM-encoded
form of each cert in the chain, preceded by some hopefully-useful data as
comments.  Add `--no-pem` to skip the PEM and leave the comments.

We'll warn, and exit with an error, if we're within 1 week of the expiration of
a certificate.  This is adjustable with a flag.  Exiting if a cert has expired
is not adjustable (but shouldn't happen with verified chains).

There's logic for OCSP verification, taken from my smtpdane tool (I'm the
copyright holder of that tool so this is fine, relicensing this copied 20 line
snippet as part of natscli).
@philpennock philpennock merged commit 458dfa6 into main Jan 9, 2023
@philpennock philpennock deleted the pdp/server-tls branch January 9, 2023 22:26
@philpennock
Copy link
Member Author

Rebased/squashed and merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants