From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Andrew Dunstan <andrew(at)dunslane(dot)net> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Allow matching whole DN from a client certificate |
Date: | 2021-01-18 10:23:40 |
Message-ID: | 5F3535C1-41C4-4C39-B551-3720A668422A@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
I've circled back to this and tested/read it more, and I'm still of the opinion
that it's a good feature addition. A few small comments on the patch:
+ is the default, the username is matched against the certificate's
In the docs we use "user name" instead of "username" in descriptive text.
There are a couple of "username" in this added paragraph.
+ This option is probably best used in comjunction with a username map.
Typo: s/comjunction/conjunction/
+ /* use commas instead of slashes */
+ X509_NAME_print_ex(bio, x509name, 0, XN_FLAG_SEP_COMMA_PLUS);
I don't have strong opinions on whether we shold use slashes or commas, but I
think it needs to be documented that commas are required since slashes is the
more common way to print a DN. pg_stat_ssl and sslinfo are also displaying the
DN with slashes.
/* Make sure we have received a username in the certificate */
- if (port->peer_cn == NULL ||
- strlen(port->peer_cn) <= 0)
+ peer_username = port->hba->clientcertname == clientCertCN ? port->peer_cn : port->peer_dn;
Nitpickering nitpickery perhaps, but when we can inspect the DN and not just
the CN it seems a bit odd to talk about "username", which is again echoed in
the errormessage just below here. Not sure what would be better though, since
"user information" doesn't really convey enough detail/meaning.
+ /* and extract the Common Name / Distinguished Name from it. */
Not introduced in this patch, but single line comments should not be punctuated
IIRC.
The patch still applies and tests pass, I'm moving this to ready for committer.
cheers ./daniel
From | Date | Subject | |
---|---|---|---|
Next Message | David Geier | 2021-01-18 10:43:30 | search_plan_tree(): handling of non-leaf CustomScanState nodes causes segfault |
Previous Message | Greg Nancarrow | 2021-01-18 10:19:49 | Re: Parallel INSERT (INTO ... SELECT ...) |