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

[design] Functions for explicitly fetched signature algorithms #22672

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

levitte
Copy link
Member

@levitte levitte commented Nov 9, 2023

This design goes into more details what was outlined in the design for
fetching composite (PKEY) algorithms and using them.

It also changes what functionality will be used for this. The design for
signature was originally to add modified initializers for DigestSign and
DigestVerify, but recent OTC discussions redirected us to have a closer look
at EVP_PKEY_sign() and EVP_PKEY_verify().

Finally, it also takes into account the need to specify the signature
to be verified against with EVP_PKEY_verify() streaming functions,
which has been discussed in #22357.

Related to #22357 (in progress), #22129 (merged), and openssl/project#231

@levitte levitte mentioned this pull request Nov 9, 2023
2 tasks
@levitte levitte force-pushed the design-functions-for-explicitly-fetched-signature-algorithms branch from 317c78b to 07c769d Compare November 9, 2023 09:56
@levitte
Copy link
Member Author

levitte commented Nov 9, 2023

This design is still missing some provider interfaces to be added. I'm currently looking more closely at that. But don't let that stop you from commenting or discussing further in #22671, input is welcome!

@levitte levitte force-pushed the design-functions-for-explicitly-fetched-signature-algorithms branch from 07c769d to b986ca5 Compare November 9, 2023 10:26
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

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

This looks good so far, although I need to think through how applications should change to be able to use these new functions but still work with old providers and legacy.

@levitte
Copy link
Member Author

levitte commented Nov 9, 2023

This looks good so far, although I need to think through how applications should change to be able to use these new functions but still work with old providers and legacy.

This new functionality won't support legacy. This is about explicitly fetched algorithms, there's nothing legacy about that.

The streaming functions (update, final) will also not be possible to use with old providers. The one-shot functions should continue to work transparently, however.

@t8m t8m added branch: master Merge to master branch triaged: design The issue/pr deals with a design document labels Nov 14, 2023
@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 61 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@levitte levitte force-pushed the design-functions-for-explicitly-fetched-signature-algorithms branch from a4be04b to 1dd902c Compare January 16, 2024 13:41
@levitte levitte marked this pull request as ready for review January 16, 2024 13:41
@levitte
Copy link
Member Author

levitte commented Jan 16, 2024

Rebased...

Also, I had forgotten that this was still draft, it should have been made ready for review a while ago.

@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 30 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@openssl-machine
Copy link
Collaborator

This PR is waiting for the creator to make requested changes but it has not been updated for 61 days. If you have made changes or commented to the reviewer please make sure you re-request a review (see icon in the 'reviewers' section).

@openssl-machine
Copy link
Collaborator

This PR has been closed. It was waiting for the creator to make requested changes but it has not been updated for 90 days.

@t8m t8m reopened this Apr 16, 2024
@t8m t8m added approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member tests: exempted The PR is exempt from requirements for testing labels Apr 16, 2024
@t8m
Copy link
Member

t8m commented Apr 16, 2024

@levitte please drop the unrelated fuzz-corpora submodule change.

@levitte levitte force-pushed the design-functions-for-explicitly-fetched-signature-algorithms branch from 1dd902c to e50903a Compare April 17, 2024 08:05
@levitte
Copy link
Member Author

levitte commented Apr 17, 2024

I squashed and rebased too, while I was at it

@romen
Copy link
Member

romen commented May 14, 2024

@vdukhovni this is a relevant discussion #23240 (comment)

@levitte
Copy link
Member Author

levitte commented May 14, 2024

There was a discussion on how ED25519 would fit in this API proposal, considering its double-pass nature (which I had frankly forgotten).

