Hi! Thanks for pointing this out, I've updated the patch, and made it bit differently to ensure it works regardless where the key came from.
Aki > On 21/08/2023 23:04 EEST Michal Hlavinka <mhlav...@redhat.com> wrote: > > > Hi, > > this patch does not fix the issue. It fixes that first test that failed, > but it fails a little bit further: > """ > test_read_increment .................................................. : ok > test-stream.c:241: Assert failed: is_2->stream_errno == 0 > test_write_read_v1 ................................................... : > FAILED > test-stream.c:294: Assert failed: is_2->stream_errno == 0 > test_write_read_v1_short ............................................. : > FAILED > """ > > previously compression format was forced only for hex dump, this patch > affects how some keys are created, but does not affect those that are > loaded directly for example through PEM_read_bio_PUBKEY in: > dcrypt-openssl3.c: > dcrypt_openssl_load_public_key:2460:PEM_read_bio_PUBKEY > > Cheers, > Michal Hlavinka > > On 21/08/2023 09:08, Aki Tuomi wrote: > > I think this should fix it https://cmouse.fi/ec-point.patch, can you please > > give it a try? > > > > Aki > > > >> On 21/08/2023 09:34 EEST Aki Tuomi <aki.tu...@open-xchange.com> wrote: > >> > >> > >>> On 16/08/2023 01:09 EEST Michal Hlavinka <mhlav...@redhat.com> wrote: > >>> > >>> > >>> Hi, > >>> > >>> I've started testing main git branch and found an issue with openssl 3. > >>> > >>> Test suite fails: > >>> test_load_v1_key ........................................... : ok > >>> test-crypto.c:445: Assert failed: ret == TRUE > >>> test-crypto.c:446: Assert failed: error == NULL > >>> Panic: file dcrypt-openssl3.c: line 2917 > >>> (dcrypt_openssl_public_key_type): assertion failed: (key != NULL && > >>> key->key != NULL) > >>> Error: Raw backtrace: test-crypto(+0x5af01) [0x55951b667f01] -> > >>> test-crypto(backtrace_get+0x2f) [0x55951b66877f] -> > >>> test-crypto(+0x31291) [0x55951b63e291] -> test-crypto(+0x312d5) > >>> [0x55951b63e2d5] -> test-crypto(+0x16025) [0x55951b623025] -> > >>> libdcrypt_openssl.so(+0x70ff) [0x7f5a995b60ff] -> test-crypto(+0x2a17b) > >>> [0x55951b63717b] -> test-crypto(+0x3055d) [0x55951b63d55d] -> > >>> test-crypto(test_run+0x6c) [0x55951b63d61c] -> test-crypto(main+0x4e) > >>> [0x55951b62b66e] -> libc.so.6(+0x27b4a) [0x7f5a995eeb4a] -> > >>> libc.so.6(__libc_start_main+0x8b) [0x7f5a995eec0b] -> > >>> test-crypto(_start+0x25) [0x55951b62b7e5] > >>> > >>> The issue is caused by change in openssl >= 3.0.8 > >>> > >>> in lib-dcryt/dcrypt-openssl3.c method ec_key_get_pub_point_hex(...) > >>> > >>> in old version (for openssl 1.x) it uses compressed format: > >>> > >>> EC_POINT_point2hex(g, p, POINT_CONVERSION_COMPRESSED, NULL); > >>> > >>> in openssl3 version it uses > >>> > >>> EVP_PKEY_get_octet_string_param(pkey, OSSL_PKEY_PARAM_PUB_KEY, buf, > >>> sizeof(buf), &len); > >>> > >>> but key is created in dcrypt_evp_pkey_from_point(...) > >>> with OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT, "uncompressed" > >>> > >>> this was working in openssl pre 3.0.8 as compression format was ignored > >>> when dumping data and used always compressed format. > >>> > >>> From https://www.openssl.org/docs/man3.0/man7/EVP_KEYMGMT-EC.html > >>> > >>> """ Before OpenSSL 3.0.8, the implementation of providers included with > >>> OpenSSL always opted for an encoding in compressed format, > >>> unconditionally. Since OpenSSL 3.0.8, the implementation has been > >>> changed to honor the OSSL_PKEY_PARAM_EC_POINT_CONVERSION_FORMAT > >>> parameter, if set, or to default to uncompressed format.""" > >>> > >>> I don't see simple way how to force compressed format just for dumping > >>> data without effecting the key itself. Method that can be used with > >>> compression format argument is EC_POINT_point2hex but it requires quite > >>> lengthy extraction of EC_POINT first. I've tried this for testing with: > >>> > >>> static const char *ec_key_get_pub_point_hex(const EVP_PKEY *pkey) > >>> { > >>> if (!EVP_PKEY_is_a(pkey, "EC")) > >>> return NULL; > >>> size_t len; > >>> > >>> EVP_PKEY_get_utf8_string_param(pkey, OSSL_PKEY_PARAM_GROUP_NAME, NULL, > >>> 0, &len); > >>> buffer_t *parambuf = t_buffer_create(len+1); > >>> EVP_PKEY_get_utf8_string_param(pkey, OSSL_PKEY_PARAM_GROUP_NAME, (char > >>> *)parambuf->data, len+1, &len); > >>> int nid = OBJ_txt2nid(parambuf->data); > >>> > >>> EVP_PKEY_get_octet_string_param(pkey, OSSL_PKEY_PARAM_PUB_KEY, NULL, 0, > >>> &len); > >>> parambuf = t_buffer_create(len+1); > >>> EVP_PKEY_get_octet_string_param(pkey, OSSL_PKEY_PARAM_PUB_KEY, > >>> (unsigned char *)parambuf->data, len+1, &len); > >>> > >>> EC_GROUP *ec_grp = EC_GROUP_new_by_curve_name(nid); > >>> if (ec_grp == NULL) > >>> return NULL; > >>> EC_POINT *point = EC_POINT_new(ec_grp); > >>> if (point == NULL) { > >>> EC_GROUP_free(ec_grp); > >>> return NULL; > >>> } > >>> > >>> if (!EC_POINT_oct2point(ec_grp, point, parambuf->data, len, NULL)) > >>> return NULL; > >>> char *ret = EC_POINT_point2hex(ec_grp, point, > >>> POINT_CONVERSION_COMPRESSED, NULL); > >>> EC_GROUP_free(ec_grp); > >>> EC_POINT_free(point); > >>> return ret; > >>> } > >>> > >>> and it fixed the issue, but there may be an easier way how to achieve > >>> that. > >>> > >>> Cheers > >>> Michal Hlavinka > >>> > >>> > >> > >> Thank you for reporting this, we'll take a look at it! > >> > >> Aki > > > > _______________________________________________ > dovecot mailing list -- dovecot@dovecot.org > To unsubscribe send an email to dovecot-le...@dovecot.org _______________________________________________ dovecot mailing list -- dovecot@dovecot.org To unsubscribe send an email to dovecot-le...@dovecot.org
