On Mon, Apr 14, 2025 at 1:16 PM Daniel Gustafsson wrote:
> Just to close the loop, this was done yesterday as 2970c75dd982.
Thanks!
--Jacob
> On 9 Apr 2025, at 20:45, Daniel Gustafsson wrote:
>
>> On 9 Apr 2025, at 20:41, Jacob Champion
>> wrote:
>>
>> Hello,
>>
>> On Thu, Apr 3, 2025 at 8:51 AM Daniel Gustafsson wrote:
>>> Committed, after another round of testing and looking.
>>
>> I think we may want to consider marking sslk
> On 9 Apr 2025, at 20:41, Jacob Champion
> wrote:
>
> Hello,
>
> On Thu, Apr 3, 2025 at 8:51 AM Daniel Gustafsson wrote:
>> Committed, after another round of testing and looking.
>
> I think we may want to consider marking sslkeylogfile as a debug
> option (that is, opt->dispchar = "D") in f
Hello,
On Thu, Apr 3, 2025 at 8:51 AM Daniel Gustafsson wrote:
> Committed, after another round of testing and looking.
I think we may want to consider marking sslkeylogfile as a debug
option (that is, opt->dispchar = "D") in fe-connect.c. Besides being a
true "debug option", this would also pre
> On 1 Apr 2025, at 22:22, Daniel Gustafsson wrote:
>
> I took another pass at this one and did a few small tweaks, so I would to take
> it for another spin across CI before looking at committing it. There are no
> functionality changes, only polish.
Committed, after another round of testing an
I took another pass at this one and did a few small tweaks, so I would to take
it for another spin across CI before looking at committing it. There are no
functionality changes, only polish.
--
Daniel Gustafsson
v11-0001-libpq-Add-support-for-dumping-SSL-keylog-to-file.patch
Description: Binar
> On 20 Mar 2025, at 10:39, Álvaro Herrera wrote:
> In initialize_SSL(), the test for conn->sslkeylogfile is inside the
> #ifdef for the existance of the SSL function. I think it's better to
> log a message (probably just a warning) that says "this feature is not
> supported with this TLS librar
On Mon, 17 Mar 2025 at 16:48, Jacob Champion
wrote:
>
> On Sun, Mar 16, 2025 at 6:49 AM Daniel Gustafsson wrote:
> > IIRC the reasoning has been that if a rogue user can inject an environment
> > variable into your session and read your files it's probably game over
> > anyways.
>
> (Personally
On Thu, Mar 20, 2025 at 6:11 AM Jelte Fennema-Nio wrote:
> I'm not saying there's no attack possible here (although I cannot
> think of one), but we allow configuring every other SSL option using
> an env var^1. So if there is an attack possible, why would that only
> apply to being able to contro
On Thu, Mar 20, 2025 at 3:58 AM Heikki Linnakangas wrote:
>
> I'm not sure if openssl has some locking on it,
OpenSSL leaves it up to the application (us). OpenSSL 3.5 will
apparently add a builtin implementation, which from a quick skim does
use locking. As another datapoint, libcurl's implement
On 20/03/2025 11:39, Álvaro Herrera wrote:
Hello,
It seems there's rough consensus on proceeding with a connection param
and no environment variable. TBH it's not very clear to me that an
envvar is a great way to drive this, even if there weren't security
considerations at play, just considerin
Hello,
It seems there's rough consensus on proceeding with a connection param
and no environment variable. TBH it's not very clear to me that an
envvar is a great way to drive this, even if there weren't security
considerations at play, just considering the case of a multithreaded
program that op
On Tue, Mar 18, 2025 at 5:43 AM Daniel Gustafsson wrote:
> Personally I think this feature has enough value even without the env var to
> not postpone it, especially since adding an env var in 19 will still be
> backwards compatible. I would go for option 1 to stay on the safe side and
> allow ti
On Tue, 18 Mar 2025 at 13:43, Daniel Gustafsson wrote:
> Since there is disagreement over this, we should either 1) go ahead with the
> latest patch without an env var and revisit the discussion during v19; 2)
> adding the env var back into the patch as PGSSLKEYLOGFILE or; 3) postponing
> all
> o
> On 17 Mar 2025, at 16:48, Jacob Champion
> wrote:
>
> On Sun, Mar 16, 2025 at 6:49 AM Daniel Gustafsson wrote:
>> IIRC the reasoning has been that if a rogue user can inject an environment
>> variable into your session and read your files it's probably game over
>> anyways.
>
> (Personally
On Sun, Mar 16, 2025 at 6:49 AM Daniel Gustafsson wrote:
> IIRC the reasoning has been that if a rogue user can inject an environment
> variable into your session and read your files it's probably game over
> anyways.
(Personally I'm no longer as convinced by this line of argument as I
once was.
> On 14 Mar 2025, at 15:27, Peter Eisentraut wrote:
>
> On 13.03.25 19:31, Tom Lane wrote:
>> Jacob Champion writes:
>>> Adding the PG prefix to the envvar name addresses my collision
>>> concern, but I think Tom's comment upthread [1] was saying that we
>>> should not provide any envvar at all:
On Fri, 14 Mar 2025 at 03:38, Daniel Gustafsson wrote:
>
>
>
> > On 13 Mar 2025, at 19:31, Tom Lane wrote:
> >
> > Jacob Champion writes:
> >> Adding the PG prefix to the envvar name addresses my collision
> >> concern, but I think Tom's comment upthread [1] was saying that we
> >> should not pr
On 13.03.25 19:31, Tom Lane wrote:
Jacob Champion writes:
Adding the PG prefix to the envvar name addresses my collision
concern, but I think Tom's comment upthread [1] was saying that we
should not provide any envvar at all:
I think it might be safer if we only accepted it as a connection
p
> On 14 Mar 2025, at 00:02, Abhishek Chanda wrote:
>
> Thanks, Daniel.
>
> Should there be the ifdef guard in here as well?
>
> + {"sslkeylogfile", NULL, NULL, NULL,
> + "SSL-Key-Log-File", "", 0, /* sizeof("") = 0 */
> + offsetof(struct pg_conn, sslkeylogfile)},
No, we want the option to work
> On 13 Mar 2025, at 19:31, Tom Lane wrote:
>
> Jacob Champion writes:
>> Adding the PG prefix to the envvar name addresses my collision
>> concern, but I think Tom's comment upthread [1] was saying that we
>> should not provide any envvar at all:
>
>>> I think it might be safer if we only ac
Thanks, Daniel.
Should there be the ifdef guard in here as well?
+ {"sslkeylogfile", NULL, NULL, NULL,
+ "SSL-Key-Log-File", "", 0, /* sizeof("") = 0 */
+ offsetof(struct pg_conn, sslkeylogfile)},
+
A small nit, this line should say NULL
+ /* line is guaranteed by OpenSSL to be NUL terminated *
On Wed, Mar 5, 2025 at 9:21 AM Daniel Gustafsson wrote:
> I managed to misunderstand skip blocks in TAP tests in the 0002, so the
> attached version fixes that. It has been failing on Debian in CI which I have
> yet to look into.
Drive-by comment:
> +{"sslkeylogfile", "PGSSLKEYLOGFILE",
> +
Jacob Champion writes:
> Adding the PG prefix to the envvar name addresses my collision
> concern, but I think Tom's comment upthread [1] was saying that we
> should not provide any envvar at all:
>> I think it might be safer if we only accepted it as a connection
>> parameter and not via an envi
> On 3 Mar 2025, at 16:23, Daniel Gustafsson wrote:
> The attached 0002 also contains documentation touchups and comments etc. 0001
> is your patch from v6.
I managed to misunderstand skip blocks in TAP tests in the 0002, so the
attached version fixes that. It has been failing on Debian in CI
> On 28 Feb 2025, at 07:20, Abhishek Chanda wrote:
>
> Attached a v6 with O_NOFOLLOW removed, I noticed that it failed on
> windows. Please let me know if I should do this in any other way that
> is portable across platforms.
Not sure if there is a portable way to can support.
required v
Attached a v6 with O_NOFOLLOW removed, I noticed that it failed on
windows. Please let me know if I should do this in any other way that
is portable across platforms.
Thanks
On Thu, Feb 27, 2025 at 11:39 PM Abhishek Chanda
wrote:
>
> Thanks for the review, Daniel. Please find v5 of this patch at
Thanks for the review, Daniel. Please find v5 of this patch attached.
I tried to bump up the minimum OpenBSD version in installation.sgml,
do I need to add this anywhere else? Please let me know if this needs
anything else.
Thanks
On Thu, Feb 20, 2025 at 4:25 PM Daniel Gustafsson wrote:
>
> > On
> On 20 Feb 2025, at 04:36, Abhishek Chanda wrote:
> Please find attached a new version of this patch. I rebased on master,
> added better error reporting and skipped the permissions check on
> windows. Please let me know if this needs any changes. I tested this
> locally using meson running all T
Hi all,
Please find attached a new version of this patch. I rebased on master,
added better error reporting and skipped the permissions check on
windows. Please let me know if this needs any changes. I tested this
locally using meson running all TAP tests.
Thanks
On Fri, Jan 10, 2025 at 5:16 PM
Thanks, Daniel. Here is an updated patch with all previous changes,
added a simple connection test and another check to make sure file
mode is correct, and the env var fix. Please let me know if anything
needs to be changed. I tested this locally using meson running all TAP
tests, and also manually
> On 10 Jan 2025, at 04:59, Abhishek Chanda wrote:
Thanks for the new version.
+ {"sslkeylogfile", "SSLKEYLOGFILE",
The env var should be PGSSLKEYLOGFILE with the PG prefix.
> 3. Added docs and tests
There is no test in the attached patch, did you write one but forgot to git add
it befor
Thanks for the feedback, everyone. Attached a followup with the
following changes compared to the initial version:
1. Converted sslkeylogfile to a connection parameter
2. Added a mechanism to chmod the key log file to 0600
3. Added docs and tests
I tested this manually. Also ran make check and ma
On Wed, Jan 8, 2025 at 5:17 PM Tom Lane wrote:
> I think it might be safer if we only accepted it as a connection
> parameter and not via an environment variable.
Making it a connection parameter also keeps us from colliding with any
other linked libraries' use of SSLKEYLOGFILE (I'm thinking abou
> On 9 Jan 2025, at 02:17, Tom Lane wrote:
> I think it might be safer if we only accepted it as a connection
> parameter and not via an environment variable.
+1
--
Daniel Gustafsson
Abhishek Chanda writes:
> Attached is a patch to add support for logging secrets used in TLS
> connection after psql is initialized. This adds a new env var
> SSLKEYLOGFILE on the client side that points to a text file where keys
> will be logged.
I wonder if this poses a security risk, ie someth
Hello folks,
Attached is a patch to add support for logging secrets used in TLS
connection after psql is initialized. This adds a new env var
SSLKEYLOGFILE on the client side that points to a text file where keys
will be logged. If a user runs psql multiple times with the same
SSLKEYLOGFILE, new e
37 matches
Mail list logo