To me, that would make it perfectly suitable for oneshot functionality (which is essentially what EVP_PKEY_sign() is), and not at all suitable for a stream style functionality. So, for this algorithm, it would be perfectly feasible to initiate it with this function (that's the function that was designed before the last fixup that replaced it with EVP_PKEY_sign_primitive_init and EVP_PKEY_sign_data_init):

int EVP_PKEY_sign_init_ex2(EVP_PKEY_CTX *pctx,
                           EVP_SIGNATURE *algo, const OSSL_PARAM params[]);

... and then call EVP_PKEY_sign() to perform the actual (oneshot) signature.

In provider terms, I would have an ED25519 implementation implement OSSL_FUNC_SIGNATURE_SIGN_INIT and OSSL_FUNC_SIGNATURE_SIGN, and that should be sufficient. (As a matter of fact, this should conceptually be possible to do, and provide the use of the current EVP_PKEY_sign functionality, given an EVP_PKEY_CTX loaded with an ED25519 key).


All that said, what you could call such a signature implementation? primitive or data? I bet we don't have a consensus on this, and that's exactly a reason why I quite dislike the added EVP_PKEY_sign_primitive and EVP_PKEY_sign_data function sets.

@t8m
Copy link
Member

t8m commented May 14, 2024

That's why you need all these APIs:

  1. Oneshot primitive signature operation
  2. Oneshot data signature operation
  3. Streaming data signature operation

Let's look at which algos would implement what:

Algo Oneshot primitive Oneshot data Streaming data
RSA x - -
sha256WithRSAEncryption - x x
ECDSA x - -
ecdsa-with-SHA256 - x x
Ed25519 (pure) - x -
Ed25519ph x x x

Let me explain the Ed25519ph - the primitive operation signs a hash value created with SHA-512, the oneshot data uses SHA-512 to hash arbitrary message (in one shot) and then signs the hash value with the Ed25519ph primitive and similarly for the streaming data signature.

As you can see for some algorithms all the three operations make sense and are doing something different.

@levitte
Copy link
Member Author

levitte commented May 14, 2024

Let me explain the Ed25519ph - the primitive operation signs a hash value created with SHA-512, the oneshot data uses SHA-512 to hash arbitrary message (in one shot) and then signs the hash value with the Ed25519ph primitive and similarly for the streaming data signature.

To me, the "primitive operation" sounds exactly like feeding a SHA-512 hash result to ED25519. This is, incidently, exactly what RFC8032 says:

For Ed25519ph, phflag=1 and PH is SHA512 instead. That is, the input
is hashed using SHA-512 before signing with Ed25519.

So in a way, one could say that it makes ED25519 the "primitive" for ED25519ph.

UPDATE: The "primitive" ED25519ph, as you express it, makes it exactly the same as ED25519, the only difference being that tbs (in EVP_PKEY_sign() terms) is assumed to be a SHA-512(dom2(1,'')||M).


I think we, again, need to actually define, at least for ourselves, what we want "primitive" to mean, on a conceptual level... at least if we want to continue with that distinction.

@levitte
Copy link
Member Author

levitte commented May 14, 2024

In a chat, @t8m and I came to the following definition of "primitive":

  • an algorithm where the user usually must do some extra work with the input for safe or standard compliant usage

I'll try to work that into the design text

@levitte
Copy link
Member Author

levitte commented May 15, 2024

There's been an exchange in #23240 that lead me to think that "primitive" isn't the best name for what's been discussed. How about "digest" or "prehash":

int EVP_PKEY_sign_init_for_digest(EVP_PKEY_CTX *pctx,
                                  EVP_SIGNATURE *algo, const OSSL_PARAM params[]);

I think that gives a better idea what this is supposed to do.

While I'm at it, "message" might also be a better name than "data"...

(I imagine that EVP_PKEY_sign_init_for_digest call with a fetched "RSA" would refuse to work with RSA_NO_PADDING, but would otherwise work just as EVP_PKEY_sign_init() does with an RSA pkey)

@t8m
Copy link
Member

t8m commented May 15, 2024

(I imagine that EVP_PKEY_sign_init_for_digest call with a fetched "RSA" would refuse to work with RSA_NO_PADDING, but would otherwise work just as EVP_PKEY_sign_init() does with an RSA pkey)

Heh, and that's why I hate the EVP_PKEY_sign_init_for_digest name. I really do not understand what is the problem with the primitive word.

I have strong opinion the existing EVP_PKEY_sign_init() must be internally just an alias (apart from the EVP_SIGNATURE argument) for the primitive sign init (or whatever you call it). Otherwise this will be unimplementable properly on the provider side. Or, in other words, on the provider side you have to reflect the difference between the primitive and data inits otherwise the same confusion that is on the EVP API side will happen on the provider side.

@levitte
Copy link
Member Author

levitte commented May 15, 2024

(I imagine that EVP_PKEY_sign_init_for_digest call with a fetched "RSA" would refuse to work with RSA_NO_PADDING, but would otherwise work just as EVP_PKEY_sign_init() does with an RSA pkey)

Heh, and that's why I hate the EVP_PKEY_sign_init_for_digest name. I really do not understand what is the problem with the primitive word.

In light of PKCS#1, where the cryptographic primitives are RSAEP, RSADP, RSASP1 and RSAVP1, using "primitive" for something that's a bit higher level can be quite confusing. Meanwhile, the intent with this form is explicitly to take a pre-hash as input, right? I find it more clear if the function name reflects that intention.

I have strong opinion the existing EVP_PKEY_sign_init() must be internally just an alias (apart from the EVP_SIGNATURE argument) for the primitive sign init (or whatever you call it). Otherwise this will be unimplementable properly on the provider side. Or, in other words, on the provider side you have to reflect the difference between the primitive and data inits otherwise the same confusion that is on the EVP API side will happen on the provider side.

Agreed, and if you look at the updated design, that's what I did.

@levitte
Copy link
Member Author

levitte commented May 16, 2024

I really do not understand what is the problem with the primitive word.

It's a source of confusion. For example, if you come from reading PKCS#1, the primitives are cryptographic primitives are RSAEP, RSADP, RSASP1 and RSAVP1. They take raw data (represented as numbers) with no further assumptions made on them. That's a fairly intuitive use of the word "primitive", actually meaning primitive.

Of course, EVP_PKEY_sign() gives access to that sort of functionality under certain conditions (RSA_NO_PADDING for RSA keys, for example), but it also gives access to more functionality which, from a PKCS#1 perspective, is going away from being primitive. My intuition would have me believe that something called EVP_PKEY_sign_primitive() gives me access to the RSA_NO_PADDING functionality for RSA keys, which is not at all what is intended. So circling back to intention, using a name which indicates that the intended input is digests seems clearer.

@levitte
Copy link
Member Author

levitte commented May 16, 2024

I kind of got a sort of "aha!" moment when I saw @davidben talk in terms of SignDigest and SignMessage, back in #23240. That kinda connected the dots for me.

@t8m
Copy link
Member

t8m commented May 16, 2024

The problem is that this function has to be able to do both - the sign primitive and the sign digest operations. But yeah, I am not going to insist on the primitive name.

@vdukhovni
Copy link

I kind of got a sort of "aha!" moment when I saw @davidben talk in terms of SignDigest and SignMessage, back in #23240. That kinda connected the dots for me.

I was hoping you'd see it that way. That is, instead of thinking in terms of how things are implemented, we should focus on the semantic properties of the APIs. And as @davidben pointed out, these are:

  1. An IUF (init, update, final) API for digesting a message with a given signature algorithm, which may be a combination of a public key algorithm and choice of digest, or the digest may be a fixed feature of the signature algorithm as in Ed25519ph, where it is always SHA2-512. If the digest is configurable, the signature will typically cover not just the digest value, but also the choice of digest algorithm. Additional algorithm-specific parameters may be applicable.
  2. A one-shot message signature, as in pure EdDSA (25519 or 448). Any internal digest is implicit, however there may still be additional parameters (e.g. the optional "context" in Ed25519ctx and Ed448ctx).
  3. A raw one-shot signature of input that the caller should have pre-hashed, but this is an unsafe interface, mostly IMHO for testing debugging (as with "rsautl -raw, that I use from time to time to peek inside RSA signed blobs).

We're presently missing a IUF APIs for Ed25519ph and Ed448ph, which would be nice to also expose via pkeyutl(1).

David

@levitte
Copy link
Member Author

levitte commented May 16, 2024

What we don't have is a IUF API that uses a full, explicitly fetched EVP_SIGNATURE. That would be useful for, like you said, ED25519ph, but also sha256WithRSAEncryption. That's what this design is for.

@vdukhovni
Copy link

What we don't have is a IUF API that uses a full, explicitly fetched EVP_SIGNATURE. That would be useful for, like you said, ED25519ph, but also sha256WithRSAEncryption. That's what this design is for.

Yes. Does the prior discussion then suggest a better name than "primitive"? Given IUF, we want some sort of context, that may as well capture at least the full signature algorithm (RSAwithXXX or Ed25519ph, ...), and perhaps also the key (unless that's part of "init", with "create" or "new" allocating the context).

Do you have a fully fleshed out IUF API (modulo its name)?

@levitte
Copy link
Member Author

levitte commented May 16, 2024

This design is all about extending the EVP_PKEY_sign function set with I for explicitly fetched EVP_SIGNATURE and specific uses, along with the U and F functions. The idea is that EVP_PKEY_CTX should be preloaded with the key to be used, and the provider interface gets an added query function to find out what key types a signature implementation supports.

This actually summarizes what's written in the design file. I encourage you to have a closer look.

@t8m
Copy link
Member

t8m commented May 16, 2024

Of course one option would be to call the new function for the sign primitive or sign digest operation just EVP_PKEY_sign_init_ex().

Whether that makes it good enough name to not be confused with the EVP_PKEY_sign_message_init() I do not know.

@levitte
Copy link
Member Author

levitte commented May 16, 2024

Of course one option would be to call the new function for the sign primitive or sign digest operation just EVP_PKEY_sign_init_ex().

That was my original design (it would have to be _ex2, EVP_PKEY_sign_init_ex() already exists)... so we're kind of coming full circle.

@t8m
Copy link
Member

t8m commented May 17, 2024

Of course one option would be to call the new function for the sign primitive or sign digest operation just EVP_PKEY_sign_init_ex().

That was my original design (it would have to be _ex2, EVP_PKEY_sign_init_ex() already exists)... so we're kind of coming full circle.

Yeah, unfortunately, given there is no better name that would encompass both primitive (i.e. no padding in case of RSA) and less-primitive operations (i.e. PKCS1 1.5 or PSS padding).

#### Signing a stream

``` C
int EVP_PKEY_sign_init_for_message(EVP_PKEY_CTX *pctx,
Copy link
Member

Choose a reason for hiding this comment

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

I would call it EVP_PKEY_sign_message_init().

const unsigned char *in, size_t inlen);
int EVP_PKEY_sign_final(EVP_PKEY_CTX *ctx,
unsigned char *sig, size_t *siglen);
```
Copy link
Member

Choose a reason for hiding this comment

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

We will also need a oneshot. I.e., EVP_PKEY_sign_message(). And I would change those two above to EVP_PKEY_sign_message_update() and EVP_PKEY_sign_message_final().

@openssl-machine
Copy link
Collaborator

This PR is in a state where it requires action by @openssl/committers but the last update was 30 days ago

@levitte
Copy link
Member Author

levitte commented Jul 4, 2024

@vdukhovni, you've asked me to provide some demo code for you to look at. I'm currently working on the implementation in #23416, which includes enhanced documentation of both EVP_PKEY_sign.pod and EVP_PKEY_verify.pod, with a number of examples in each. Is that good enough to at least eyeball?

(all examples refer to "RSA-SHA256", which I haven't finished implementing next, but am currently working on, with the hope to be done today or tomorrow)

This design goes into more details what was outlined in the design for
[fetching composite (PKEY) algorithms and using them].

It also changes what functionality will be used for this.  The design for
signature was originally to add modified initializers for DigestSign and
DigestVerify, but recent OTC discussions redirected us to have a closer look
at EVP_PKEY_sign() and EVP_PKEY_verify().

[fetching composite (PKEY) algorithms and using them]:
    ./fetching-composite-algorithms.md
This modifies the design - yet again - as discussions have revealed aspects
that weren't immediately apparent.
This mostly expands the explanatory text, but also makes a statement on how
the new functionality is designed.
@levitte levitte force-pushed the design-functions-for-explicitly-fetched-signature-algorithms branch from 720e57e to b48d288 Compare July 4, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: review pending This pull request needs review by a committer branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: design The issue/pr deals with a design document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet