Обсуждение: [oauth] Add TLS support to OAuth tests
Hi!! After some discussion about PGOAUTHCAFILE[1] variable, the need of having the TLS support for the tests added in a different patch[2]. I'm attaching a patch that add this TLS support making use of the already certs system in the `src/test/ssl/` directory and just making the `oauth_server.py` script able to support TLS only, this removes the plain HTTP support from the server. My Python skills are old, but I tried to keep the modifications as simple as possible, even that there's an easy way to have the context in Python 3.14, I decided to just have a context because the function HTTPSServer() is only available from Python 3.14 and above, which is not so widely used yet, in the future for sure will be more simple to use that function. [1] https://www.postgresql.org/message-id/flat/16a91d02795cb991963326a902afa764e4d721db.camel%40gmail.com [2] https://www.postgresql.org/message-id/flat/9850AB21-78A7-43CD-94C6-FA8E3BC9F1B3%40yesql.se#11e8d9febb3c3b428caa383f0c3f6acf -- Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Вложения
On Fri, Feb 27, 2026 at 11:17 AM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:
> I'm attaching a patch that add this TLS support making use of the
> already certs system in the `src/test/ssl/` directory and just making
> the `oauth_server.py` script able to support TLS only, this removes the
> plain HTTP support from the server.
Thank you!
> My Python skills are old, but I tried to keep the modifications as
> simple as possible, even that there's an easy way to have the context
> in Python 3.14, I decided to just have a context because the function
> HTTPSServer() is only available from Python 3.14 and above, which is
> not so widely used yet, in the future for sure will be more simple to
> use that function.
Yep, we need to support back to Python 3.6.8 (which I've tested your
patch with locally).
Comments:
> Signed-off-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com>
Note that we have no signoff convention here, at least that I'm aware
of. It's not a problem to include it, but this will be replaced by an
`Author:` line in the final commit. Submissions here fall under the
Archive Policy:
https://www.postgresql.org/about/policies/archives/
> +my $certdir = dirname(__FILE__) . "/../../../ssl/ssl";
> +$ENV{PGOAUTHCAFILE} = "$certdir/root+server_ca.crt";
I think we should inject this directory from the build system, instead
of walking across the tree.
> - my $issuer = "http://127.0.0.1:$port";
> + my $issuer = "https://localhost:$port";
Unfortunately this will cause strange bugs [1]: we aren't listening on
IPv6, but Curl will attempt to contact a DNS hostname on both IPv4 and
IPv6 simultaneously, leading to intermittent failures. So we'll need
to get the certificate's SANs working...
> +[ req ]
> +distinguished_name = req_distinguished_name
> +prompt = no
> +
> +[ req_distinguished_name ]
> +CN = localhost
> +OU = PostgreSQL test suite
> +
> +# For Subject Alternative Names
> +[ v3_req ]
> +subjectAltName = @alt_names
...which will require the v3_req section to be enabled here. (We can
move the CommonName into SANs as well, since Curl is a modern web
client.)
> -# To test against HTTP rather than HTTPS, we need to enable PGOAUTHDEBUG. But
> -# first, check to make sure the client refuses such connections by default.
> -$node->connect_fails(
I think we should keep this test; it's an important safeguard. This is
also a good chance to test the case where certificate verification
fails.
> server-ip-in-dnsname \
> + server-ip-localhost \
> server-single-alt-name \
nitpick: Though the naming scheme is not really all that well defined,
I think this probably belongs with the other -alt-name certs rather
than the -ip- certs.
--
To avoid making you play fetch-a-rock, I've attached these comments in
code form as v1.1-0002 (but you're under no obligation to take them if
you'd prefer to do it a different way :D). I also ran oauth_server.py
through Black.
Thanks!
--Jacob
[1] https://postgr.es/m/CAOYmi%2Bn4EDOOUL27_OqYT2-F2rS6S%2B3mK-ppWb2Ec92UEoUbYA%40mail.gmail.com
Вложения
Hello! > Yep, we need to support back to Python 3.6.8 (which I've tested your > patch with locally). That's indeed good to know, there's a way to know which is the oldest version of Python used? What do you think about adding this information on top of the `oauth_server.py` file? Just as a reminder because I suspected that the lower version will be 3.9 not 3.6 > Comments: > > > Signed-off-by: Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> > > Note that we have no signoff convention here, at least that I'm aware > of. It's not a problem to include it, but this will be replaced by an > `Author:` line in the final commit. Submissions here fall under the > Archive Policy: > > https://www.postgresql.org/about/policies/archives/ Yes, I'm fully aware of this, it's just my default git configuration to add the sign-off, I'll check to remove that, thanks for the point! > > > +my $certdir = dirname(__FILE__) . "/../../../ssl/ssl"; > > +$ENV{PGOAUTHCAFILE} = "$certdir/root+server_ca.crt"; > > I think we should inject this directory from the build system, > instead > of walking across the tree. I wasn't aware of this, will keep in mind for hte future > > > - my $issuer = "http://127.0.0.1:$port"; > > + my $issuer = "https://localhost:$port"; > > Unfortunately this will cause strange bugs [1]: we aren't listening > on > IPv6, but Curl will attempt to contact a DNS hostname on both IPv4 > and > IPv6 simultaneously, leading to intermittent failures. So we'll need > to get the certificate's SANs working... I wasn't aware of this, good learning! I'll keep it in mind for the future, and probably we can work on IPv6 support later. > > server-ip-in-dnsname \ > > + server-ip-localhost \ > > server-single-alt-name \ > > nitpick: Though the naming scheme is not really all that well > defined, > I think this probably belongs with the other -alt-name certs rather > than the -ip- certs. I had my questions if add it to the alt-name too, and now I'm thinking in other test ideas due to this, like using multiple alternative names for the host and issuer, but for sure, for the future not here! > -- > > To avoid making you play fetch-a-rock, I've attached these comments > in > code form as v1.1-0002 (but you're under no obligation to take them > if > you'd prefer to do it a different way :D). I also ran oauth_server.py > through Black. I did the same! just added the comments I mentioned, besides that, all good on my side. Thank you! --- Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> EnterpriseDB
Вложения
On Fri, Feb 27, 2026 at 9:09 PM Jonathan Gonzalez V.
<jonathan.abdiel@gmail.com> wrote:
> That's indeed good to know, there's a way to know which is the oldest
> version of Python used?
We have a partial list of minimum dependencies here:
https://wiki.postgresql.org/wiki/Minimum_Dependency_Versions
> What do you think about adding this information
> on top of the `oauth_server.py` file?
I think if we put it into the repo somewhere (and honestly, we
probably should), it should be centralized rather than per-file. It'll
rot really quickly otherwise.
So I've pulled in part of your -0003, done some more comment-smithing,
and squashed it all down for commit. Attached in v2.
Thanks!
--Jacob
Вложения
Hi! > > What do you think about adding this information > > on top of the `oauth_server.py` file? > > I think if we put it into the repo somewhere (and honestly, we > probably should), it should be centralized rather than per-file. > It'll > rot really quickly otherwise. Yeah, you're right, I couldn't find anything on the repo with the name of tools or dependencies required or anything like that that contain the versions, probably for another discussion indeed! > So I've pulled in part of your -0003, done some more comment- > smithing, > and squashed it all down for commit. Attached in v2. Yeah I saw! nice on putting the `cert_dir` outside in the `Makefile` and `meson.build` file! Thanks for everything! I'll wait for this to be committed to come with a new version of the other patch [1] [1] https://www.postgresql.org/message-id/flat/16a91d02795cb991963326a902afa764e4d721db.camel@gmail.com -- Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> EnterpriseDB
Вложения
On Tue, Mar 3, 2026 at 3:15 AM Jonathan Gonzalez V. <jonathan.abdiel@gmail.com> wrote: > Thanks for everything! I'll wait for this to be committed to come with > a new version of the other patch [1] Committed, thanks! --Jacob