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

Update pubkey.rst #4586

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Update pubkey.rst #4586

merged 1 commit into from
Jan 24, 2025

Conversation

randombit
Copy link
Owner

Along with a few doc comments in pubkey.h

@randombit randombit force-pushed the jack/update-pubkey-rst branch from 3abebbd to faf49c0 Compare January 23, 2025 12:32
@coveralls
Copy link

coveralls commented Jan 23, 2025

Coverage Status

coverage: 91.212% (+0.006%) from 91.206%
when pulling 3236904 on jack/update-pubkey-rst
into c637fc6 on master.

Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lot's of important/useful information. Thanks a lot! Below are some suggestions and typo nits.

doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
Comment on lines +1042 to +1049
RSA signature padding schemes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

These signature padding mechanisms are specific to RSA; no other public
key algorithms included in Botan make use of then. For historical reasons,
many different padding schemes have been defined for RSA over the years.
The most common are PSS and the (now obsolete) PKCS1v15.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this is really crucial information for crypto newcomers!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The situation was a lot more confused in earlier releases

[Pre 2.0] Rabin-Williams and Nyberg-Rueppel could use the same padding schemes as RSA

"EMSA1" (aka the plain hash with possible truncation) was a real padding scheme that was used with *DSA and could be used with RSA as well.

Now things are pretty clearly bifurcated to "RSA specific" and "everyone else"

doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
Comment on lines +1157 to +1163
Sign inputs directly with no hashing or padding

.. warning::

This exists as an escape hatch allowing an application to define
some protocol-specific padding scheme. Don't use this unless you
know what you are doing.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud: once the PK_Options (#4318) are fully operational, we could potentially replace this escape hatch with the ability for a user to create their own subclass of EMSA to build application-specific paddings. And then pass it along as .with_padding(ptr_to_an_object_of_my_emsa). (Assuming that class EMSA is even public API.)

That makes it more involved to YOLO your own padding, with more boilerplate. But perhaps that's actually a good thing.

doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
Comment on lines 1299 to 1300
Return a shared key. The *salt* will be hashed along with the shared secret by the
KDF; this can be useful to bind the shared secret to a specific usage.
Copy link
Collaborator

@reneme reneme Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add something like: "if no KDF was defined, the salt and the key length are typically ignored." (Or a formulation along those lines.)

I actually find this to be an API pitfall, because users might have designed their protocol around the idea of injecting that context while it fact it is just silently ignored. Here, a to-be-built API along the lines of PK_Options (#4318) might come in handy. It would make it easy to throw an exception if .with_salt() (or .with_output_length()) was given but no KDF actually ended up consuming those values in derive_key().

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we already do reject a salt if a KDF is not being used but I agree with your point in the broad sense.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to describe how keylen param is ignored without a kdf

doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
doc/api_ref/pubkey.rst Outdated Show resolved Hide resolved
@randombit randombit force-pushed the jack/update-pubkey-rst branch from a5275f1 to 3236904 Compare January 24, 2025 11:29
Along with a few doc comments in pubkey.h
@randombit randombit merged commit 8e7ab0e into master Jan 24, 2025
39 checks passed
@randombit randombit deleted the jack/update-pubkey-rst branch January 24, 2025 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants