On 14/05/2024 01:29, Jacob Champion wrote:
> Definitely not a major problem, but I think
> select_next_encryption_method() has gone stale, since it originally
> provided generality and lines of fallback that no longer exist. In
> other words, I think the following code is now misleading:
>
>> if (conn->sslmode[0] == 'a')
>> SELECT_NEXT_METHOD(ENC_PLAINTEXT);
>>
>> SELECT_NEXT_METHOD(ENC_NEGOTIATED_SSL);
>> SELECT_NEXT_METHOD(ENC_DIRECT_SSL);
>>
>> if (conn->sslmode[0] != 'a')
>> SELECT_NEXT_METHOD(ENC_PLAINTEXT);
>
> To me, that implies that negotiated mode takes precedence over direct,
> but the point of the patch is that it's not possible to have both. And
> if direct SSL is in use, then sslmode can't be "allow" anyway, and we
> definitely don't want ENC_PLAINTEXT.
>
> So if someone proposes a change to select_next_encryption_method(),
> you'll have to remember to stare at init_allowed_encryption_methods()
> as well, and think really hard about what's going on. And vice-versa.
> That worries me.
Ok, yeah, I can see that now. Here's a new version to address that. I
merged ENC_SSL_NEGOTIATED_SSL and ENC_SSL_DIRECT_SSL to a single method,
ENC_SSL. The places that need to distinguish between them now check
conn-sslnegotiation. That seems more clear now that there is no fallback.
--
Heikki Linnakangas
Neon (https://neon.tech)