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

Introduce EVP level fetchable sigalg functionality #14467

Open
levitte opened this issue Mar 8, 2021 · 10 comments
Open

Introduce EVP level fetchable sigalg functionality #14467

levitte opened this issue Mar 8, 2021 · 10 comments
Labels
triaged: feature The issue/pr requests/adds a feature
Milestone

Comments

@levitte
Copy link
Member

levitte commented Mar 8, 2021

In this discussion, a "sigalg" is understood to me a full signature algorithm, i.e. not just the keypair signing and verifying primitives,
but rather the algorithm that allows signing an unlimited size blob. In some part of OpenSSL, this is refered to as "sigandhash". So for example, "SHA1WithRSAEncryption" is a sigalg. So is, interestingly enough, "ED25519".

So far, the EVP functionality allows us to compose sigalgs from an EVP_MD and an EVP_PKEY. However, there is no functionality to ask for signing with a sigalg directly. Any application or library that try to use the algorithm OID in an X509_ALGOR as starting point has to take the ASN1_OBJECT given there, somehow find out what digest and pkey identity it refers to, treat the result separately until EVP_DigestSignInit() or EVP_DigestVerifyInit() is called. For OIDs that OpenSSL has internal knowledge of, this is possible OBJ_find_sigid_algs(). However, when new algorithms and new combinations appear, this knowledge becomes aged, or must be actively maintained.

Another way to handle this would be to allow providers to implement sigalgs among their set of signature implementations, and following along the line of thought from issue #14278, it would also allow them to include the corresponding OID. For example:

    { "sha1WithRSAEncryption:1.2.840.113549.1.1.5", "default=yes",
      ossl_rsa_sha1_signature_functions },

To be able to leverage this, EVP_DigestSignInit() and EVP_DigestVerifyInit() aren't suitable, as they are centered around EVP_MD_CTX, and EVP_MD isn't really a signature method. I therefore propose a new API which is related to EVP_Digest{Sign,Verify}, but made for sigalgs, with the possibility to compose a sigalgs from a signing and a hash algorithm
as an option.

    int EVP_SigalgSignInit(EVP_PKEY_CTX *pctx,
                           const char *sigalgname, const char *opt_hashname);
    int EVP_SigalgSignUpdate(EVP_PKEY_CTX *ctx, const void *data, size_t dsize);
    __owur int EVP_SigalgSignFinal(EVP_PKEY_CTX *ctx, unsigned char *sigret,
                                   size_t *siglen);
    __owur int EVP_SigalgSign(EVP_PKEY_CTX *pctx,
                              unsigned char *sigret, size_t *siglen,
                              const unsigned char *tbs, size_t tbslen);
    
    int EVP_SigalgVerifyInit(EVP_PKEY_CTX *pctx,
                             const char *sigalgname, const char *opt_hashname);
    int EVP_SigalgVerifyUpdate(EVP_PKEY_CTX *ctx, const void *data, size_t dsize);
    __owur int EVP_SigalgVerifyFinal(EVP_PKEY_CTX *ctx, const unsigned char *sig,
                                     size_t siglen);
    __owur int EVP_SigalgVerify(EVP_PKEY_CTX *pctx,
                                const unsigned char *sig, size_t siglen,
                                const unsigned char *tbs, size_t tbslen);

On the provider side, there's no need to do much, the available digest_sign and digest_verify functionality is suitable enough.

With this, code that does something like this:

    EVP_PKEY *pkey = /* value from somewhere */;
    X509_ALGOR *alg = /* value from somewhere */;
    int sigid, mdnid, pknid;
    const EVP_MD *md;
    EVP_MD_CTX *mdctx;
    
    sigid = OBJ_obj2nid(alg->algorithm);
    OBJ_find_sigid_algs(sigid, &mdnid, &pknid);
    md = EVP_get_digestbynid(mdnid);
    mdctx = EVP_MD_CTX_new();
    EVP_DigestSignInit(mdctx, NULL, md, NULL, pkey);

