diff mbox series

[ima-evm-utils] ima-evm-utils: ima_sign supports SM2 and SM3 algorithm combination

Message ID 20210131032721.79050-1-tianjia.zhang@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series [ima-evm-utils] ima-evm-utils: ima_sign supports SM2 and SM3 algorithm combination | expand

Commit Message

tianjia.zhang Jan. 31, 2021, 3:27 a.m. UTC
The combination of SM2 and SM3 algorithms has been implemented in the
kernel. At present, the ima-evm-utils signature tool does not support
this combination of algorithms. This is because SM2 sign require a
USERID, which requires the use of a higher-level sign functions of
OpenSSL. this patch use the EVP_DigestSign series of functions to
sign to support various signature algorithm combinations.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

Comments

Vitaly Chikunov Jan. 31, 2021, 5:48 p.m. UTC | #1
Tianjia,

On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
> The combination of SM2 and SM3 algorithms has been implemented in the
> kernel. At present, the ima-evm-utils signature tool does not support
> this combination of algorithms. This is because SM2 sign require a
> USERID, which requires the use of a higher-level sign functions of
> OpenSSL. this patch use the EVP_DigestSign series of functions to
> sign to support various signature algorithm combinations.
> 
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
>  src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
>  1 file changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libimaevm.c b/src/libimaevm.c
> index fa6c278..89b9b88 100644
> --- a/src/libimaevm.c
> +++ b/src/libimaevm.c
> @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>  	EVP_PKEY *pkey;
>  	char name[20];
>  	EVP_PKEY_CTX *ctx = NULL;
> +	EVP_MD_CTX *mctx = NULL;
>  	const EVP_MD *md;
>  	size_t sigsize;
>  	const char *st;
> @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>  		return -1;
>  	}
>  
> +#if OPENSSL_VERSION_NUMBER < 0x30000000

Perhaps, this check is not needed at all if it isn't enabled for new
openssl?


> +	/*
> +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
> +	 * key are used at the same time, check whether it is SM2 private key.
> +	 */
> +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
> +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
> +		if (curve == NID_sm2)
> +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
> +	}
> +#endif
> +
>  	calc_keyid_v2(&keyid, name, pkey);
>  	hdr->keyid = keyid;
>  
>  	st = "EVP_PKEY_CTX_new";
>  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
>  		goto err;
> -	st = "EVP_PKEY_sign_init";
> -	if (!EVP_PKEY_sign_init(ctx))
> +	st = "EVP_MD_CTX_new";
> +	if (!(mctx = EVP_MD_CTX_new()))
>  		goto err;
> +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
> +		st = "EVP_PKEY_CTX_set1_id";
> +		/* Set SM2 default userid */
> +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))

You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
I don't really get the API change. Can you explain more this
requirement?

Thanks,

