Content-Type: multipart/mixed; boundary="------------AFI5JTFCHdsCNef1YO0LtBJf"
Message-ID: <6aedcaa5-60f3-49af-a857-2c76ba55a1f3@iki.fi>
Date: Sat, 27 Apr 2024 01:50:54 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Direct SSL connection with ALPN and HBA rules
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: Michael Paquier <michael@paquier.xyz>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
References: <ZiH7vHwEJeRKHlST@paquier.xyz>
 <c786e226-4039-4522-ba07-92501fce9b6a@iki.fi>
 <CAOYmi+kbYPX78nu0k7OODxUhoD6H1OXdugiMK1z9Nn33G-qdcQ@mail.gmail.com>
 <a0983295-67ae-487e-84e3-6e1148af3a25@iki.fi>
 <CAOYmi+nv2wc+FzpSUsTQt=-hKgP9zmvXw2fbX57CbrL7SLYz-w@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: <CAOYmi+nv2wc+FzpSUsTQt=-hKgP9zmvXw2fbX57CbrL7SLYz-w@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/6aedcaa5-60f3-49af-a857-2c76ba55a1f3%40iki.fi>
Precedence: bulk

This is a multi-part message in MIME format.
--------------AFI5JTFCHdsCNef1YO0LtBJf
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

On 23/04/2024 20:02, Jacob Champion wrote:
> On Fri, Apr 19, 2024 at 2:43 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>> On 19/04/2024 19:48, Jacob Champion wrote:
>>> On Fri, Apr 19, 2024 at 6:56 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>>> With direct SSL negotiation, we always require ALPN.
>>>
>>>    (As an aside: I haven't gotten to test the version of the patch that
>>> made it into 17 yet, but from a quick glance it looks like we're not
>>> rejecting mismatched ALPN during the handshake as noted in [1].)
>>
>> Ah, good catch, that fell through the cracks. Agreed, the client should
>> reject a direct SSL connection if the server didn't send ALPN. I'll add
>> that to the Open Items so we don't forget again.
> 
> Yes, the client should also reject, but that's not what I'm referring
> to above. The server needs to fail the TLS handshake itself with the
> proper error code (I think it's `no_application_protocol`?); otherwise
> a client implementing a different protocol could consume the
> application-level bytes coming back from the server and act on them.
> That's the protocol confusion attack from ALPACA we're trying to
> avoid.

I finally understood what you mean. So if the client supports ALPN, but 
the list of protocols that it provides does not include 'postgresql', 
the server should reject the connection with 'no_applicaton_protocol' 
alert. Makes sense. I thought OpenSSL would do that with the alpn 
callback we have, but it does not.

The attached patch makes that change. I used the alpn_cb() function in 
openssl's own s_server program as example for that.

Unfortunately the error message you got in the client with that was 
horrible (I modified the server to not accept the 'postgresql' protocol):

psql "dbname=postgres sslmode=require host=localhost"
psql: error: connection to server at "localhost" (::1), port 5432 
failed: SSL error: SSL error code 167773280

This is similar to the case with system errors discussed at 
https://postgr.es/m/b6fb018b-f05c-4afd-abd3-318c649faf18@highgo.ca, but 
this one is equally bad on OpenSSL 1.1.1 and 3.3.0. It seems like an 
OpenSSL bug to me, because there is an error string "no application 
protocol" in the OpenSSL sources (ssl/ssl_err.c):

     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_APPLICATION_PROTOCOL),
     "no application protocol"},

and in the server log, you get that message. But the error code seen in 
the client is different. There are also messages for other alerts, for 
example:

     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_TLSV13_ALERT_MISSING_EXTENSION),
     "tlsv13 alert missing extension"},

The bottom line is that that seems like a bug of omission to me in 
OpenSSL, but I wouldn't hold my breath waiting for it to be fixed. We 
can easily check for that error code and print the right message 
ourselves however, as in the attached patch.

