Feedback-ID: i131946ab:Fastmail
Content-Type: multipart/mixed; boundary="------------hYxhs0C21eYy0R2YgD8je2X9"
Message-ID: <49b48719-d6cf-9e03-fab3-e4505cf9d585@enterprisedb.com>
Date: Thu, 14 Jul 2022 22:12:33 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: [PATCH] Log details for client certificate failures
Content-Language: en-US
To: Jacob Champion <jchampion@timescale.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <d13c4a5787c2a3f83705124f0391e0738c796751.camel@vmware.com>
 <cd03fa63-d4d9-8d3b-f9f5-13206a28d888@enterprisedb.com>
 <a922e857c4e4a073f219bc5005b862eb22255c20.camel@vmware.com>
 <290871e5-f1d9-54a8-5df4-dbd9e604c08d@enterprisedb.com>
 <7c0468dcadbc0d0c987daf9ca08c8d111948eae6.camel@vmware.com>
 <9b8338210b865d79239ec35ca523792ce331c0a3.camel@vmware.com>
 <c60a8948-00a0-147d-75df-62699193371f@enterprisedb.com>
 <CAAWbhmggsEacYW+cahN7USucsTgK9P4c5VMLaxWRdXvGQ=+BHA@mail.gmail.com>
 <CAAWbhmiHyYajeN-SGRo_Nix+XbLtMyVkWinAoFVFy1kpbWT-zg@mail.gmail.com>
 <61aa4b6c-9de2-d61b-821a-a34ab1e515e2@enterprisedb.com>
 <CAAWbhmgtVs0VciBwL4KbhQnLfTQKqwJjUUCxzA3SFECPn=ntTA@mail.gmail.com>
 <2ef20161-e99d-a6ea-a36d-577843a2daae@enterprisedb.com>
 <CAAWbhmiQVSAc_rOdW8q9R3GJehxkmaW5-CMbYLgrCjhotSs8Bg@mail.gmail.com>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: 
 <CAAWbhmiQVSAc_rOdW8q9R3GJehxkmaW5-CMbYLgrCjhotSs8Bg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/49b48719-d6cf-9e03-fab3-e4505cf9d585%40enterprisedb.com>
Precedence: bulk

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

On 13.07.22 01:06, Jacob Champion wrote:
>> I had to read up on this "ex_data" API.  Interesting.  But I'm wondering
>> a bit about how the life cycle of these objects is managed.  What
>> happens if the allocated error string is deallocated before its
>> containing object?  Or vice versa?
> 
> Yeah, I'm currently leaning heavily on the lack of any memory context
> switches here. And I end up leaking out a pointer to the stale stack
> of be_tls_open_server(), which is gross -- it works since there are no
> other clients, but that could probably come back to bite us.
> 
> The ex_data API exposes optional callbacks for new/dup/free (I'm
> currently setting those to NULL), so we can run custom code whenever
> the SSL* is destroyed. If you'd rather the data have the same lifetime
> of the SSL* object, we can switch to malloc/strdup/free (or even
> OPENSSL_strdup() in later versions). But since we don't have any use
> for the ex_data outside of this function, maybe we should just clear
> it before we return, rather than carrying it around.

Concretely, I was thinking like the attached top-up patch.

The other way can surely be made to work somehow, but this seems much 
simpler and with fewer questions about the details.
--------------hYxhs0C21eYy0R2YgD8je2X9
Content-Type: text/plain; charset=UTF-8;
 name="v5-0002-squash-Log-details-for-client-certificate-failure.patch"
Content-Disposition: attachment;
 filename*0="v5-0002-squash-Log-details-for-client-certificate-failure.pa";
 filename*1="tch"
Content-Transfer-Encoding: base64

