Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect reaction to already registered OIDs and NIDs #621

Open
baentsch opened this issue Jan 9, 2025 · 7 comments
Open

Incorrect reaction to already registered OIDs and NIDs #621

baentsch opened this issue Jan 9, 2025 · 7 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@baentsch
Copy link
Member

baentsch commented Jan 9, 2025

With the addition of ML-DSA OIDs to the openssl registry, the "all-or-nothing" logic registering new algorithms proves to be incorrect.

This issue therefore suggests to change this to just skip registering those algorithms failing in this stage but let registration of other algorithms proceed.

Tagging @t8m for advice whether this is sensible or another approach is feasible or advisable: The core problem is that core_obj_create fails to register a new NID (returns NID_undef upon trying to re-register mldsa). After that failure, the name/NID association doesn't exist and the algorithm cannot be used. So we again face the "naming conundrum" also for sig algs...

@t8m
Copy link

t8m commented Jan 9, 2025

IMO the best way would be first to check whether an object with such name already exists and if so just skip the creation of it.

@baentsch
Copy link
Member Author

baentsch commented Jan 9, 2025

IMO the best way would be first to check whether an object with such name already exists and if so just skip the creation of it.

That was straightforward to do/accept -- unfortunately when the OBJid/NID creation doesn't succeed, the algorithm isn't available under that name and with that OID, so all further logic, incl. X.509, using that name (lowercase-no-hyphen) fails...

@baentsch
Copy link
Member Author

baentsch commented Jan 9, 2025

Disregard the above: Your proposal is to check the OIDs presence (OBJ_obj2txt), right? That then gives both name and NID. But then oqsprovider must somehow "dynamically" adapt to that name... If that's your suggestion, @t8m (?) let me think how to do this -- at least quite a few static structures would need to become dynamic....

@t8m
Copy link

t8m commented Jan 9, 2025

That then gives both name and NID. But then oqsprovider must somehow "dynamically" adapt to that name...

Does it need to adapt to that name? Can't we expect that at least one of the aliases in the algorithm naming mapping will be matching?

@baentsch
Copy link
Member Author

Can't we expect that at least one of the aliases in the algorithm naming mapping will be matching?

I'm not sure I understand: Which alias you're referring to? Is there already a suitable mapping? Any way I can check this mapping manually? Or what would oqsprovider have to do to establish this mapping (if it can)?

This is how I understand things: "mldsa44" fails to register as "its" OID already is registered to "ML-DSA-44". It therefore seems that all efforts to try to "resolve" the algorithm "mldsa44" fail (even after removing the restrictive checks above), e.g., via the OBJ_xxx2xxx calls. So I guess the question is: Is there a way (and desire) to create an alias "mldsa44" for "ML-DSA-44" (and equivalently for the other variants)? Thanks in advance for any guidance how/where to add that, @t8m.

@t8m
Copy link

t8m commented Jan 10, 2025

Hmm... I assume (at least for now) that oqsprovider will have to stop supporting algorithms for which it fails to register the OBJs. It would be able to keep supporting them but it would have to change their internal names to the same as registered by openssl and use the existing names only as aliases (in the provided algorithm name). Basically it would have to align the naming with OpenSSL.

@baentsch
Copy link
Member Author

baentsch commented Jan 10, 2025

Basically it would have to align the naming with OpenSSL.

That'd be the cleanest solution and also the one most easily resolving #617: Let me simply switch it over and see whether anyone complains :->>.

Edit/add: Yes, the whole macro logic complains/breaks down :-( Sigh. Time to add even more macros, changing "-"->"_" :-/. Let's see when I'm finding the motivation for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants