Created attachment 210201 [details] tls.c::sm_RSA_generate_key() patch, based on revision 463590 of patch-tls.c https://svnweb.freebsd.org/base?view=revision&revision=339260 Revision 339260 update made sendmail compatibible with OpenSSL 1.1.1. The change has been based on patch-tls.c taken from mail/sendmail port. It introduces sm_RSA_generate_key() function used replacement of original RSA_generate_key/RSA_generate_key_ex function. It's code is broken badly. It may use NULL pointer and it returns NULL all the times. Code fragment of sm_RSA_generate_key() starting with: ----- bn_rsa_r4 = BN_new(); rc = BN_set_word(bn_rsa_r4, RSA_F4); if ((bn_rsa_r4 != NULL) && BN_set_word(bn_rsa_r4, RSA_F4) && (rsa = RSA_new()) != NULL) ----- ... is broken badly. It calls BN_set_word(bn_rsa_r4, RSA_F4) twice, first time before the bn_rsa_r4 has been verified non-NULL causing possible NULL dereferrence. The "if" body returns NULL if call to RSA_generate_key_ex() has succeeded. It's main issue causing the sm_RSA_generate_key() is returning NULL all the times. But it's not the only issue - code here is just wrong. It doesn't free bn_rsa_r4 most of time causing memory leak. It doesn't honor function parameters using hardcoded constants instead. The bug has been introduced in revision 463887 of the patch-tls.c port's patch: https://svnweb.freebsd.org/ports?view=revision&revision=463887 The patch I'm proposing here is based on correct version of port's patch-tls.c (e.g. 463887): https://svnweb.freebsd.org/ports?view=revision&revision=463590
Created attachment 210203 [details] tls.c::sm_RSA_generate_key() patch, simpler version Further analysis has discovered all bugs mentioned are caused by two forgotten/excessive lines. The duplicate call of BN_set_word (rc = BN_set_word(bn_rsa_r4, RSA_F4)) and the excessive "return NULL" that should not be here at all. This version of patch proposes much smaller change than the previous one.
Take.
Sorry for the delay. Staged at https://reviews.freebsd.org/D23734
A commit references this bug: Author: hrs Date: Thu Feb 27 19:40:29 UTC 2020 New revision: 358404 URL: https://svnweb.freebsd.org/changeset/base/358404 Log: Fix broken STARTTLS when SharedMemoryKey is enabled. OpenSSL 1.1 API patch for sendmail had a bug which prevented sm_RSA_generate_key() function from working. This function is used to generate a temporary RSA key for a shared memory region used for TLS processing. Note that 12.0 and 12.1-RELEASE include this bug. This affects only if SM_CONF_SHM compile-time option (enabled by default) and SharedMemoryKey run-time option (not enabled by default) in a .cf file are specified. The latter corresponds to confSHARED_MEMORY_KEY in a .mc file. PR: 242861 MFC after: 3 days Differential Revision: https://reviews.freebsd.org/D23734 Changes: head/contrib/sendmail/src/tls.c
A commit references this bug: Author: hrs Date: Wed Mar 18 18:21:59 UTC 2020 New revision: 359081 URL: https://svnweb.freebsd.org/changeset/base/359081 Log: MFC of r358404, r358410, r358412, and r358413: Fix broken STARTTLS when SharedMemoryKey is enabled. OpenSSL 1.1 API patch for sendmail had a bug which prevented sm_RSA_generate_key() function from working. This function is used to generate a temporary RSA key for a shared memory region used for TLS processing. Note that 12.0 and 12.1-RELEASE include this bug. This affects only if SM_CONF_SHM compile-time option (enabled by default) and SharedMemoryKey run-time option (not enabled by default) in a .cf file are specified. The latter corresponds to confSHARED_MEMORY_KEY in a .mc file. Fix style inconsistencies. Do not free p and g parameters after calling DH_set0_pqg(3). PR: 242861 Differential Revision: https://reviews.freebsd.org/D23734 Changes: _U stable/12/ stable/12/contrib/sendmail/src/tls.c