Added ssl support

[chandr-andr]

Hello! Many thanks for your action for PostgreSQL.

I'm working on https://github.com/qaspen-python/psqlpy and I need to turn on SSL mode in PostgreSQL for a new feature testing.
So, I've made some changes and decided to make PR here for the future fellas.

I've tested these changes here: psqlpy-python/psqlpy#60. They work fine.

[ikalnytskyi]

Hey @chandr-andr,

Thank you for the patch. Do I understand correctly that since you're working on the PostgreSQL driver, you want to make sure that the driver can properly deal with both secure and insecure connections? I mean, application developers normally just don't care.

If that's indeed the case, I don't mind coming up with a solution that can help you to achieve your goal. However, I'm a bit hesitant to accept the patch that just hardcodes the cert/pkey generation code. I can imagine that someone else would like to provide their own cert/pkey pair. So at this point, I'd consider the following:

Remove the cert/pkey generation and let the user generate the pair. I know that it'd require extra lines in your CI to prepare the pair, but I think it's a trade-off we can agree on. It just few lines of code for you.

Let the action receive the TLS pair via its input parameters. If passed, secure connections with them. If not passed, keep using insecure connections.

[chandr-andr]

Hello, @ikalnytskyi! Thanks for the feedback.
It's a good idea to allow people create their own cert/pkey pair but remove auto generation isn't very useful because at the end in most cases you need to have only root.crt (ca_file_output parameter) to make secure connection with PostgreSQL.

So, my suggestion is

1. If ssl is on and new parameters ssl_cert_file and ssl_key_file are specified, use them.
2. If only ssl is on, generate cert/pkey pair automatically.

What do you think about it?

[ikalnytskyi]

Hey @chandr-andr,

I'm sorry for being away. I have some time now to spare on open-source, so let's get to the point when we can merge this and make a release.

It's a good idea to allow people create their own cert/pkey pair but remove auto generation isn't very useful because at the end in most cases you need to have only root.crt (ca_file_output parameter) to make secure connection with PostgreSQL.

I think when it comes to most cases, people wouldn't even want encryption in tests. I'm hesitant to accept cert/pkey generation primarily due to the fact that it's going to be easy to support on all supported github action runners. I just checked the list of installed software and it seems the openssl version varies from 1.1 to 3.0, and somewhere absent. It may become quite tedious to support all this, and may not worth the complication it brings.

You may give a shot though. You can enable SSL generation for the customized build matrix, and see what fails. If it doesn't bring much complication to support all possible runners, I think we can go with the built-in cert generation, although I'd still ask you refactor the patch to return the path to generated CA as an output parameter instead of being provided by an end user.

[ikalnytskyi]

I rebased this patch and tried to run tests in here #43 I'm a bit puzzled at the moment. I'd expect tests to fail yet they've passed. I've got an impression that SSL is not enforced for some reason. I'll dig into this.

[ikalnytskyi]

@chandr-andr Thank you for your contribution! I made some changes to your patch and merged it as part of #43 (your authorship has been preserved).

There are two most notable changes compared to your original patch:

1. The path to the server certificate is returned via the certificate-path output. In your patch, it was a user who provided an output path for the certificate.
2. Both connection-uri and service-name outputs now defaults to sslmode=verify-ca and sslrootcert=path/to/server.crt in order to make sure that SSL is used and the certificate is verified. libpq (and most third party driveres) defaults to sslmode=prefer which means if SSL is not used, it can transparently fallback to unencrypted connections, and I wanted to avoid that.

I'm closing this PR now.