Can be replaced with:

    EVP_PKEY *pkey = /* value from somewhere */;
    X509_ALGOR *alg = /* value from somewhere */;
    EVP_PKEY_CTX *pctx;
    char txtoidbuf[OSSL_MAX_NAME_SIZE];
    
    pctx = EVP_PKEY_CTX_new(pkey, NULL);
    OBJ_obj2txt(txtoidbuf, sizeof(txtoidbuf), alg->algorithm, 0);
    EVP_SigalgSignInit(pctx, txtoidbuf, NULL);

That leaves zero need for OpenSSL to maintain knowledge about the composition of diverse sigalgs, it's enough that providers share that knowledge through its signature implementation names.

I imagine EVP_SigalgSignInit() and EVP_SigalgVerifyInit() implemented in such a way that it allows sigalg composition where needed, so calls like these would be possible:

    /*
     * The sigalgname argument is used to fetch the ímplementation.
     * There can be a fallback that uses OBJ_find_sigid_algs()
     * internally to find a usable composition.
     */
    EVP_SigalgSignInit(pctx, "sha1WithRSAEncryption", NULL);
    /*
     * The combination of a signature algorithm and a hash algorithm
     * would be available as an option.
     */
    EVP_SigalgSignInit(pctx, "RSAEncryption", "SHA1");
    /*
     * This combination derives the signature algorithm from the
     * EVP_PKEY embedded in |pctx|, and combines that with "SHA1" for
     * a complete sigalg.  This effectively reproduces what is usually
     * done with EVP_DigestSignInit(), and would be useful for all the
     * applications where you specify the hash algorithm separately.
     */
    EVP_SigalgSignInit(pctx, NULL, "SHA1");

There are several benefits with this:

  1. Just as with Add OIDs among algorithm names + don't go via NIDs when fetching an algorithm from a ASN1_OBJECT #14278, this would allow providers to have algorithms directly fetchable by with name and OID in text form.
  2. libcrypto doesn't need to maintain a sigid <-> mdnid+pknid map.
  3. going from an algorithm OID to setting up a signature operation is much more straight forward.
  4. this paves the way for pluggable signature.
@levitte levitte added issue: feature request The issue was opened to request a feature triaged: feature The issue/pr requests/adds a feature labels Mar 8, 2021
@levitte levitte added this to the 3.0.0 beta1 milestone Mar 8, 2021
@levitte levitte removed the issue: feature request The issue was opened to request a feature label Mar 8, 2021
@levitte
Copy link
Member Author

levitte commented Mar 8, 2021

[insert standard mantra]

@levitte
Copy link
Member Author

levitte commented Mar 8, 2021

Also, having fetchable sigalgs would remove the need of functions like EVP_PKEY_supports_digest_nid(), which someone recently tried to implement for provider keys, see #14431

@paulidale
Copy link
Contributor

This doesn't seem to support the use case where the digest is done in one provider and the signing in another.

@levitte
Copy link
Member Author

levitte commented Mar 9, 2021

This doesn't seem to support the use case where the digest is done in one provider and the signing in another.

An additional property query argument (and, I assume, library context) to the init functions would fix that. That would of course mostly be sensible with the composite type of init without tying ourselves in a knot.

@beldmit
Copy link
Member

beldmit commented Mar 9, 2021

I don't understand whether it will reduce the burden of maintaining the hash-signature compatibility list or just partially moves it to the provider's side. But it may introduce the other chain of fallbacks to the application: try the sigalg approach first, composite approach next...

@levitte
Copy link
Member Author

levitte commented Mar 9, 2021

Well, it does push it to the provider... It means that they have to make one OSSL_DISPATCH for each sigalg, and I assume that the only difference would be in the "newctx" functions, which would have to be specialized for each sigalg, but that they otherwise re-use the exact same digest_sign and digest_verify as the "base" signature implementation.

Point is still that the providers are those that have this knowledge, and this is crucial to be aware of as soon as algorithms that libcrypto has no internal knowledge of are implemented. National signature schemes is the prime example.

As a pointed example (:wink:), libcrypto has the following internal knowledge about GOST:

