#181 support TLS Insecure connection - added flag --tlsinsecure #1098
Conversation
mschneider82
force-pushed
the
feat-tlsinsecure
branch
from
e8d44f4
to
9c6e160
Compare
| @@ -307,6 +308,18 @@ func natsOpts() []nats.Option { | |||
| connectionName = "NATS CLI Version " + Version | |||
| } | |||
|
|
|||
| if opts().TlsInsecure { | |||
There was a problem hiding this comment.
I believe custom tlsconfigs has to be set using nats.Secure().
Again, this works today only due to chance and due to flow of nats.go which can change at any time. This feature has to go into nats.go please engage there and add an option so it's done right and we know it will work and be maintained in the long term.
There was a problem hiding this comment.
What makes me a little confusion is the Description of nats.Secure() :
// Secure is an Option to enable TLS secure connections that skip server verification by default.
// Pass a TLS Configuration for proper TLS.
// A TLS Configuration using InsecureSkipVerify should NOT be used in a production setting.
In my first PR i tried to use nats.Secure() to enable InsecureSkipVerify, this only works if i supply a tls.Config{} struct. So currently nats.Secure() is broken because it doesnt handle the mentioned Auth things?
There was a problem hiding this comment.
Secure() without a tlsc sets the Secure option true
https://github.com/nats-io/nats.go/blob/c693ec3784c00cf5f9ad9c0f4010b352482ab86d/nats.go#L880
Later this is called that will create a tlsc and set all the things so just calling Secure() would be fine
https://github.com/nats-io/nats.go/blob/c693ec3784c00cf5f9ad9c0f4010b352482ab86d/nats.go#L2088
if you do want to set a tls at least also set MinVersion: tls.VersionTLS12 here
There was a problem hiding this comment.
But all that aside, I am still not happy supporting this option.
Why is your TLS setup broken that you need this?
There was a problem hiding this comment.
This is for development env, I embedded nats js server and didnt want to use plain connection, plain is way more insecure than using a self signed tls.
I have tested just calling nats.Secure() without an tls.Config and the tls failure still exists.
There was a problem hiding this comment.
@mschneider82: I cloned the repository, applied the changes and installed the adjusted CLI. Finally I can just connect to all our test environments and see what is going on with my JetStream subscriptions -- without having to set up each of them with the right certificates locally.
This is worth gold! 🥇
Thank you!
There was a problem hiding this comment.
Yes the patched Version, especially with the env NATS_TLSINSECURE set is total fine. It could be so simple to implement...
There was a problem hiding this comment.
Sometimes its not if you can but if you should.
The need here, as stated, is laziness and not technical need .. "connect to all our test environments...without having to set up each of them with the right certificates locally." security is often inconvenient but enabling it to be bypassed is often ill advised.
I guess you love your rsh with zero password prompts to root user with no encryption or security - because it's convenient and you just don't like the effort of setting up a correct system?
Yeah, we don't do that, we set up TLS correctly.
There was a problem hiding this comment.
We already support bypassing TLS - just don't set it up since disabling key aspects of it has the same outcome.
There was a problem hiding this comment.
We already support bypassing TLS - just don't set it up since disabling key aspects of it has the same outcome.
Can you elaborate?
But note, that I'm not interested in bypassing TLS. I'm just interested in disabling the server certificate verification in natsCLI. The communication is still TLS encrypted.
Exactly what this PR does: it doesn't disable TLS. Just allow connection to a server with a certificate it can't verify against a CA.
|
This feature is very helpful and even our production use insecure certificate. I am looking forward it in the next release. |
|
If setting up TLS correctly to get the benefits of TLS including man in tbe middle attack prevention is too much effort. Then don’t set up TLS and make it clear to everyone you don’t care for those features. Selectively disabling things like peer verification means you may as well not set up TLS. |
@ripienaar How is setting setting |
|
It’s much more than a debugging tool. It’s capable of creating jetstream assets, removing them, destroying data, creating accounts and users and severely interrupting even destroying jetstream clusters. It’s a full admin tool and equivalent to root level access for the nats server. |
This doesn't answer the question. Note, that I never said the NATS cli is merely a debug tool, but it is in the use case I need Overall I see a lot of made up reasons to dismiss this MR. A lot of aggression and assumptions, too. |
This PR adds a new flag for
--tlsinsecureor envNATS_TLSINSECURESince the server side supports it, the cli should also support it.
fixes #181
tested in a pod: