Feature/ssl sendto

[antazoey]

• New choice TLS-TCP for --protocol option used by send-to commands:

  • code42 security-data send-to
  • code42 alerts send-to
  • code42 audit-logs send-to
    for more securely transporting data.

• --certs option for send-to commands when using --protocol TLS-TCP.

Also includes a bit of a refactor, and adds missing tests by making it super easy to test both search and send-to at the same time.

I have an unpushed branch, on the extractor project, that includes dev certs, rsyslog config updates, and Dockerfile updates that makes this easier. It is ok to push a branch with dev certs in it?

Here is how I tested (though I also made a branch on the extractor automatically move the certs into their correct locations):

Followed this guide: https://www.thegeekdiary.com/how-to-configure-rsyslog-server-to-accept-logs-via-ssl-tls/
I didn't put any data for any of the fields excepting choosing `y` for all yes/no questions

From /etc/pki/tls/certs directory:

certtool --generate-privkey --outfile ca-key.pem
certtool --generate-self-signed --load-privkey ca-key.pem --outfile ca.pem
certtool --generate-privkey --outfile rslclient-key.pem --bits 2048
certtool --generate-request --load-privkey rslclient-key.pem --outfile request.pem
certtool --generate-certificate --load-request request.pem --outfile rslclient-cert.pem --load-ca-certificate ca.pem --load-ca-privkey ca-key.pem
certtool --generate-privkey --outfile rslserver-key.pem --bits 2048
certtool --generate-request --load-privkey rslserver-key.pem --outfile request.pem
certtool --generate-certificate --load-request request.pem --outfile rslserver-cert.pem --load-ca-certificate ca.pem --load-ca-privkey ca-key.pem
cp rslserver* ../private/
cp ca.pem ../private/

service rsyslog restart

docker cp docker_centossyslog_1:/etc/pki/tls/certs .#
# Then remove or ignore irrelevant certs

[timabrmsn]

This is not what I would expect the default behavior to be. If I don't provide a cert file, it doesn't do any validation? If an end-user's server has a valid CA-signed cert, then they would expect to not need to provide a cert file, and would expect it to validate automatically.

I think this should be: If I try to connect to a server with an invalid/self-signed cert without using --cert option, it should always warn and fail. I can alternately use --cert to provide that cert if I know it's valid but just not CA-signed. And we should maybe have a way to explicitly skip validation (maybe --certs ignore?), for those cases where it's a self-signed cert on the server, but they don't have access to that cert file.

We can do this by something like:

context = ssl.create_default_context(cafile=certs, server_hostname=host)
sock = context.wrap_socket(sock)

If cafile is None, it defaults to using OS cert chains for validation.

https://docs.python.org/3/library/ssl.html#socket-creation

[antazoey]

Ah, that makes sense. I did not consider automatic cert validation.

Is an inaccessible self-signed cert the only case where skipping validation would make sense? Seems like the user should just use TCP alone then.

[antazoey]

I want to make sure of this behavior, because the ssl.wrap_context method defaults to CERT_NONE if you don't pass any certs

[antazoey]

Have you confirmed that automatic cert validation is not currently happening?

[timabrmsn]

That's probably the main case, but there could be others. Like if the server had a valid CA-signed cert, but dns lookup wasn't working on the client machine so they had to use just the host IP..

But plain TCP wouldn't work in the cases where the server is only listening for TLS connections, as the connection would get rejected server-side.

[alanag13]

@timabrmsn good find in considering certs signed by a valid CA. That wasn't something that I had considered since this functionality is typically used to point at an on-prem syslog server, but it's certainly a valid case.

[antazoey]

The reason I thought it would confusing was because of the UDP/TCP - those could also be considered insecure.

Perhaps --ignore-cert-validation is best. Explicit.

[antazoey]

Similarly to before, I can't ever successfully use certs, maybe because they are self-signed?
Other than that, I think the behavior of using the --ignore-cert-validation works now the same as it did before when excluding the --certs flag. Perhaps there are some implementation improvements yet though

[timabrmsn]

I got it to work by generating a key + cert with localhost and 127.0.0.1 as subject alternate name (SAN) params (https://medium.com/@antelle/how-to-generate-a-self-signed-ssl-certificate-for-an-ip-address-f0dd8dddf754)

Create a config file:

[req]
default_bits  = 2048
distinguished_name = req_distinguished_name
req_extensions = req_ext
x509_extensions = v3_req
prompt = no
[req_distinguished_name]
countryName = XX
stateOrProvinceName = N/A
localityName = N/A
organizationName = Self-signed certificate
commonName = 120.0.0.1: Self-signed certificate
[req_ext]
subjectAltName = @alt_names
[v3_req]
subjectAltName = @alt_names
[alt_names]
IP.1 = 127.0.0.1
DNS.1 = localhost

Generate the cert+key from the config:

openssl req -x509 -nodes -days 730 -newkey rsa:2048 -keyout key.pem -out cert.pem -config san.cnf

Now tell ncat to use that cert when listening with ssl:

ncat -kl -p 9999 --ssl --ssl-cert cert.pem --ssl-key key.pem

Running the CLI without --certs gives us the self-signed error:

code42 audit-logs send-to localhost:9999 -b 1d -p TLS-TCP 
Error: Unable to connect to localhost:9999. Failed with error: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self signed certificate (_ssl.c:1122).

Providing the same cert ncat is using works:

code42 audit-logs send-to localhost:9999 -b 1d -p TLS-TCP --certs cert.pem

[antazoey]

heck yeah, thanks for these instructions

[antazoey]

When making the latest changes, I noticed that Alerts send_to defaults to JSON while the securitydata and auditlogs default to RAW-JSON.

[alanag13]

When making the latest changes, I noticed that Alerts send_to defaults to JSON while the securitydata and auditlogs default to RAW-JSON.

should we update Alerts to use RAW-JSON by default? This wouldn't be a breaking change, and I think fields being null on alerts is pretty rare.

[timabrmsn]

It looks like we don't error if you try to use -p TCP when sending to a TLS server, the command appears to run ok, but nothing comes through. I wonder if we can catch that and raise a CLIError somehow?

[antazoey]

It looks like we don't error if you try to use -p TCP when sending to a TLS server, the command appears to run ok, but nothing comes through. I wonder if we can catch that and raise a CLIError somehow?

Huh, that is interesting point. Can servers be equipped to handle both? Will there always be an error?

Maybe there is a way to force an error

[antazoey]

It looks like we don't error if you try to use -p TCP when sending to a TLS server, the command appears to run ok, but nothing comes through. I wonder if we can catch that and raise a CLIError somehow?

It also permits UDP

[timabrmsn]

Since TLS is over TCP the initialization of the TCP connection works, but a server listening with TLS is going to respond initially with a TLS handshake message that the client would negotiate before actually sending the payload data. We would probably need to check for any response message from the server and if it's a handshake, raise an error.

UDP as a protocol is designed for firing and forgetting, not getting any acknowledgement if it was successful or not, so there's nothing we're going to be able to do there.

[timabrmsn]

Actually, scratch that. The server doesn't return a handshake response unless the client sends one. It should close the TCP connection if the data the client sends isn't a TLS handshake initiation... Poking at some things to see if there's an easy way to check/handle that.

[timabrmsn]

Ah, it actually is erroring now, we're just eating the error output in the logger here: https://github.com/code42/code42cli/blob/feature/ssl-sendto/src/code42cli/logger/__init__.py#L19

If you flip that to True and try to send -P TCP to a TLS server it spews a ton of stack traces to stderr.

I did define this to correctly raise a click exception on a network connection break: https://github.com/code42/code42cli/blob/feature/ssl-sendto/src/code42cli/logger/__init__.py#L40-L47

But it looks like I might not have ever actually put that in place on the logger itself. If we use that logic here it should work: https://github.com/code42/code42cli/blob/feature/ssl-sendto/src/code42cli/logger/handlers.py#L72

[antazoey]

To get Code42CLIError to work, we raise a custom exception here, and that gets caught in click_ext.groups.py line 61, which then gets converted into a Code42CLIError. The reason for this is because we are unable to import Code42CLIError here because of a circular dependency. Before, it was raising a ClickException, but that text is not red like it is for Code42CLIError