> +			goto err;
> +	}
> +	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
>  	st = "EVP_get_digestbyname";
>  	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
>  		goto err;
> -	st = "EVP_PKEY_CTX_set_signature_md";
> -	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
> +	st = "EVP_DigestSignInit";
> +	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
> +		goto err;
> +	st = "EVP_DigestSignUpdate";
> +	if (!EVP_DigestSignUpdate(mctx, hash, size))
>  		goto err;
> -	st = "EVP_PKEY_sign";
> +	st = "EVP_DigestSignFinal";
>  	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
> -	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
> +	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
>  		goto err;
>  	len = (int)sigsize;
>  
> @@ -964,6 +988,7 @@ err:
>  			ERR_reason_error_string(ERR_peek_error()), st);
>  		output_openssl_errors();
>  	}
> +	EVP_MD_CTX_free(mctx);
>  	EVP_PKEY_CTX_free(ctx);
>  	EVP_PKEY_free(pkey);
>  	return len;
> -- 
> 2.19.1.3.ge56e4f7
Vitaly Chikunov Jan. 31, 2021, 5:56 p.m. UTC | #2
On Sun, Jan 31, 2021 at 08:48:46PM +0300, Vitaly Chikunov wrote:
> Tianjia,
> 
> On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
> > The combination of SM2 and SM3 algorithms has been implemented in the
> > kernel. At present, the ima-evm-utils signature tool does not support
> > this combination of algorithms. This is because SM2 sign require a
> > USERID, which requires the use of a higher-level sign functions of
> > OpenSSL. this patch use the EVP_DigestSign series of functions to
> > sign to support various signature algorithm combinations.
> > 
> > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > ---
> >  src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
> >  1 file changed, 31 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > index fa6c278..89b9b88 100644
> > --- a/src/libimaevm.c
> > +++ b/src/libimaevm.c
> > @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> >  	EVP_PKEY *pkey;
> >  	char name[20];
> >  	EVP_PKEY_CTX *ctx = NULL;
> > +	EVP_MD_CTX *mctx = NULL;
> >  	const EVP_MD *md;
> >  	size_t sigsize;
> >  	const char *st;
> > @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> >  		return -1;
> >  	}
> >  
> > +#if OPENSSL_VERSION_NUMBER < 0x30000000
> 
> Perhaps, this check is not needed at all if it isn't enabled for new
> openssl?
> 
> 
> > +	/*
> > +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
> > +	 * key are used at the same time, check whether it is SM2 private key.
> > +	 */
> > +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
> > +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> > +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
> > +		if (curve == NID_sm2)
> > +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
> > +	}
> > +#endif
> > +
> >  	calc_keyid_v2(&keyid, name, pkey);
> >  	hdr->keyid = keyid;
> >  
> >  	st = "EVP_PKEY_CTX_new";
> >  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
> >  		goto err;
> > -	st = "EVP_PKEY_sign_init";
> > -	if (!EVP_PKEY_sign_init(ctx))
> > +	st = "EVP_MD_CTX_new";
> > +	if (!(mctx = EVP_MD_CTX_new()))
> >  		goto err;
> > +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
> > +		st = "EVP_PKEY_CTX_set1_id";
> > +		/* Set SM2 default userid */
> > +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
> 
> You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
> I don't really get the API change. Can you explain more this
> requirement?

EVP_PKEY_sign should be able to sign with any algo, unless there is
openssl bug. Is there bug preventing SM2 signing with EVP_PKEY_sign?

> 
> Thanks,
> 
> > +			goto err;
> > +	}
> > +	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
> >  	st = "EVP_get_digestbyname";
> >  	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
> >  		goto err;
> > -	st = "EVP_PKEY_CTX_set_signature_md";
> > -	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
> > +	st = "EVP_DigestSignInit";
> > +	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
> > +		goto err;
> > +	st = "EVP_DigestSignUpdate";
> > +	if (!EVP_DigestSignUpdate(mctx, hash, size))
> >  		goto err;
> > -	st = "EVP_PKEY_sign";
> > +	st = "EVP_DigestSignFinal";
> >  	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
> > -	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
> > +	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
> >  		goto err;
> >  	len = (int)sigsize;
> >  
> > @@ -964,6 +988,7 @@ err:
> >  			ERR_reason_error_string(ERR_peek_error()), st);
> >  		output_openssl_errors();
> >  	}
> > +	EVP_MD_CTX_free(mctx);
> >  	EVP_PKEY_CTX_free(ctx);
> >  	EVP_PKEY_free(pkey);
> >  	return len;
> > -- 
> > 2.19.1.3.ge56e4f7
Vitaly Chikunov Jan. 31, 2021, 6:52 p.m. UTC | #3
Tianjia,

On Sun, Jan 31, 2021 at 08:56:23PM +0300, Vitaly Chikunov wrote:
> On Sun, Jan 31, 2021 at 08:48:46PM +0300, Vitaly Chikunov wrote:
> > On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
> > > The combination of SM2 and SM3 algorithms has been implemented in the
> > > kernel. At present, the ima-evm-utils signature tool does not support
> > > this combination of algorithms. This is because SM2 sign require a
> > > USERID, which requires the use of a higher-level sign functions of
> > > OpenSSL. this patch use the EVP_DigestSign series of functions to
> > > sign to support various signature algorithm combinations.
> > > 
> > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > > ---
> > >  src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
> > >  1 file changed, 31 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > index fa6c278..89b9b88 100644
> > > --- a/src/libimaevm.c
> > > +++ b/src/libimaevm.c
> > > @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> > >  	EVP_PKEY *pkey;
> > >  	char name[20];
> > >  	EVP_PKEY_CTX *ctx = NULL;
> > > +	EVP_MD_CTX *mctx = NULL;
> > >  	const EVP_MD *md;
> > >  	size_t sigsize;
> > >  	const char *st;
> > > @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> > >  		return -1;
> > >  	}
> > >  
> > > +#if OPENSSL_VERSION_NUMBER < 0x30000000
> > 
> > Perhaps, this check is not needed at all if it isn't enabled for new
> > openssl?
> > 
> > 
> > > +	/*
> > > +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
> > > +	 * key are used at the same time, check whether it is SM2 private key.
> > > +	 */
> > > +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
> > > +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> > > +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
> > > +		if (curve == NID_sm2)

Also, this patch does not compile on openssl 1.1.0 because 
NID_sm2 and EVP_PKEY_SM2 are not defined.

> > > +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
> > > +	}
> > > +#endif
> > > +
> > >  	calc_keyid_v2(&keyid, name, pkey);
> > >  	hdr->keyid = keyid;
> > >  
> > >  	st = "EVP_PKEY_CTX_new";
> > >  	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
> > >  		goto err;
> > > -	st = "EVP_PKEY_sign_init";
> > > -	if (!EVP_PKEY_sign_init(ctx))
> > > +	st = "EVP_MD_CTX_new";
> > > +	if (!(mctx = EVP_MD_CTX_new()))
> > >  		goto err;
> > > +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
> > > +		st = "EVP_PKEY_CTX_set1_id";
> > > +		/* Set SM2 default userid */
> > > +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
> > 
> > You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
> > I don't really get the API change. Can you explain more this
> > requirement?
> 
> EVP_PKEY_sign should be able to sign with any algo, unless there is
> openssl bug. Is there bug preventing SM2 signing with EVP_PKEY_sign?
> 
> > > +			goto err;
> > > +	}
> > > +	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
> > >  	st = "EVP_get_digestbyname";
> > >  	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
> > >  		goto err;
> > > -	st = "EVP_PKEY_CTX_set_signature_md";
> > > -	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
> > > +	st = "EVP_DigestSignInit";
> > > +	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
> > > +		goto err;
> > > +	st = "EVP_DigestSignUpdate";
> > > +	if (!EVP_DigestSignUpdate(mctx, hash, size))
> > >  		goto err;
> > > -	st = "EVP_PKEY_sign";
> > > +	st = "EVP_DigestSignFinal";
> > >  	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
> > > -	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))

Also, this conversion to EVP_DigestSign is incorrect, because
EVP_PKEY_sign does not hash the data (and expected to work over a ready
digest, see man EVP_PKEY_sign). While EVP_DigestSignFinal does hash the
data.

You should really try to run `make check` before sending patches.

Thanks,

> > > +	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
> > >  		goto err;
> > >  	len = (int)sigsize;
> > >  
> > > @@ -964,6 +988,7 @@ err:
> > >  			ERR_reason_error_string(ERR_peek_error()), st);
> > >  		output_openssl_errors();
> > >  	}
> > > +	EVP_MD_CTX_free(mctx);
> > >  	EVP_PKEY_CTX_free(ctx);
> > >  	EVP_PKEY_free(pkey);
> > >  	return len;
> > > -- 
> > > 2.19.1.3.ge56e4f7
tianjia.zhang Feb. 1, 2021, 9:10 a.m. UTC | #4
On 2/1/21 2:52 AM, Vitaly Chikunov wrote:
> Tianjia,
> 
> On Sun, Jan 31, 2021 at 08:56:23PM +0300, Vitaly Chikunov wrote:
>> On Sun, Jan 31, 2021 at 08:48:46PM +0300, Vitaly Chikunov wrote:
>>> On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
>>>> The combination of SM2 and SM3 algorithms has been implemented in the
>>>> kernel. At present, the ima-evm-utils signature tool does not support
>>>> this combination of algorithms. This is because SM2 sign require a
>>>> USERID, which requires the use of a higher-level sign functions of
>>>> OpenSSL. this patch use the EVP_DigestSign series of functions to
>>>> sign to support various signature algorithm combinations.
>>>>
>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>> ---
>>>>   src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
>>>>   1 file changed, 31 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/src/libimaevm.c b/src/libimaevm.c
>>>> index fa6c278..89b9b88 100644
>>>> --- a/src/libimaevm.c
>>>> +++ b/src/libimaevm.c
>>>> @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>>>>   	EVP_PKEY *pkey;
>>>>   	char name[20];
>>>>   	EVP_PKEY_CTX *ctx = NULL;
>>>> +	EVP_MD_CTX *mctx = NULL;
>>>>   	const EVP_MD *md;
>>>>   	size_t sigsize;
>>>>   	const char *st;
>>>> @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>>>>   		return -1;
>>>>   	}
>>>>   
>>>> +#if OPENSSL_VERSION_NUMBER < 0x30000000
>>>
>>> Perhaps, this check is not needed at all if it isn't enabled for new
>>> openssl?
>>>
>>>
>>>> +	/*
>>>> +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
>>>> +	 * key are used at the same time, check whether it is SM2 private key.
>>>> +	 */
>>>> +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
>>>> +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
>>>> +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
>>>> +		if (curve == NID_sm2)
> 
> Also, this patch does not compile on openssl 1.1.0 because
> NID_sm2 and EVP_PKEY_SM2 are not defined.
> 

OPENSSL 1.1.0 version does not support SM2, the macro detection of 
OPENSSL_VERSION should be added here.

>>>> +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
>>>> +	}
>>>> +#endif
>>>> +
>>>>   	calc_keyid_v2(&keyid, name, pkey);
>>>>   	hdr->keyid = keyid;
>>>>   
>>>>   	st = "EVP_PKEY_CTX_new";
>>>>   	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
>>>>   		goto err;
>>>> -	st = "EVP_PKEY_sign_init";
>>>> -	if (!EVP_PKEY_sign_init(ctx))
>>>> +	st = "EVP_MD_CTX_new";
>>>> +	if (!(mctx = EVP_MD_CTX_new()))
>>>>   		goto err;
>>>> +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
>>>> +		st = "EVP_PKEY_CTX_set1_id";
>>>> +		/* Set SM2 default userid */
>>>> +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
>>>
>>> You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
>>> I don't really get the API change. Can you explain more this
>>> requirement?
>>
>> EVP_PKEY_sign should be able to sign with any algo, unless there is
>> openssl bug. Is there bug preventing SM2 signing with EVP_PKEY_sign?
>>
The SM2 signature is different from the conventional signature 
algorithm. For example, to sign a message, not the hash signature of the 
message, but the hash signature after adding the message to ZA. The ZA 
value includes the SM3 hash of the following data, including some 
parameters and public keys of ECC, and the USERID passed in by the user.
it also specified in
https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02.

It seems that there is no good way. I can only add the calculation of 
the ZA value when calculating the file hash.

>>>> +			goto err;
>>>> +	}
>>>> +	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
>>>>   	st = "EVP_get_digestbyname";
>>>>   	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
>>>>   		goto err;
>>>> -	st = "EVP_PKEY_CTX_set_signature_md";
>>>> -	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
>>>> +	st = "EVP_DigestSignInit";
>>>> +	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
>>>> +		goto err;
>>>> +	st = "EVP_DigestSignUpdate";
>>>> +	if (!EVP_DigestSignUpdate(mctx, hash, size))
>>>>   		goto err;
>>>> -	st = "EVP_PKEY_sign";
>>>> +	st = "EVP_DigestSignFinal";
>>>>   	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
>>>> -	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
> 
> Also, this conversion to EVP_DigestSign is incorrect, because
> EVP_PKEY_sign does not hash the data (and expected to work over a ready
> digest, see man EVP_PKEY_sign). While EVP_DigestSignFinal does hash the
> data.
> 
> You should really try to run `make check` before sending patches.
> 
> Thanks,
> 