RnJvbSA2NjJiYzFkMTkxMDE4NjVmOTFjNmVkNGE5OGMwZjEzZjM3NTdlMWM3IE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBQZXRlciBFaXNlbnRyYXV0IDxwZXRlckBlaXNlbnRy
YXV0Lm9yZz4KRGF0ZTogVGh1LCAxNCBKdWwgMjAyMiAyMjowODo1MyArMDIwMApTdWJqZWN0
OiBbUEFUQ0ggdjUgMi8yXSBzcXVhc2ghIExvZyBkZXRhaWxzIGZvciBjbGllbnQgY2VydGlm
aWNhdGUgZmFpbHVyZXMKCi0tLQogc3JjL2JhY2tlbmQvbGlicHEvYmUtc2VjdXJlLW9wZW5z
c2wuYyB8IDI5ICsrKysrLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQogMSBmaWxlIGNoYW5nZWQs
IDUgaW5zZXJ0aW9ucygrKSwgMjQgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2Jh
Y2tlbmQvbGlicHEvYmUtc2VjdXJlLW9wZW5zc2wuYyBiL3NyYy9iYWNrZW5kL2xpYnBxL2Jl
LXNlY3VyZS1vcGVuc3NsLmMKaW5kZXggOGQ1ZWVlMGQxNi4uMjE0NzUyNGZmYyAxMDA2NDQK
LS0tIGEvc3JjL2JhY2tlbmQvbGlicHEvYmUtc2VjdXJlLW9wZW5zc2wuYworKysgYi9zcmMv
YmFja2VuZC9saWJwcS9iZS1zZWN1cmUtb3BlbnNzbC5jCkBAIC04MSwxMyArODEsNyBAQCBz
dGF0aWMgYm9vbCBzc2xfaXNfc2VydmVyX3N0YXJ0Owogc3RhdGljIGludAlzc2xfcHJvdG9j
b2xfdmVyc2lvbl90b19vcGVuc3NsKGludCB2KTsKIHN0YXRpYyBjb25zdCBjaGFyICpzc2xf
cHJvdG9jb2xfdmVyc2lvbl90b19zdHJpbmcoaW50IHYpOwogCi0vKiBIZWxwZXJzIGZvciBz
dG9yaW5nIGFwcGxpY2F0aW9uIGRhdGEgaW4gdGhlIFNTTCBoYW5kbGUgKi8KLXN0cnVjdCBw
Z19leF9kYXRhCi17Ci0JLyogdG8gYnViYmxlIGVycm9ycyBvdXQgb2YgY2FsbGJhY2tzICov
Ci0JY2hhcgkgICAqZXJyZGV0YWlsOwotfTsKLXN0YXRpYyBpbnQJc3NsX2V4X2luZGV4Owor
c3RhdGljIGNvbnN0IGNoYXIgKmNlcnRfZXJyZGV0YWlsOwogCiAvKiAtLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0gKi8KIC8q
CQkJCQkJIFB1YmxpYyBpbnRlcmZhY2UJCQkJCQkqLwpAQCAtMTEwLDcgKzEwNCw2IEBAIGJl
X3Rsc19pbml0KGJvb2wgaXNTZXJ2ZXJTdGFydCkKIAkJU1NMX2xpYnJhcnlfaW5pdCgpOwog
CQlTU0xfbG9hZF9lcnJvcl9zdHJpbmdzKCk7CiAjZW5kaWYKLQkJc3NsX2V4X2luZGV4ID0g
U1NMX2dldF9leF9uZXdfaW5kZXgoMCwgTlVMTCwgTlVMTCwgTlVMTCwgTlVMTCk7CiAJCVNT
TF9pbml0aWFsaXplZCA9IHRydWU7CiAJfQogCkBAIC00MjIsNyArNDE1LDYgQEAgYmVfdGxz
X29wZW5fc2VydmVyKFBvcnQgKnBvcnQpCiAJaW50CQkJZXJyOwogCWludAkJCXdhaXRmb3I7
CiAJdW5zaWduZWQgbG9uZyBlY29kZTsKLQlzdHJ1Y3QgcGdfZXhfZGF0YSBleGRhdGEgPSB7
MH07CiAJYm9vbAkJZ2l2ZV9wcm90b19oaW50OwogCiAJQXNzZXJ0KCFwb3J0LT5zc2wpOwpA
QCAtNDU1LDE0ICs0NDcsNiBAQCBiZV90bHNfb3Blbl9zZXJ2ZXIoUG9ydCAqcG9ydCkKIAkJ
CQkJCVNTTGVycm1lc3NhZ2UoRVJSX2dldF9lcnJvcigpKSkpKTsKIAkJcmV0dXJuIC0xOwog
CX0KLQlpZiAoIVNTTF9zZXRfZXhfZGF0YShwb3J0LT5zc2wsIHNzbF9leF9pbmRleCwgJmV4
ZGF0YSkpCi0JewotCQllcmVwb3J0KENPTU1FUlJPUiwKLQkJCQkoZXJyY29kZShFUlJDT0RF
X1BST1RPQ09MX1ZJT0xBVElPTiksCi0JCQkJIGVycm1zZygiY291bGQgbm90IGF0dGFjaCBh
cHBsaWNhdGlvbiBkYXRhIHRvIFNTTCBoYW5kbGU6ICVzIiwKLQkJCQkJCVNTTGVycm1lc3Nh
Z2UoRVJSX2dldF9lcnJvcigpKSkpKTsKLQkJcmV0dXJuIC0xOwotCX0KIAogCXBvcnQtPnNz
bF9pbl91c2UgPSB0cnVlOwogCkBAIC01NjEsNyArNTQ1LDcgQEAgYmVfdGxzX29wZW5fc2Vy
dmVyKFBvcnQgKnBvcnQpCiAJCQkJCQkoZXJyY29kZShFUlJDT0RFX1BST1RPQ09MX1ZJT0xB
VElPTiksCiAJCQkJCQkgZXJybXNnKCJjb3VsZCBub3QgYWNjZXB0IFNTTCBjb25uZWN0aW9u
OiAlcyIsCiAJCQkJCQkJCVNTTGVycm1lc3NhZ2UoZWNvZGUpKSwKLQkJCQkJCSBleGRhdGEu
ZXJyZGV0YWlsID8gZXJyZGV0YWlsX2ludGVybmFsKCIlcyIsIGV4ZGF0YS5lcnJkZXRhaWwp
IDogMCwKKwkJCQkJCSBjZXJ0X2VycmRldGFpbCA/IGVycmRldGFpbF9pbnRlcm5hbCgiJXMi
LCBjZXJ0X2VycmRldGFpbCkgOiAwLAogCQkJCQkJIGdpdmVfcHJvdG9faGludCA/CiAJCQkJ
CQkgZXJyaGludCgiVGhpcyBtYXkgaW5kaWNhdGUgdGhhdCB0aGUgY2xpZW50IGRvZXMgbm90
IHN1cHBvcnQgYW55IFNTTCBwcm90b2NvbCB2ZXJzaW9uIGJldHdlZW4gJXMgYW5kICVzLiIs
CiAJCQkJCQkJCSBzc2xfbWluX3Byb3RvY29sX3ZlcnNpb24gPwpAQCAtNTcwLDYgKzU1NCw3
IEBAIGJlX3Rsc19vcGVuX3NlcnZlcihQb3J0ICpwb3J0KQogCQkJCQkJCQkgc3NsX21heF9w
cm90b2NvbF92ZXJzaW9uID8KIAkJCQkJCQkJIHNzbF9wcm90b2NvbF92ZXJzaW9uX3RvX3N0
cmluZyhzc2xfbWF4X3Byb3RvY29sX3ZlcnNpb24pIDoKIAkJCQkJCQkJIE1BWF9PUEVOU1NM
X1RMU19WRVJTSU9OKSA6IDApKTsKKwkJCQljZXJ0X2VycmRldGFpbCA9IE5VTEw7CiAJCQkJ
YnJlYWs7CiAJCQljYXNlIFNTTF9FUlJPUl9aRVJPX1JFVFVSTjoKIAkJCQllcmVwb3J0KENP
TU1FUlJPUiwKQEAgLTExNDUsMTIgKzExMzAsMTAgQEAgdHJ1bmNhdGVfY2VydF9uYW1lKGNo
YXIgKm5hbWUpCiBzdGF0aWMgaW50CiB2ZXJpZnlfY2IoaW50IG9rLCBYNTA5X1NUT1JFX0NU
WCAqY3R4KQogewotCVNTTAkJICAgKnNzbDsKIAlpbnQJCQlkZXB0aDsKIAlpbnQJCQllcnJj
b2RlOwogCWNvbnN0IGNoYXIgKmVycnN0cmluZzsKIAlTdHJpbmdJbmZvRGF0YSBzdHI7Ci0J
c3RydWN0IHBnX2V4X2RhdGEgKmV4ZGF0YTsKIAlYNTA5CSAgICpjZXJ0OwogCiAJaWYgKG9r
KQpAQCAtMTE2MCw3ICsxMTQzLDYgQEAgdmVyaWZ5X2NiKGludCBvaywgWDUwOV9TVE9SRV9D
VFggKmN0eCkKIAl9CiAKIAkvKiBQdWxsIGFsbCB0aGUgaW5mb3JtYXRpb24gd2UgaGF2ZSBv
biB0aGUgdmVyaWZpY2F0aW9uIGZhaWx1cmUuICovCi0Jc3NsID0gWDUwOV9TVE9SRV9DVFhf
Z2V0X2V4X2RhdGEoY3R4LCBTU0xfZ2V0X2V4X2RhdGFfWDUwOV9TVE9SRV9DVFhfaWR4KCkp
OwogCWRlcHRoID0gWDUwOV9TVE9SRV9DVFhfZ2V0X2Vycm9yX2RlcHRoKGN0eCk7CiAJZXJy
Y29kZSA9IFg1MDlfU1RPUkVfQ1RYX2dldF9lcnJvcihjdHgpOwogCWVycnN0cmluZyA9IFg1
MDlfdmVyaWZ5X2NlcnRfZXJyb3Jfc3RyaW5nKGVycmNvZGUpOwpAQCAtMTIxMiw5ICsxMTk0
LDggQEAgdmVyaWZ5X2NiKGludCBvaywgWDUwOV9TVE9SRV9DVFggKmN0eCkKIAkJcGZyZWUo
c3ViamVjdCk7CiAJfQogCi0JLyogU3RvcmUgb3VyIGRldGFpbCBtZXNzYWdlIGluIFNTTCBh
cHBsaWNhdGlvbiBkYXRhLCB0byBiZSBsb2dnZWQgbGF0ZXIuICovCi0JZXhkYXRhID0gU1NM
X2dldF9leF9kYXRhKHNzbCwgc3NsX2V4X2luZGV4KTsKLQlleGRhdGEtPmVycmRldGFpbCA9
IHN0ci5kYXRhOworCS8qIFN0b3JlIG91ciBkZXRhaWwgbWVzc2FnZSB0byBiZSBsb2dnZWQg
bGF0ZXIuICovCisJY2VydF9lcnJkZXRhaWwgPSBzdHIuZGF0YTsKIAogCXJldHVybiBvazsK
IH0KLS0gCjIuMzcuMAoK

--------------hYxhs0C21eYy0R2YgD8je2X9--