id_GostR3411_94_with_GostR3410_2001 id_GostR3411_94 id_GostR3410_2001
id_GostR3411_94_with_GostR3410_94 id_GostR3411_94 id_GostR3410_94
id_GostR3411_94_with_GostR3410_94_cc id_GostR3411_94 id_GostR3410_94_cc
id_GostR3411_94_with_GostR3410_2001_cc id_GostR3411_94 id_GostR3410_2001_cc
id_tc26_signwithdigest_gost3410_2012_256 id_GostR3411_2012_256 id_GostR3410_2012_256
id_tc26_signwithdigest_gost3410_2012_512 id_GostR3411_2012_512 id_GostR3410_2012_512

This knowledge logically belongs with a GOST provider, and would be rather easy to maintain, approximately like this:

const OSSL_ALGORITHM gost_signature_algorithms[] = {
    /* ... */
    { "id-GostR3411-94-with-GostR3410-2001:GOST R 34.11-94 with GOST R 34.10-2001:1.2.643.2.2.3",
      /* props */ NULL, gost_r3411_r3410_2001_signature_functions },
    { "id-GostR3411-94-with-GostR3410-94:GOST R 34.11-94 with GOST R 34.10-94:1.2.643.2.2.4",
      /* props */ NULL, gost_r3411_r3410_94_signature_functions },
    { "id-GostR3411-94-with-GostR3410-94-cc:GOST R 34.11-94 with GOST R 34.10-94 Cryptocom:1.2.643.2.9.1.3.3",
      /* props */ NULL, gost_r3411_r3410_94_cc_signature_functions },
    { "id-GostR3411-94-with-GostR3410-2001-cc:GOST R 34.11-94 with GOST R 34.10-2001 Cryptocom:1.2.643.2.9.1.3.4",
      /* props */ NULL, gost_r3411_r3410_2001_cc_signature_functions },
    { "id-tc26-signwithdigest-gost3410-2012-256:GOST R 34.10-2012 with GOST R 34.11-2012 (256 bit):1.2.643.7.1.1.3.2",
      /* props */ NULL, gost_r3411_r3410_2012_256_signature_functions },
    { "id-tc26-signwithdigest-gost3410-2012-512:GOST R 34.10-2012 with GOST R 34.11-2012 (512 bit):1.2.643.7.1.1.3.3",
      /* props */ NULL, gost_r3411_r3410_2012_512_signature_functions },
    /* ... */
};

The final point is that there should be no need whatsoever for a GOST provider to inform libcrypto what these sigalgs are composed of. So the whole association table for GOST in obj_xref.txt could simply be removed.

@beldmit
Copy link
Member

beldmit commented Mar 9, 2021

Yes. It perfectly fits for the current GOST implementation. Each GOST algorithm has exactly one associated digest, so the table entrances should be removed. It will somehow work for some future PKCS#11 provider that uses hardware implementation for both hash and signature. We will have to dance some fetch dance when we get the digest from some software provider and the key from PKCS#11 one, but it's OK.

BTW, I don't see how does it solve any problem with RSA.

@levitte
Copy link
Member Author

levitte commented Mar 9, 2021

BTW, I don't see how does it solve any problem with RSA.

It removes the need to know in libcrypto what any sigalg is composed of. Ideally, I'd like to be able to throw away the need to use OBJ_find_sigid_algs().

This would also (at least ideally) mean that we can drop the whole dance we do in ciphersuite setup in libssl, and fairly easily open up for new algos that aren't known internally in libcrypto.

@mattcaswell
Copy link
Member

This doesn't seem to support the use case where the digest is done in one provider and the signing in another.

Any provider implementing a signature primitive is at liberty to defer the hash implementation to some other provider. That is actually the way it already works with EVP_DigestSign, i.e. what we fetch is the signature algorithm and we pass the digest name as a parameter. The provider is then responsible for finding the digest implementation.

libssl actually has a problem at the moment in that it assumes a sigalg is available if it can fetch both the signature primitive and the digest primitive. However that may lead to the wrong answer because it is possible that a signature is available and a digest is available, but not in combination.

@nhorman
Copy link
Contributor

nhorman commented Jun 10, 2024

@levitte I see you still have a draft pr open against this. Is this still something you are (or want to ) work on?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

No branches or pull requests

5 participants