This is indeed a problem, thanks for pointing it out. Or to add ZA, I 
ignored this, maybe I should implement a version that does not support 
ZA first. Or do you have any better suggestions?

Thanks for your reply.

Best regards,
Tianjia

>>>> +	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
>>>>   		goto err;
>>>>   	len = (int)sigsize;
>>>>   
>>>> @@ -964,6 +988,7 @@ err:
>>>>   			ERR_reason_error_string(ERR_peek_error()), st);
>>>>   		output_openssl_errors();
>>>>   	}
>>>> +	EVP_MD_CTX_free(mctx);
>>>>   	EVP_PKEY_CTX_free(ctx);
>>>>   	EVP_PKEY_free(pkey);
>>>>   	return len;
>>>> -- 
>>>> 2.19.1.3.ge56e4f7
Vitaly Chikunov Feb. 1, 2021, 12:31 p.m. UTC | #5
On Mon, Feb 01, 2021 at 05:10:59PM +0800, Tianjia Zhang wrote:
> 
> 
> On 2/1/21 2:52 AM, Vitaly Chikunov wrote:
> > Tianjia,
> > 
> > On Sun, Jan 31, 2021 at 08:56:23PM +0300, Vitaly Chikunov wrote:
> > > On Sun, Jan 31, 2021 at 08:48:46PM +0300, Vitaly Chikunov wrote:
> > > > On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
> > > > > The combination of SM2 and SM3 algorithms has been implemented in the
> > > > > kernel. At present, the ima-evm-utils signature tool does not support
> > > > > this combination of algorithms. This is because SM2 sign require a
> > > > > USERID, which requires the use of a higher-level sign functions of
> > > > > OpenSSL. this patch use the EVP_DigestSign series of functions to
> > > > > sign to support various signature algorithm combinations.
> > > > > 
> > > > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > > > > ---
> > > > >   src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
> > > > >   1 file changed, 31 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/src/libimaevm.c b/src/libimaevm.c
> > > > > index fa6c278..89b9b88 100644
> > > > > --- a/src/libimaevm.c
> > > > > +++ b/src/libimaevm.c
> > > > > @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> > > > >   	EVP_PKEY *pkey;
> > > > >   	char name[20];
> > > > >   	EVP_PKEY_CTX *ctx = NULL;
> > > > > +	EVP_MD_CTX *mctx = NULL;
> > > > >   	const EVP_MD *md;
> > > > >   	size_t sigsize;
> > > > >   	const char *st;
> > > > > @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
> > > > >   		return -1;
> > > > >   	}
> > > > > +#if OPENSSL_VERSION_NUMBER < 0x30000000
> > > > 
> > > > Perhaps, this check is not needed at all if it isn't enabled for new
> > > > openssl?
> > > > 
> > > > 
> > > > > +	/*
> > > > > +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
> > > > > +	 * key are used at the same time, check whether it is SM2 private key.
> > > > > +	 */
> > > > > +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
> > > > > +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
> > > > > +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
> > > > > +		if (curve == NID_sm2)
> > 
> > Also, this patch does not compile on openssl 1.1.0 because
> > NID_sm2 and EVP_PKEY_SM2 are not defined.
> > 
> 
> OPENSSL 1.1.0 version does not support SM2, the macro detection of
> OPENSSL_VERSION should be added here.

Or just #ifdef EVP_PKEY_SM2.

