Feedback-ID: i131946ab:Fastmail
Message-ID: <949f1c20-771c-aef7-058f-2c10885db955@enterprisedb.com>
Date: Thu, 22 Sep 2022 07:52:23 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.2.2
Subject: Re: [PoC] Let libpq reject unexpected authentication requests
Content-Language: en-US
To: Jacob Champion <jchampion@timescale.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Michael Paquier <michael@paquier.xyz>,
 "David G. Johnston" <david.g.johnston@gmail.com>
References: <9e5a8ccddb8355ea9fa4b75a1e3a9edc88a70cd3.camel@vmware.com>
 <CAKFQuwabmJHf+68wo0=dd9EE=9H0RbO=S0SJ23Kpnn8peANZoA@mail.gmail.com>
 <CAAWbhmjOVfRr=Pjp3ZjcVLnHoXBXvn=hcjC2JV02cXB71uWjWg@mail.gmail.com>
 <CAAWbhmitLHUBkUUJmsG2CuVfH=OXTzj6pU1C8XYerYBM2TBfbQ@mail.gmail.com>
 <CAAWbhmi4V9zEAvfUSCDFx1pOr3ZWrV9fuxkv_2maRqvyc-m9PQ@mail.gmail.com>
 <CAAWbhmhjuHUfCs4NbB6p_MeuAgqq+et17SN=QR52HRH8zcMfFQ@mail.gmail.com>
 <77358b4c-6497-e5d3-c13c-57522654af25@enterprisedb.com>
 <CAAWbhmiKUFtkzF5_UFrJNssyTh2QEB7qF70K8zC9KWG0jMxmGw@mail.gmail.com>
 <adfab57e-bcc6-836b-0406-83bfe7f82061@enterprisedb.com>
 <CAAWbhmjMRCFjvjWu-=rmYCW4eRo8b1OjwyREm5QS-YFGGvex-w@mail.gmail.com>
 <ebf31cf7-b0af-7407-ada2-dda6d2767ba0@enterprisedb.com>
 <CAAWbhmg5ddYa=5niqvb4vY7-tpWc2Dj7e=-cNT+WZ64s8V_hVQ@mail.gmail.com>
 <CAAWbhmgOuheFzozhej0HHXJ6wi3Vu73kLrtvD6p9=mJOGOMEfw@mail.gmail.com>
 <914a0125-3dfb-920f-51b6-8d44a8edbdd2@enterprisedb.com>
 <CAAWbhmiRxVNsfaq8DLgxoAh2TBu1SthP9jycutXBz32Z2k80pw@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: 
 <CAAWbhmiRxVNsfaq8DLgxoAh2TBu1SthP9jycutXBz32Z2k80pw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/949f1c20-771c-aef7-058f-2c10885db955%40enterprisedb.com>
Precedence: bulk

On 22.09.22 01:37, Jacob Champion wrote:
> On Wed, Sep 21, 2022 at 3:36 PM Peter Eisentraut
> <peter.eisentraut@enterprisedb.com> wrote:
>> So let's look at the two TODO comments you have:
>>
>>            * TODO: how should !auth_required interact with an incomplete
>>            * SCRAM exchange?
>>
>> What specific combination of events are you thinking of here?
> 
> Let's say the client is using `require_auth=!password`. If the server
> starts a SCRAM exchange. but doesn't finish it, the connection will
> still succeed with the current implementation (because it satisfies
> the "none" case). This is also true for a client setting of
> `require_auth=scram-sha-256,none`. I think this is potentially
> dangerous, but it mirrors the current behavior of libpq and I'm not
> sure that we should change it as part of this patch.

It might be worth reviewing that behavior for other reasons, but I think 
semantics of your patch are correct.

>> In any case, it seems to me that it would be safer to *not*
>> make this assumption at first and then have someone more knowledgeable
>> make the argument that it would be safe.
> 
> I think I'm okay with that, regardless. Here's one of the wrinkles:
> right now, both of the following connstrings work:
> 
>      require_auth=gss gssencmode=require
>      require_auth=gss gssencmode=prefer
> 
> If we don't treat gssencmode as providing GSS auth, then the first
> case will always fail, because there will be no GSS authentication
> packet over an encrypted connection. Likewise, the second case will
> almost always fail, unless the server doesn't support gssencmode at
> all (so why are you using prefer?).
> 
> If you're okay with those limitations, I will rip out the code. The
> reason I'm not too worried about it is, I don't think it makes much
> sense to be strict about your authentication requirements while at the
> same time leaving the choice of transport encryption up to the server.

The way I understand what you explained here is that it would be more 
sensible to leave that code in.  I would be okay with that.