-- 
Heikki Linnakangas
Neon (https://neon.tech)

--------------AFI5JTFCHdsCNef1YO0LtBJf
Content-Type: text/x-patch; charset=UTF-8;
 name="0001-Reject-SSL-connection-if-ALPN-is-used-but-there-s-no.patch"
Content-Disposition: attachment;
 filename*0="0001-Reject-SSL-connection-if-ALPN-is-used-but-there-s-no.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSAxMjVlOWFkZGE2Y2RhYjY0NGI2NjA3NzJlMjljNzEzODYzZTkzY2MyIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBIZWlra2kgTGlubmFrYW5nYXMgPGhlaWtraS5saW5u
YWthbmdhc0Bpa2kuZmk+CkRhdGU6IFNhdCwgMjcgQXByIDIwMjQgMDE6NDc6NTUgKzAzMDAK
U3ViamVjdDogW1BBVENIIDEvMV0gUmVqZWN0IFNTTCBjb25uZWN0aW9uIGlmIEFMUE4gaXMg
dXNlZCBidXQgdGhlcmUncyBubwogY29tbW9uIHByb3RvY29sCgpJZiB0aGUgY2xpZW50IHN1
cHBvcnRzIEFMUE4gYnV0IHRyaWVzIHRvIHVzZSBzb21lIG90aGVyIHByb3RvY29sLCBsaWtl
CkhUVFBTLCByZWplY3QgdGhlIGNvbm5lY3Rpb24gaW4gdGhlIHNlcnZlci4gVGhhdCBpcyBz
dXJlbHkgYSBjb25mdXNpb24Kb2Ygc29tZSBzb3J0IGxpa2UgdHJ5aW5nIHRvIFBvc3RncmVT
UUwgc2VydmVyIHdpdGggYQpicm93c2VyLiBGdXJ0aGVybW9yZSwgdGhlIEFMUE4gUkZDIDcz
MDEgc2F5czoKCj4gSW4gdGhlIGV2ZW50IHRoYXQgdGhlIHNlcnZlciBzdXBwb3J0cyBubyBw
cm90b2NvbHMgdGhhdCB0aGUgY2xpZW50Cj4gYWR2ZXJ0aXNlcywgdGhlbiB0aGUgc2VydmVy
IFNIQUxMIHJlc3BvbmQgd2l0aCBhIGZhdGFsCj4gIm5vX2FwcGxpY2F0aW9uX3Byb3RvY29s
IiBhbGVydC4KClRoaXMgY29tbWl0IG1ha2VzIHRoZSBzZXJ2ZXIgZm9sbG93IHRoYXQgYWR2
aWNlLgoKSW4gdGhlIGNsaWVudCwgc3BlY2lmaWNhbGx5IGNoZWNrIGZvciB0aGUgT3BlblNT
TCBlcnJvciBjb2RlIGZvciB0aGUKIm5vX2FwcGxpY2F0aW9uX3Byb3RvY29sIiBhbGVydC4g
T3RoZXJ3aXNlIHlvdSBnb3QgYSBjcnlwdGljICJTU0wKZXJyb3I6IFNTTCBlcnJvciBjb2Rl
IDE2Nzc3MzI4MCIgZXJyb3IgaWYgeW91IHRyaWVkIHRvIGNvbm5lY3QgdG8gYQpub24tUG9z
dGdyZVNRTCBzZXJ2ZXIgdGhhdCByZWplY3RzIHRoZSBjb25uZWN0aW9uIHdpdGgKIm5vX2Fw
cGxpY2F0aW9uX3Byb3RvY29sIi4gRVJSX3JlYXNvbl9lcnJvcl9zdHJpbmcoKSByZXR1cm5z
IE5VTEwgZm9yCnRoYXQgY29kZSwgd2hpY2ggZnJhbmtseSBzZWVtcyBsaWtlIGFuIE9wZW5T
U0wgYnVnIHRvIG1lLCBidXQgd2UgY2FuCmVhc2lseSBwcmludCBhIGJldHRlciBtZXNzYWdl
IG91cnNlbHZlcy4KLS0tCiBzcmMvYmFja2VuZC9saWJwcS9iZS1zZWN1cmUtb3BlbnNzbC5j
ICAgIHwgMTAgKysrKysrKy0tLQogc3JjL2ludGVyZmFjZXMvbGlicHEvZmUtc2VjdXJlLW9w
ZW5zc2wuYyB8IDEyICsrKysrKysrKysrKwogMiBmaWxlcyBjaGFuZ2VkLCAxOSBpbnNlcnRp
b25zKCspLCAzIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2xpYnBx
L2JlLXNlY3VyZS1vcGVuc3NsLmMgYi9zcmMvYmFja2VuZC9saWJwcS9iZS1zZWN1cmUtb3Bl
bnNzbC5jCmluZGV4IGZjNDZhMzM1MzkuLjYwY2Y2OGFhYzQgMTAwNjQ0Ci0tLSBhL3NyYy9i
YWNrZW5kL2xpYnBxL2JlLXNlY3VyZS1vcGVuc3NsLmMKKysrIGIvc3JjL2JhY2tlbmQvbGli
cHEvYmUtc2VjdXJlLW9wZW5zc2wuYwpAQCAtMTMzNiwxMCArMTMzNiwxNCBAQCBhbHBuX2Ni
KFNTTCAqc3NsLAogCiAJaWYgKHJldHZhbCA9PSBPUEVOU1NMX05QTl9ORUdPVElBVEVEKQog
CQlyZXR1cm4gU1NMX1RMU0VYVF9FUlJfT0s7Ci0JZWxzZSBpZiAocmV0dmFsID09IE9QRU5T
U0xfTlBOX05PX09WRVJMQVApCi0JCXJldHVybiBTU0xfVExTRVhUX0VSUl9OT0FDSzsKIAll
bHNlCi0JCXJldHVybiBTU0xfVExTRVhUX0VSUl9OT0FDSzsKKwl7CisJCS8qCisJCSAqIFRo
ZSBjbGllbnQgZG9lc24ndCBzdXBwb3J0IG91ciBwcm90b2NvbC4gIFJlamVjdCB0aGUgY29u
bmVjdGlvbgorCQkgKiB3aXRoIFRMUyAibm9fYXBwbGljYXRpb25fcHJvdG9jb2wiIGFsZXJ0
LCBwZXIgUkZDIDczMDEuCisJCSAqLworCQlyZXR1cm4gU1NMX1RMU0VYVF9FUlJfQUxFUlRf
RkFUQUw7CisJfQogfQogCiAKZGlmZiAtLWdpdCBhL3NyYy9pbnRlcmZhY2VzL2xpYnBxL2Zl
LXNlY3VyZS1vcGVuc3NsLmMgYi9zcmMvaW50ZXJmYWNlcy9saWJwcS9mZS1zZWN1cmUtb3Bl
bnNzbC5jCmluZGV4IDBkZTIxZGM3ZTQuLmVlMWE0N2YyYjEgMTAwNjQ0Ci0tLSBhL3NyYy9p
bnRlcmZhY2VzL2xpYnBxL2ZlLXNlY3VyZS1vcGVuc3NsLmMKKysrIGIvc3JjL2ludGVyZmFj
ZXMvbGlicHEvZmUtc2VjdXJlLW9wZW5zc2wuYwpAQCAtMTc0MSw2ICsxNzQxLDE4IEBAIFNT
TGVycm1lc3NhZ2UodW5zaWduZWQgbG9uZyBlY29kZSkKIAkJcmV0dXJuIGVycmJ1ZjsKIAl9
CiAKKwlpZiAoRVJSX0dFVF9MSUIoZWNvZGUpID09IEVSUl9MSUJfU1NMICYmCisJCUVSUl9H
RVRfUkVBU09OKGVjb2RlKSA9PSBTU0xfQURfUkVBU09OX09GRlNFVCArIFNTTF9BRF9OT19B
UFBMSUNBVElPTl9QUk9UT0NPTCkKKwl7CisJCS8qCisJCSAqIFNlcnZlciBhYm9ydGVkIHRo
ZSBjb25uZWN0aW9uIHdpdGggVExTICJub19hcHBsaWNhdGlvbl9wcm90b2NvbCIKKwkJICog
YWxlcnQuICBUaGUgRVJSX3JlYXNvbl9lcnJvcl9zdHJpbmcoKSBmdW5jdGlvbiBkb2Vzbid0
IGdpdmUgYW55CisJCSAqIGVycm9yIHN0cmluZyBmb3IgdGhhdCBmb3Igc29tZSByZWFzb24s
IHNvIGRvIGl0IG91cnNlbHZlcy4KKwkJICovCisJCXNucHJpbnRmKGVycmJ1ZiwgU1NMX0VS
Ul9MRU4sIGxpYnBxX2dldHRleHQoIm5vIGFwcGxpY2F0aW9uIHByb3RvY29sIikpOworCQly
ZXR1cm4gZXJyYnVmOworCX0KKwogCS8qCiAJICogSW4gT3BlblNTTCAzLjAuMCBhbmQgbGF0
ZXIsIEVSUl9yZWFzb25fZXJyb3Jfc3RyaW5nIHJhbmRvbWx5IHJlZnVzZXMgdG8KIAkgKiBt
YXAgc3lzdGVtIGVycm5vIHZhbHVlcy4gIFdlIGNhbiBjb3ZlciB0aGF0IHNob3J0Y29taW5n
IHdpdGggdGhpcyBiaXQKLS0gCjIuMzkuMgoK

--------------AFI5JTFCHdsCNef1YO0LtBJf--