Require certificate registered within cluster before choosing CQL SSL

[zimnx]

Previously, SSL was preferred when client_encryption_options.enabled
coming from ScyllaDB configuration was true and SSL port is open,
even when Scylla Manager did not have any client certificate registered
for particular cluster.

This caused issues when ScyllaDB cluster was exposing both CQL and CQL
SSL with mTLS, because even when Manager was not registered with
certificates, it still insisted to establish sessions using SSL port.
CQL healthchecks was also affected.

It can be further improved by implementing #3679 which will add a switch deciding whether user wants to use either SSL or non SSL port.

Fixes #3698

[zimnx]

Seems like failures of CI tests, are also present on master. Is this a known issue?

[karol-kokoszka]

@zimnx restore schema in 5.4 is a known issue.
Ill review the PR tomorrow.

If it’s no restore schema related then it’s a regression

[Michal-Leszczynski]

I'm not an expert on this topic, but the following fragment of code:

 scyllaCluster.SslOpts = &gocql.SslOptions{
 	Config: &tls.Config{
 		InsecureSkipVerify: true,
 	},
 }

Makes it so that SM never validates Scylla certs. In general, is this desired or should be changed in the future? (I'm aware that it was always there)

[zimnx]

Makes it so that SM never validates Scylla certs. In general, is this desired or should be changed in the future? (I'm aware that it was always there)

It should be changed. The thing is, Manager currently lacks Trusted CA parameter during cluster registration. Without it, it's not possible to trust certificates not signed by publicly trusted CAs, which is rarely the case.

[karol-kokoszka]

@Michal-Leszczynski that's a good point. But, on the other hand, manager desire is not to save some sensitive information to Scylla cluster. It just need connection to check if the node lives. We can ignore the certs.

[karol-kokoszka]

👍

[zimnx]

It just need connection to check if the node lives.

Certificate validation is also disabled for CQL session, which is serious as Manager might connect to different cluster when there's a MITM.

[Michal-Leszczynski]

What about a situation when:

• SM doesn't have TLS certs
• ClientEncryptionEnabled = true
• ClientEncryptionRequireAuth = false

This implementation suggests to use regular CQL port, but is there anything stopping SM from using CQL SSL port without specifying certs? Wouldn't it be better/safer?

[Michal-Leszczynski]

This PR also removes the following part of CQL SSL port fallback:

// Scylla API always returns non-empty NativeTransportPortSSL even when
// value is explicitly disabled in configuration file.
// This makes impossible to determine which port is being used for CQL
// frontend. To workaround it, we try to dial SSL port when
// client encryption is enabled. If any error happens, assume this port
// is not used.
// Ref: https://github.com/scylladb/scylla/issues/7206

Which goes in pair with the following fragment of scylla.yaml:

# Enabling native transport encryption in client_encryption_options allows you to either use
# encryption for the standard port or to use a dedicated, additional port along with the unencrypted
# standard native_transport_port.
# Enabling client encryption and keeping native_transport_port_ssl disabled will use encryption
# for native_transport_port. Setting native_transport_port_ssl to a different value
# from native_transport_port will use encryption for native_transport_port_ssl while
# keeping native_transport_port unencrypted.
#native_transport_port_ssl: 9142

My understanding is that in some configurations NativeTransportPortSsl might be disabled even when ClientEncryptionEnabled is enabled (possibly with ClientEncryptionRequireAuth enabled as well).
Mentioned issue is still open, so is it safe to remove this fallback behavior?

[zimnx]

What about a situation when:

* SM doesn't have TLS certs

* ClientEncryptionEnabled = true

* ClientEncryptionRequireAuth = false

This implementation suggests to use regular CQL port, but is there anything stopping SM from using CQL SSL port without specifying certs? Wouldn't it be better/safer?

Yep, that would be better. Added support for it.

[zimnx]

This PR also removes the following part of CQL SSL port fallback:
My understanding is that in some configurations NativeTransportPortSsl might be disabled even when ClientEncryptionEnabled is enabled (possibly with ClientEncryptionRequireAuth enabled as well).
Mentioned scylladb/scylladb#7206 is still open, so is it safe to remove this fallback behavior?

This fallback is error prone and gives false negatives. Implemented probe had just 1s timeout and was sent from Manager Server which for case when given node is in different DC may return different results between runs.

Because referenced issue is not solved by Scylla, there's no reliable way to tell whether given port is open or not.
Instead on relying on unreliable probes, Manager will add switch where user will decide whether his cluster should use TLS or not. This will be solved as part of #3679.

[karol-kokoszka]

@Michal-Leszczynski there is unresolved conversation under your comment. Please let us know if it's ok, so that @zimnx can merge the PR.