> 
> > > > > +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
> > > > > +	}
> > > > > +#endif
> > > > > +
> > > > >   	calc_keyid_v2(&keyid, name, pkey);
> > > > >   	hdr->keyid = keyid;
> > > > >   	st = "EVP_PKEY_CTX_new";
> > > > >   	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
> > > > >   		goto err;
> > > > > -	st = "EVP_PKEY_sign_init";
> > > > > -	if (!EVP_PKEY_sign_init(ctx))
> > > > > +	st = "EVP_MD_CTX_new";
> > > > > +	if (!(mctx = EVP_MD_CTX_new()))
> > > > >   		goto err;
> > > > > +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
> > > > > +		st = "EVP_PKEY_CTX_set1_id";
> > > > > +		/* Set SM2 default userid */
> > > > > +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
> > > > 
> > > > You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
> > > > I don't really get the API change. Can you explain more this
> > > > requirement?
> > > 
> > > EVP_PKEY_sign should be able to sign with any algo, unless there is
> > > openssl bug. Is there bug preventing SM2 signing with EVP_PKEY_sign?
> > > 
> The SM2 signature is different from the conventional signature algorithm.
> For example, to sign a message, not the hash signature of the message, but
> the hash signature after adding the message to ZA. The ZA value includes the
> SM3 hash of the following data, including some parameters and public keys of
> ECC, and the USERID passed in by the user.
> it also specified in
> https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02.

I didn't read the RFC draft yet, but I believe EVP_PKEY_sign should be
able to sign with any padding scheme (that's why we call
EVP_PKEY_CTX_set_signature_md, for example, to tell how to encode
proper padding). Maybe you should ask OpenSSL people for suggestions?

Thanks,

