Move OpenSSL code to newer API

[ffesti]

Avoid the now deprecated RSA and DSA data types and use the generic EVP_PKEY

Resolves: #2294

[pmatilai]

You'll need to rebase this first.

[pmatilai]

Leftovers?

[ffesti]

Yup, It's gone now.

[pmatilai]

I'm getting this on a local build (this code isn't getting built at all in the CI):

/home/pmatilai/repos/rpm/rpmio/rpmpgp_legacy/digest_openssl.c: In function ‘constructRSASigningKey’:
/home/pmatilai/repos/rpm/rpmio/rpmpgp_legacy/digest_openssl.c:223:5: error: ‘param_bld’ may be used uninitialized [-Werror=maybe-uninitialized]
  223 |     OSSL_PARAM_BLD_free(param_bld);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/pmatilai/repos/rpm/rpmio/rpmpgp_legacy/digest_openssl.c:201:21: note: ‘param_bld’ was declared here
  201 |     OSSL_PARAM_BLD *param_bld = OSSL_PARAM_BLD_new();
      |                     ^~~~~~~~~
/home/pmatilai/repos/rpm/rpmio/rpmpgp_legacy/digest_openssl.c:224:5: error: ‘params’ may be used uninitialized [-Werror=maybe-uninitialized]
  224 |     OSSL_PARAM_free(params);
      |     ^~~~~~~~~~~~~~~~~~~~~~~
/home/pmatilai/repos/rpm/rpmio/rpmpgp_legacy/digest_openssl.c:208:17: note: ‘params’ was declared here
  208 |     OSSL_PARAM *params = OSSL_PARAM_BLD_to_param(param_bld);
      |                 ^~~~~~
cc1: all warnings being treated as errors

[pmatilai]

Doesn't this bump the required OpenSSL version to something newer than 1.0.2 which is the oldest currently supported version?

[ffesti]

OK, turns out this is code based on OpenSSL 3.0 which is from 2021. So it is a bit new. Otoh it no longer is the default variant to be built and the next release shouldn't be backported to some ancient enterprise distribution.

[pmatilai]

Ack, thought so. I don't see the version requirement as a problem (being non-default etc), just that the docs + build require needs updating, which is done now 👍

This looks fine to me but then I haven't got the slightest about the openssl API, would be nice to have someone more familiar with stuff have a look. @sgallagher , @DemiMarie , @nwalfield - can you have a look / know someone who could?

[nwalfield]

@pmatilai: I'm not an expert on OpenSSL. We were recently contacted by the RedHat Crypto Team (cc: @simo5, @sahanaprasad07) about a similar change, and they offered to help with the porting and review. I suspect they'll be willing to take a look at this, too.

[ffesti]

That would be great! It's not that I have a clue about OpenSSL either. I just banged it with a hammer until it seemed to work.

[pmatilai]

Okay, this has hung around long enough now.

[mlschroe]

I think you broke DSA signatures: it calls EVP_PKEY_verify with padded_sig which is constructed from just sig->r. But constructDSASignature (called at the beginning) takes sig->r and sig->s and creates a DSA_SIG from it.

I'm pretty sure PKEY_verify to be passed something DER encoded instead...

We need a testcase for DSA signatures...

[simo5]

@mlschroe there are still DSA keys in use somewhere?

[nwalfield]

@mlschroe there are still DSA keys in use somewhere?

FESCO decided decided that Fedora 38 would continue to accept DSA keys, and that Fedora 39 should disable them. Looking at the fedora crypto policies repository, however, they appear to still be allowed in the default policy.

[pmatilai]

There may not be DSA keys in active use but they do exist in old distros and packages people may want to install for whatever reason. If we broke it we should fix it.

[simo5]

I would think people can just install those w/o checking the signatures ... but I am not advocating against fixes

[mlschroe]

AFAICT the code in question was never released, so there's nothing to fix on your side. (I already fixed it in the "legacy" parser repo)

[pmatilai]

Seems I've managed to throroughly confuse myself with the recent split 😂

So yup, we still need to support the internal parser in 4.19.x but this change is not there, and while we still have openssl-related code in >= 4.20, DSA is not part of it.