Some improvements

[sjaeckel]

I've reordered and refactored some of the commits of #697 in order to limit its scope and simplify the merge and a potential faster release of 2.0

With these changes merged I currently don't see any planned ABI or API breakage (besides #515, but that's an entirely different discussion).

This should be the last PR with intentional changes before a v2.0.0-rc1, c.f. #568

[karel-m]

I have just quickly changed the libtomcrypt in my CryptX perl bindings for the branch "some-improvements" from this PR.

It failed to build (which is probably expected) https://github.com/DCIT/perl-CryptX/actions/runs/18219248768/job/51875345151

I will try to adopt CryptX to the new changes (after that we will see whether CryptX test suite reveals something).

[sjaeckel]

I have just quickly changed the libtomcrypt in my CryptX perl bindings for the branch "some-improvements" from this PR.

Cool, thanks!

It failed to build (which is probably expected) https://github.com/DCIT/perl-CryptX/actions/runs/18219248768/job/51875345151

If I'm not mistaken there are two errors

[148] ff. is already caused by #524

[187] ff. is indeed caused by the API change of rsa_verify_hash_ex() in this PR

Both are expected, so thanks already in advance for fixing this in CryptX and testing those changes ☺️

[148] https://github.com/DCIT/perl-CryptX/actions/runs/18219248768/job/51875345151#step:7:149
[187] https://github.com/DCIT/perl-CryptX/actions/runs/18219248768/job/51875345151#step:7:188

[karel-m]

I have updated my perl bindings and it seems to work fine.

Just one minor suggestion: please add @param mgf_hash_idx ..... to description of rsa_sign_hash_ex + rsa_verify_hash_ex

[karel-m]

I am not sure about having: type = LTC_ECCSIG_RFC5656 (as I understand the "type" is rather a format specification not a flag defining whether to use deterministic signing) e.g. one might want LTC_ECCSIG_RFC5656 + LTC_ECCSIG_ETH27 at the same time.

[karel-m]

ad struct ltc_ecc_sig_opts could we simplify int *recid; to int recid; ?

[sjaeckel]

ad struct ltc_ecc_sig_opts could we simplify int *recid; to int recid; ?

Yeah, we could, but that would mean that it must be initialized to -1 if one is not interested in it (which is nearly always the case). With the current implementation via a pointer, it is disabled by default, so it'd be easier to use for the majority of users. But if you think it'd be better as int recid feel free to say so! I don't really have a hard opinion on that.

I am not sure about having: type = LTC_ECCSIG_RFC5656 (as I understand the "type" is rather a format specification not a flag defining whether to use deterministic signing) e.g. one might want LTC_ECCSIG_RFC5656 + LTC_ECCSIG_ETH27 at the same time.

I guess you're confusing RFC5656 and RFC6979 :)

[karel-m]

I guess you're confusing RFC5656 and RFC6979 :)

Oh, yes. Sorry for the noise.

[karel-m]

FYI current branch some-improvements seems to fail on arm and cywin (will investigate later)

[karel-m]

FYI current branch some-improvements seems to fail on arm and cywin (will investigate later)

it is a segfault (memory corruption) related to ecc_sign_hash_v2 (still not sure if in libtomcrypt or in C code of my perl bindings)

[sjaeckel]

FYI current branch some-improvements seems to fail on arm and cywin (will investigate later)

it is a segfault (memory corruption) related to ecc_sign_hash_v2 (still not sure if in libtomcrypt or in C code of my perl bindings)

hmm

$ ./test ecc
[...]
SUCCESS: passed=1 failed=0 nop=0 duration=41.5sec real=41.5sec
$ file test                                              
test: ELF 64-bit LSB executable, ARM aarch64, version 1 (GNU/Linux), statically linked, BuildID[sha1]=8aab34cfb2c11a97baefcf2787a078587a6795ea, for GNU/Linux 4.3.0, with debug_info, not stripped

At least the tests can't reproduce it for AARCH64 with Qemu ...

[karel-m]

on cywin it fails inside ecc_rfc6979_key in the first hmac_memory_multi (which is strange)

[karel-m]

The fix DCIT/perl-CryptX@cccc52b

now passes tests also on cygwin + arm

[sjaeckel]

After few hours I am still unable to switch my perl bindings to the new RSA API. I make it to compile but it fails during some operations (enc/dec, sign/verify) and it is not easy for me to debug why.

ugh, sorry! The tests seem to work fine for me.

I've just pushed my WIP state with the updated macros etc.

[sjaeckel]

but still getting errors like "Invalid PK key or key type specified for function" + on other places errors like "Invalid hash specified" etc.

It could absolutely be that the generic validity checks don't handle everything correctly, I've only tested against the built-in set of tests... 😬

[karel-m]

shouldn't the s_rsa_key_valid_pss_algs be rather s_rsa_key_valid_pss_oaep_algs and allow also padding LTC_PKCS_1_OAEP?

[karel-m]

The simple example I am not able to implement in the new RSA API (sorry for just a code fragment extracted from my perl bindings):

/* load the key in DER format */
rsa_import(data, (unsigned long)data_len, &self->key);

/* try to encrypt */
ltc_rsa_op_parameters rsa_oparams = {
  .prng = &self->pstate,
  .wprng = self->pindex,
  .padding = LTC_PKCS_1_OAEP,
  .u.crypt.lparam = lparam_ptr,
  .u.crypt.lparamlen = (unsigned long)lparam_len,
  .params.hash_alg = hash_descriptor[lparam_hash_id].name,
  .params.mgf1_hash_alg = hash_descriptor[mgf_hash_id].name,
  .params.saltlen = 0,
  .params.pss_oaep = 1,
};
rv = rsa_encrypt_key_v2(data_ptr, (unsigned long)data_len, buffer, &buffer_len, &rsa_oparams, &self->key);

[sjaeckel]

I'll have a look into it later, but please remove .params.pss_oaep = 1, and it works ... 🙈

[karel-m]

I'll have a look into it later, but please remove .params.pss_oaep = 1, and it works ...

It worked. But IMO a bit surprising to me (as the whole concept of key->params / check->params / ltc_rsa_op_parameters ...)

Anyway, still bunch of other failures (will investigate later).

[sjaeckel]

[...] a bit surprising to me (as the whole concept of key->params / check->params / ltc_rsa_op_parameters ...)

Yeah, a bit, but not really. The key->params exist because there's the possibility to restrict the PKCS#1 version via X.509 certificates. The re-use of ltc_rsa_parameters inside ltc_rsa_op_parameters, came out of laziness because it's kind of shared details (minus the pss_oaep field with isn't required there).

Regular RSA keys can be used with all PKCS#1 versions.

Restricted RSA keys ("PSS-keys") must only be used with PKCS#1 versions > 1.5, and come with a set of parameters. There's already rsa_import_x509.c::rsa_decode_parameters(), but that one isn't used yet, resp. I'll have to check whether it should be. I've other stuff to finish before continuing on this.