Add ability to export ecc keys in short/oid form.

[manuelm]

As already discussed in #13 this adds the new PK_CURVEOID flag + integration into perl-CryptX. I've split the commits for libtomcrypt so you can better merge them to upstream.

Regarding commit cb7369a6d2ac315b8c1d9c7be379f8c251351710: I'm not sure if we really should allow setting the OID as importing a custom OID again definitely won't work. If we abandon this commit, we should also abandon the last one.

[karel-m]

As for accepting custom oid value defined by user together with other custom curve params I am not sure. Do you need this feature for your use case?

On the other hand having curve_oid in key2hash output is IMO fine even if we will not support custom oid value as a part of custom curve parameters. It might also be a good idea to add oid to curve2hash output.

[karel-m]

To make it clear I am not afraid of inability to import short DER form with unknown oid (it is already a trouble nowadays). I am afraid that we will allow user to export the key (and the export operation will be completely ok, without errors) in a format that he has no chance to read back.

So we can basically import/generate whatever but exporting *_short should IMO export only well-known OIDs (in our case those that are hardcoded in libtomcrypt/CryptX).

[manuelm]

No I don't need this feature. Allowing this was a side-effect of passing the OIDs from the perl bindings to ltc so ltc doesn't need to do the lookup on exporting. But I don't really care about this micro optimization.

So I'm in favor of removing that altogether. However this also means curve_oid in key2hash will be gone (edit: or do the OID lookup in key2hash too)

Edit2: To make that edit above clearer. I mean adding something like: int ecc_dp_oid_lookup(ltc_ecc_set_type *dp) to ltc which includes the for-loop from ecc_export_full + stores the result in the dp struct. We can then do the lookup from key2hash/curve2hash as well.

[karel-m]

Ad not short-exporting "unknown" OIDs

The thing which we probably both agree with is simply removing this_

      if (key->dp->oid.OIDlen > 0) {
          oid = &key->dp->oid;
      }

from ecc_export_full so that we will only export *_short curves that exist in ltc_ecc_sets[].

Ad ecc_dp_oid_lookup

The idea of having int ecc_dp_oid_lookup(ltc_ecc_set_type *dp) is interesting.

However I am not sure whether storing result to key->dp->oid is a good idea. I think we'd better avoid modifications of key->dp except for generate_key and import_* functions.

Ad user-defined OID value

The function ecc_dp_set was meant to accept curve parameters as mostly chars, so perhaps it might be better to change it from:

int ecc_dp_set(ltc_ecc_set_type *dp, ... char *ch_name, oid_st oid)

to

int ecc_dp_set(ltc_ecc_set_type *dp, ... char *ch_name, char* oid)

and move the loop converting ASCII OID to oid_st (section /* parse optional oid param */) from _ecc_set_dp_from_SV to ecc_dp_set

I am still a bit reluctant to allow users to pass their own oid values to generate_key or import_* functions. So I incline to having this functionality in ecc_dp_set but for now not to use it from _ecc_set_dp_from_SV.

[manuelm]

I think dp->oid should rather be cache to avoid multiple lookups. A c-user of libtomcrypt can of course always modify the underlying structure (unless we make it opaque) but that's out of scope. I'll see what I can come up with.

btw why is OID unsigned long instead of uint32_t?

[karel-m]

I am not a fan of uint32_t as it is not supported by Microsoft Visual Studio 2005 (and earlier).

As for dp->oid I leave it up to you, I only care about not short-exporting "unknown" OIDs. Not having oid in key2hash is absolutely fine. I appreciate your effort you put into this PR and I do not want to push you to implement something that does not have a use case.

[manuelm]

Please take a look at https://github.com/manuelm/perl-CryptX/commit/6d3839278aceda3ca8d276a90ddaca09e4949719

The most important change is I'm now looking up the curves OID + name after calling ecc_dp_set_bn (used by ecc_import and co.) or ecc_dp_set.

For the latter I've converted oid to char * and made name + oid optional. These optional arguments are set to NULL in the perl bindings, so no user can set a custom OID.

This also brings some consistency into the three ways key->dp can be defined (ecc_dp_set_bn, ecc_dp_set_by_oid and ecc_dp_set). After calling any of these functions, if dp->oid.OIDlen = 0 or dp->name is 'custom', the curve really is custom/not known.

[manuelm]

PR now contains the latest changes. Hope you like the 2nd attempt. I certainly do :-)

It might also be a good idea to add oid to curve2hash output.

Doesn't make much sense now as generate_key is intentionally ignoring the OID.

[karel-m]

Looks good. Merged to master.

There are two extra commits in your curve_oid_export that should probably be part of this PR or not?

[manuelm]

Which ones? git diff curve_oid_export gives me nothing. curve_oid_export was my working branch for this PR.

[karel-m]

I mean this one https://github.com/manuelm/perl-CryptX/commit/6d3839278aceda3ca8d276a90ddaca09e4949719

[manuelm]

These changes are included and split across all commits in this PR. I did a full rebase.

For third-party projects I usually create a working branch, where I often push changes. Sometimes with TODO markers and so on. For the PR I create separate, self-contained commits so the projects commit log isn't cluttered.

[karel-m]

OK, so that now it is merged to master.

I'll do some testing before making CPAN release.

Anyway thanks for your time spent on this PR

[manuelm]

Sure. Thanks for providing the bindings. They are by far the best crypto bindings for perl.