Re-work OID handling #362

Closed
opened 2018-03-23 06:24:02 -05:00 by sjaeckel · 7 comments
sjaeckel commented 2018-03-23 06:24:02 -05:00 (Migrated from github.com)

Follow-up of OID handling of #309

Follow-up of OID handling of #309
karel-m commented 2018-03-23 08:53:19 -05:00 (Migrated from github.com)

FYI: During my work on a support for encrypted pkcs8 ecc keys the OID stuff became even more complicated see https://github.com/libtom/libtomcrypt/blob/pr/ecc-asn1-part/src/pk/ecc/ecc_import_pkcs8.c

FYI: During my work on a support for encrypted pkcs8 ecc keys the OID stuff became even more complicated see https://github.com/libtom/libtomcrypt/blob/pr/ecc-asn1-part/src/pk/ecc/ecc_import_pkcs8.c
sjaeckel commented 2018-05-10 06:20:26 -05:00 (Migrated from github.com)

I know that the current way the API is built up works pretty well, but there is one basic thing I don't like and that's the usage of unsigned long to represent an OID arc which would basically result in different en-/decoding capabilities of the implementation depending on the architecture if the code wouldn't trim it all down to 32bits...
This leads to the following questions:

Should we keep the basic type of the OID array as unsigned long or change it to ulong32 or ulong64 (and kill the trimming)?

Or do we even want 100% ASN.1 standard compatibility? Then we'd have to work with mp_int to represent an arc... where one implementation would then be to only work with strings/const char* on the API and use mp_int on the insides... or do we really have to convert OID's into their numerical representation? Why can't we even work with strings on the inside and do the conversion when doing the ASN.1 en/decoding on the fly via the math provider?

When reading http://oid-info.com/faq.htm#size-limitations I'd say it'd be sufficient to change the basic type to ulong64 and optionally provide something like der_encode_object_identifier_64() etc.
The biggest problem then being the break of user-code when processing flexi-decoded OID's ...

[Edit]: I've just received an answer from the oid-info.com admins:

The longest OID described in the OID repository is 34-arcs long but it is not clear whether it is a fake as it is strangely constructed (see https://docs.bmc.com/docs/display/Configipedia/List+of+discoverable+printers).

The highest arcs (in term of integer values) are Universally Unique IDentifiers (UUIDs) used as subsequent arcs of OID {joint-iso-itu-t(2) uuid(25)}.

I'll just leave this here...

I know that the current way the API is built up works pretty well, but there is one basic thing I don't like and that's the usage of `unsigned long` to represent an OID arc which would basically result in different en-/decoding capabilities of the implementation depending on the architecture if the code wouldn't trim it all down to 32bits... This leads to the following questions: Should we keep the basic type of the OID array as `unsigned long` or change it to `ulong32` or `ulong64` (and kill the trimming)? Or do we even want 100% ASN.1 standard compatibility? Then we'd have to work with `mp_int` to represent an arc... where one implementation would then be to only work with strings/`const char*` on the API and use `mp_int` on the insides... or do we really have to convert OID's into their numerical representation? Why can't we even work with strings on the inside and do the conversion when doing the ASN.1 en/decoding on the fly via the math provider? When reading http://oid-info.com/faq.htm#size-limitations I'd say it'd be sufficient to change the basic type to `ulong64` and optionally provide something like `der_encode_object_identifier_64()` etc. The biggest problem then being the break of user-code when processing flexi-decoded OID's ... [Edit]: I've just received an answer from the oid-info.com admins: > The longest OID described in the OID repository is 34-arcs long but it is not clear whether it is a fake as it is strangely constructed (see https://docs.bmc.com/docs/display/Configipedia/List+of+discoverable+printers). > > The highest arcs (in term of integer values) are Universally Unique IDentifiers (UUIDs) used as subsequent arcs of OID <a href="http://oid-info.com/get/2.25"><code>{joint-iso-itu-t(2) uuid(25)}</code></a>. I'll just leave this here...
karel-m commented 2018-05-22 15:39:54 -05:00 (Migrated from github.com)

The more I fiddle with OID stuff the more I think that our API (public functions, public structs) shoudl use string representation e.g. "1.2345.67.8.9.10". I see the API breakage of flexi-decoded OID's but I still consider using strings more appropriate.

The more I fiddle with OID stuff the more I think that our API (public functions, public structs) shoudl use string representation e.g. `"1.2345.67.8.9.10"`. I see the API breakage of flexi-decoded OID's but I still consider using strings more appropriate.
sjaeckel commented 2018-06-02 06:22:39 -05:00 (Migrated from github.com)

Perfect, then I'll work in that direction!

Regarding the API breakage we could leave the old enum element LTC_ASN1_OBJECT_IDENTIFIER in there but don't use it anymore on output, but still process it on input? And then simply add a new LTC_ASN1_OID_STRING?
Or we replace the first by the latter?
It'd come together with a major ABI bump, so I tend to replace, but don't use the old name.

Perfect, then I'll work in that direction! Regarding the API breakage we could leave the old enum element `LTC_ASN1_OBJECT_IDENTIFIER` in there but don't use it anymore on output, but still process it on input? And then simply add a new `LTC_ASN1_OID_STRING`? Or we replace the first by the latter? It'd come together with a major ABI bump, so I tend to replace, but don't use the old name.
karel-m commented 2018-06-03 11:41:26 -05:00 (Migrated from github.com)

I would:

  • keep LTC_ASN1_OBJECT_IDENTIFIER (we use it quite a lot even in libtomcrypt itself)
  • introduce LTC_ASN1_OID_STRING
  • switch flexi-decode implementation to LTC_ASN1_OID_STRING
I would: * keep `LTC_ASN1_OBJECT_IDENTIFIER` (we use it quite a lot even in libtomcrypt itself) * introduce `LTC_ASN1_OID_STRING` * switch flexi-decode implementation to `LTC_ASN1_OID_STRING`
sjaeckel commented 2018-07-10 02:03:47 -05:00 (Migrated from github.com)

I would:

  • keep LTC_ASN1_OBJECT_IDENTIFIER (we use it quite a lot even in libtomcrypt itself)
  • introduce LTC_ASN1_OID_STRING
  • switch flexi-decode implementation to LTC_ASN1_OID_STRING

sounds good!

I'm thinking about defaulting to only decode to LTC_ASN1_OID_STRING and if s/o requests a LTC_ASN1_OBJECT_IDENTIFIER to convert.
The advantage would be:

  • we are relying on one implementation of the decoding
  • that implementation/conversion will then finally detect&report potential overflows of subidentifiers

I've also pushed a starting point to the branch rework/oid that removes oid_st. Should I already open a PR for that?

> I would: > * keep `LTC_ASN1_OBJECT_IDENTIFIER` (we use it quite a lot even in libtomcrypt itself) > * introduce `LTC_ASN1_OID_STRING` > * switch flexi-decode implementation to `LTC_ASN1_OID_STRING` sounds good! I'm thinking about defaulting to only decode to `LTC_ASN1_OID_STRING` and if s/o requests a `LTC_ASN1_OBJECT_IDENTIFIER` to convert. The advantage would be: * we are relying on one implementation of the decoding * that implementation/conversion will then finally detect&report potential overflows of subidentifiers I've also pushed a starting point to the branch [rework/oid](https://github.com/libtom/libtomcrypt/tree/rework/oid) that removes `oid_st`. Should I already open a PR for that?
karel-m commented 2018-07-10 02:45:42 -05:00 (Migrated from github.com)

Yes, I am for opening a new PR so that we see what Travis-CI thinks about the changes.

Ad: only decode to LTC_ASN1_OID_STRING 👍

Yes, I am for opening a new PR so that we see what Travis-CI thinks about the changes. Ad: _only decode to LTC_ASN1_OID_STRING_ 👍
Sign in to join this conversation.
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libtom/libtomcrypt#362
No description provided.