> 
> It seems that there is no good way. I can only add the calculation of the ZA
> value when calculating the file hash.
> 
> > > > > +			goto err;
> > > > > +	}
> > > > > +	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
> > > > >   	st = "EVP_get_digestbyname";
> > > > >   	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
> > > > >   		goto err;
> > > > > -	st = "EVP_PKEY_CTX_set_signature_md";
> > > > > -	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
> > > > > +	st = "EVP_DigestSignInit";
> > > > > +	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
> > > > > +		goto err;
> > > > > +	st = "EVP_DigestSignUpdate";
> > > > > +	if (!EVP_DigestSignUpdate(mctx, hash, size))
> > > > >   		goto err;
> > > > > -	st = "EVP_PKEY_sign";
> > > > > +	st = "EVP_DigestSignFinal";
> > > > >   	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
> > > > > -	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
> > 
> > Also, this conversion to EVP_DigestSign is incorrect, because
> > EVP_PKEY_sign does not hash the data (and expected to work over a ready
> > digest, see man EVP_PKEY_sign). While EVP_DigestSignFinal does hash the
> > data.
> > 
> > You should really try to run `make check` before sending patches.
> > 
> > Thanks,
> > 
> 
> This is indeed a problem, thanks for pointing it out. Or to add ZA, I
> ignored this, maybe I should implement a version that does not support ZA
> first. Or do you have any better suggestions?
> 
> Thanks for your reply.
> 
> Best regards,
> Tianjia
> 
> > > > > +	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
> > > > >   		goto err;
> > > > >   	len = (int)sigsize;
> > > > > @@ -964,6 +988,7 @@ err:
> > > > >   			ERR_reason_error_string(ERR_peek_error()), st);
> > > > >   		output_openssl_errors();
> > > > >   	}
> > > > > +	EVP_MD_CTX_free(mctx);
> > > > >   	EVP_PKEY_CTX_free(ctx);
> > > > >   	EVP_PKEY_free(pkey);
> > > > >   	return len;
> > > > > -- 
> > > > > 2.19.1.3.ge56e4f7
tianjia.zhang Feb. 11, 2021, 5:39 a.m. UTC | #6
On 2/1/21 8:31 PM, Vitaly Chikunov wrote:
> On Mon, Feb 01, 2021 at 05:10:59PM +0800, Tianjia Zhang wrote:
>>
>>
>> On 2/1/21 2:52 AM, Vitaly Chikunov wrote:
>>> Tianjia,
>>>
>>> On Sun, Jan 31, 2021 at 08:56:23PM +0300, Vitaly Chikunov wrote:
>>>> On Sun, Jan 31, 2021 at 08:48:46PM +0300, Vitaly Chikunov wrote:
>>>>> On Sun, Jan 31, 2021 at 11:27:21AM +0800, Tianjia Zhang wrote:
>>>>>> The combination of SM2 and SM3 algorithms has been implemented in the
>>>>>> kernel. At present, the ima-evm-utils signature tool does not support
>>>>>> this combination of algorithms. This is because SM2 sign require a
>>>>>> USERID, which requires the use of a higher-level sign functions of
>>>>>> OpenSSL. this patch use the EVP_DigestSign series of functions to
>>>>>> sign to support various signature algorithm combinations.
>>>>>>
>>>>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
>>>>>> ---
>>>>>>    src/libimaevm.c | 37 +++++++++++++++++++++++++++++++------
>>>>>>    1 file changed, 31 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/src/libimaevm.c b/src/libimaevm.c
>>>>>> index fa6c278..89b9b88 100644
>>>>>> --- a/src/libimaevm.c
>>>>>> +++ b/src/libimaevm.c
>>>>>> @@ -891,6 +891,7 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>>>>>>    	EVP_PKEY *pkey;
>>>>>>    	char name[20];
>>>>>>    	EVP_PKEY_CTX *ctx = NULL;
>>>>>> +	EVP_MD_CTX *mctx = NULL;
>>>>>>    	const EVP_MD *md;
>>>>>>    	size_t sigsize;
>>>>>>    	const char *st;
>>>>>> @@ -932,24 +933,47 @@ static int sign_hash_v2(const char *algo, const unsigned char *hash,
>>>>>>    		return -1;
>>>>>>    	}
>>>>>> +#if OPENSSL_VERSION_NUMBER < 0x30000000
>>>>>
>>>>> Perhaps, this check is not needed at all if it isn't enabled for new
>>>>> openssl?
>>>>>
>>>>>
>>>>>> +	/*
>>>>>> +	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
>>>>>> +	 * key are used at the same time, check whether it is SM2 private key.
>>>>>> +	 */
>>>>>> +	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
>>>>>> +		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
>>>>>> +		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
>>>>>> +		if (curve == NID_sm2)
>>>
>>> Also, this patch does not compile on openssl 1.1.0 because
>>> NID_sm2 and EVP_PKEY_SM2 are not defined.
>>>
>>
>> OPENSSL 1.1.0 version does not support SM2, the macro detection of
>> OPENSSL_VERSION should be added here.
> 
> Or just #ifdef EVP_PKEY_SM2.
> 
>>
>>>>>> +			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
>>>>>> +	}
>>>>>> +#endif
>>>>>> +
>>>>>>    	calc_keyid_v2(&keyid, name, pkey);
>>>>>>    	hdr->keyid = keyid;
>>>>>>    	st = "EVP_PKEY_CTX_new";
>>>>>>    	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
>>>>>>    		goto err;
>>>>>> -	st = "EVP_PKEY_sign_init";
>>>>>> -	if (!EVP_PKEY_sign_init(ctx))
>>>>>> +	st = "EVP_MD_CTX_new";
>>>>>> +	if (!(mctx = EVP_MD_CTX_new()))
>>>>>>    		goto err;
>>>>>> +	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
>>>>>> +		st = "EVP_PKEY_CTX_set1_id";
>>>>>> +		/* Set SM2 default userid */
>>>>>> +		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
>>>>>
>>>>> You cannot call EVP_PKEY_CTX_set1_id for EVP_PKEY_sign's ctx?
>>>>> I don't really get the API change. Can you explain more this
>>>>> requirement?
>>>>
>>>> EVP_PKEY_sign should be able to sign with any algo, unless there is
>>>> openssl bug. Is there bug preventing SM2 signing with EVP_PKEY_sign?
>>>>
>> The SM2 signature is different from the conventional signature algorithm.
>> For example, to sign a message, not the hash signature of the message, but
>> the hash signature after adding the message to ZA. The ZA value includes the
>> SM3 hash of the following data, including some parameters and public keys of
>> ECC, and the USERID passed in by the user.
>> it also specified in
>> https://tools.ietf.org/html/draft-shen-sm2-ecdsa-02.
> 
> I didn't read the RFC draft yet, but I believe EVP_PKEY_sign should be
> able to sign with any padding scheme (that's why we call
> EVP_PKEY_CTX_set_signature_md, for example, to tell how to encode
> proper padding). Maybe you should ask OpenSSL people for suggestions?
> 
> Thanks,
> 

Thanks for your suggestion, I got some information and will do it in the 
next version of patch.

Best regards,
Tianjia
diff mbox series

Patch

diff --git a/src/libimaevm.c b/src/libimaevm.c
index fa6c278..89b9b88 100644
--- a/src/libimaevm.c
+++ b/src/libimaevm.c
@@ -891,6 +891,7 @@  static int sign_hash_v2(const char *algo, const unsigned char *hash,
 	EVP_PKEY *pkey;
 	char name[20];
 	EVP_PKEY_CTX *ctx = NULL;
+	EVP_MD_CTX *mctx = NULL;
 	const EVP_MD *md;
 	size_t sigsize;
 	const char *st;
@@ -932,24 +933,47 @@  static int sign_hash_v2(const char *algo, const unsigned char *hash,
 		return -1;
 	}
 
+#if OPENSSL_VERSION_NUMBER < 0x30000000
+	/*
+	 * SM2 and SM3 should go together. If SM3 hash algorithm and EC private
+	 * key are used at the same time, check whether it is SM2 private key.
+	 */
+	if (hdr->hash_algo == PKEY_HASH_SM3_256 && EVP_PKEY_id(pkey) == EVP_PKEY_EC) {
+		EC_KEY *ec = EVP_PKEY_get0_EC_KEY(pkey);
+		int curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
+		if (curve == NID_sm2)
+			EVP_PKEY_set_alias_type(pkey, EVP_PKEY_SM2);
+	}
+#endif
+
 	calc_keyid_v2(&keyid, name, pkey);
 	hdr->keyid = keyid;
 
 	st = "EVP_PKEY_CTX_new";
 	if (!(ctx = EVP_PKEY_CTX_new(pkey, NULL)))
 		goto err;
-	st = "EVP_PKEY_sign_init";
-	if (!EVP_PKEY_sign_init(ctx))
+	st = "EVP_MD_CTX_new";
+	if (!(mctx = EVP_MD_CTX_new()))
 		goto err;
+	if (EVP_PKEY_id(pkey) == EVP_PKEY_SM2) {
+		st = "EVP_PKEY_CTX_set1_id";
+		/* Set SM2 default userid */
+		if (!EVP_PKEY_CTX_set1_id(ctx, "1234567812345678", 16))
+			goto err;
+	}
+	EVP_MD_CTX_set_pkey_ctx(mctx, ctx);
 	st = "EVP_get_digestbyname";
 	if (!(md = EVP_get_digestbyname(imaevm_params.hash_algo)))
 		goto err;
-	st = "EVP_PKEY_CTX_set_signature_md";
-	if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
+	st = "EVP_DigestSignInit";
+	if (!EVP_DigestSignInit(mctx, NULL, md, NULL, pkey))
+		goto err;
+	st = "EVP_DigestSignUpdate";
+	if (!EVP_DigestSignUpdate(mctx, hash, size))
 		goto err;
-	st = "EVP_PKEY_sign";
+	st = "EVP_DigestSignFinal";
 	sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
-	if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
+	if (!EVP_DigestSignFinal(mctx, hdr->sig, &sigsize))
 		goto err;
 	len = (int)sigsize;
 
@@ -964,6 +988,7 @@  err:
 			ERR_reason_error_string(ERR_peek_error()), st);
 		output_openssl_errors();
 	}
+	EVP_MD_CTX_free(mctx);
 	EVP_PKEY_CTX_free(ctx);
 	EVP_PKEY_free(pkey);
 	return len;