[PATCH 1/2] gost: add missing error reporting

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] gost: add missing error reporting

dbaryshkov
From: Dmitry Baryshkov <[hidden email]>

Add few more error reports to help debugging.

Sponsored by ROSA Linux.

Signed-off-by: Dmitry Baryshkov <[hidden email]>
---
 src/lib/libcrypto/gost/gostr341001_ameth.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c b/src/lib/libcrypto/gost/gostr341001_ameth.c
index 16295996dce7..be621d0185dd 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -96,15 +96,19 @@ decode_gost01_algor_params(EVP_PKEY *pkey, const unsigned char **p, int len)
  ec = pkey->pkey.gost;
  if (ec == NULL) {
  ec = GOST_KEY_new();
- if (ec == NULL)
+ if (ec == NULL) {
+ GOSTerror(ERR_R_MALLOC_FAILURE);
  return 0;
+ }
  if (EVP_PKEY_assign_GOST(pkey, ec) == 0)
  return 0;
  }
 
  group = EC_GROUP_new_by_curve_name(param_nid);
- if (group == NULL)
+ if (group == NULL) {
+ ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
  return 0;
+ }
  EC_GROUP_set_asn1_flag(group, OPENSSL_EC_NAMED_CURVE);
  if (GOST_KEY_set_group(ec, group) == 0) {
  EC_GROUP_free(group);
@@ -207,8 +211,10 @@ pub_decode_gost01(EVP_PKEY *pk, X509_PUBKEY *pub)
  return 0;
  }
  p = pval->data;
- if (decode_gost01_algor_params(pk, &p, pval->length) == 0)
+ if (decode_gost01_algor_params(pk, &p, pval->length) == 0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
  return 0;
+ }
 
  octet = d2i_ASN1_OCTET_STRING(NULL, &pubkey_buf, pub_len);
  if (octet == NULL) {
@@ -407,8 +413,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
  int ptype = V_ASN1_UNDEF;
  ASN1_STRING *pval = NULL;
 
- if (PKCS8_pkey_get0(&palg_obj, &pkey_buf, &priv_len, &palg, p8inf) == 0)
+ if (PKCS8_pkey_get0(&palg_obj, &pkey_buf, &priv_len, &palg, p8inf) == 0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
  return 0;
+ }
  (void)EVP_PKEY_assign_GOST(pk, NULL);
  X509_ALGOR_get0(NULL, &ptype, (const void **)&pval, palg);
  if (ptype != V_ASN1_SEQUENCE) {
@@ -416,8 +424,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
  return 0;
  }
  p = pval->data;
- if (decode_gost01_algor_params(pk, &p, pval->length) == 0)
+ if (decode_gost01_algor_params(pk, &p, pval->length) == 0) {
+ GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
  return 0;
+ }
  p = pkey_buf;
  if (V_ASN1_OCTET_STRING == *p) {
  /* New format - Little endian octet string */
--
2.25.1

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] gost: use ECerror to report EC errors

dbaryshkov
From: Dmitry Baryshkov <[hidden email]>

GOST code uses GOSTerror(EC_R_foo) to report several errors. Use
ECerror(EC_R_foo) instead to make error messages match error code.

Sponsored by ROSA Linux.

Signed-off-by: Dmitry Baryshkov <[hidden email]>
---
 src/lib/libcrypto/gost/gostr341001_ameth.c |  2 +-
 src/lib/libcrypto/gost/gostr341001_key.c   | 14 +++++++-------
 src/lib/libcrypto/gost/gostr341001_pmeth.c |  2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c b/src/lib/libcrypto/gost/gostr341001_ameth.c
index be621d0185dd..28ed55e6992f 100644
--- a/src/lib/libcrypto/gost/gostr341001_ameth.c
+++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
@@ -547,7 +547,7 @@ param_decode_gost01(EVP_PKEY *pkey, const unsigned char **pder, int derlen)
  }
  group = EC_GROUP_new_by_curve_name(nid);
  if (group == NULL) {
- GOSTerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
+ ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
  GOST_KEY_free(ec);
  return 0;
  }
diff --git a/src/lib/libcrypto/gost/gostr341001_key.c b/src/lib/libcrypto/gost/gostr341001_key.c
index 0af39f21bf33..74f8cab9d86c 100644
--- a/src/lib/libcrypto/gost/gostr341001_key.c
+++ b/src/lib/libcrypto/gost/gostr341001_key.c
@@ -121,7 +121,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
  return 0;
  }
  if (EC_POINT_is_at_infinity(key->group, key->pub_key) != 0) {
- GOSTerror(EC_R_POINT_AT_INFINITY);
+ ECerror(EC_R_POINT_AT_INFINITY);
  goto err;
  }
  if ((ctx = BN_CTX_new()) == NULL)
@@ -131,14 +131,14 @@ GOST_KEY_check_key(const GOST_KEY *key)
 
  /* testing whether the pub_key is on the elliptic curve */
  if (EC_POINT_is_on_curve(key->group, key->pub_key, ctx) == 0) {
- GOSTerror(EC_R_POINT_IS_NOT_ON_CURVE);
+ ECerror(EC_R_POINT_IS_NOT_ON_CURVE);
  goto err;
  }
  /* testing whether pub_key * order is the point at infinity */
  if ((order = BN_new()) == NULL)
  goto err;
  if (EC_GROUP_get_order(key->group, order, ctx) == 0) {
- GOSTerror(EC_R_INVALID_GROUP_ORDER);
+ ECerror(EC_R_INVALID_GROUP_ORDER);
  goto err;
  }
  if (EC_POINT_mul(key->group, point, NULL, key->pub_key, order,
@@ -147,7 +147,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
  goto err;
  }
  if (EC_POINT_is_at_infinity(key->group, point) == 0) {
- GOSTerror(EC_R_WRONG_ORDER);
+ ECerror(EC_R_WRONG_ORDER);
  goto err;
  }
  /*
@@ -156,7 +156,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
  */
  if (key->priv_key != NULL) {
  if (BN_cmp(key->priv_key, order) >= 0) {
- GOSTerror(EC_R_WRONG_ORDER);
+ ECerror(EC_R_WRONG_ORDER);
  goto err;
  }
  if (EC_POINT_mul(key->group, point, key->priv_key, NULL, NULL,
@@ -165,7 +165,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
  goto err;
  }
  if (EC_POINT_cmp(key->group, point, key->pub_key, ctx) != 0) {
- GOSTerror(EC_R_INVALID_PRIVATE_KEY);
+ ECerror(EC_R_INVALID_PRIVATE_KEY);
  goto err;
  }
  }
@@ -212,7 +212,7 @@ GOST_KEY_set_public_key_affine_coordinates(GOST_KEY *key, BIGNUM *x, BIGNUM *y)
  * out of range.
  */
  if (BN_cmp(x, tx) != 0 || BN_cmp(y, ty) != 0) {
- GOSTerror(EC_R_COORDINATES_OUT_OF_RANGE);
+ ECerror(EC_R_COORDINATES_OUT_OF_RANGE);
  goto err;
  }
  if (GOST_KEY_set_public_key(key, point) == 0)
diff --git a/src/lib/libcrypto/gost/gostr341001_pmeth.c b/src/lib/libcrypto/gost/gostr341001_pmeth.c
index 0eb1d873deaf..0e0cae99e3fc 100644
--- a/src/lib/libcrypto/gost/gostr341001_pmeth.c
+++ b/src/lib/libcrypto/gost/gostr341001_pmeth.c
@@ -246,7 +246,7 @@ pkey_gost01_sign(EVP_PKEY_CTX *ctx, unsigned char *sig, size_t *siglen,
  *siglen = 2 * size;
  return 1;
  } else if (*siglen < 2 * size) {
- GOSTerror(EC_R_BUFFER_TOO_SMALL);
+ ECerror(EC_R_BUFFER_TOO_SMALL);
  return 0;
  }
  if (tbs_len != 32 && tbs_len != 64) {
--
2.25.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] gost: add missing error reporting

kinichiro inoguchi
In reply to this post by dbaryshkov
I had checked this by portable build and all regresses passed.
I'm ok with this diff.


On Thu, Mar 26, 2020 at 09:28:01PM +0300, [hidden email] wrote:

> From: Dmitry Baryshkov <[hidden email]>
>
> Add few more error reports to help debugging.
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov <[hidden email]>
> ---
>  src/lib/libcrypto/gost/gostr341001_ameth.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c b/src/lib/libcrypto/gost/gostr341001_ameth.c
> index 16295996dce7..be621d0185dd 100644
> --- a/src/lib/libcrypto/gost/gostr341001_ameth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
> @@ -96,15 +96,19 @@ decode_gost01_algor_params(EVP_PKEY *pkey, const unsigned char **p, int len)
>   ec = pkey->pkey.gost;
>   if (ec == NULL) {
>   ec = GOST_KEY_new();
> - if (ec == NULL)
> + if (ec == NULL) {
> + GOSTerror(ERR_R_MALLOC_FAILURE);
>   return 0;
> + }
>   if (EVP_PKEY_assign_GOST(pkey, ec) == 0)
>   return 0;
>   }
>  
>   group = EC_GROUP_new_by_curve_name(param_nid);
> - if (group == NULL)
> + if (group == NULL) {
> + ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
>   return 0;
> + }
>   EC_GROUP_set_asn1_flag(group, OPENSSL_EC_NAMED_CURVE);
>   if (GOST_KEY_set_group(ec, group) == 0) {
>   EC_GROUP_free(group);
> @@ -207,8 +211,10 @@ pub_decode_gost01(EVP_PKEY *pk, X509_PUBKEY *pub)
>   return 0;
>   }
>   p = pval->data;
> - if (decode_gost01_algor_params(pk, &p, pval->length) == 0)
> + if (decode_gost01_algor_params(pk, &p, pval->length) == 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
>   return 0;
> + }
>  
>   octet = d2i_ASN1_OCTET_STRING(NULL, &pubkey_buf, pub_len);
>   if (octet == NULL) {
> @@ -407,8 +413,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
>   int ptype = V_ASN1_UNDEF;
>   ASN1_STRING *pval = NULL;
>  
> - if (PKCS8_pkey_get0(&palg_obj, &pkey_buf, &priv_len, &palg, p8inf) == 0)
> + if (PKCS8_pkey_get0(&palg_obj, &pkey_buf, &priv_len, &palg, p8inf) == 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
>   return 0;
> + }
>   (void)EVP_PKEY_assign_GOST(pk, NULL);
>   X509_ALGOR_get0(NULL, &ptype, (const void **)&pval, palg);
>   if (ptype != V_ASN1_SEQUENCE) {
> @@ -416,8 +424,10 @@ priv_decode_gost01(EVP_PKEY *pk, const PKCS8_PRIV_KEY_INFO *p8inf)
>   return 0;
>   }
>   p = pval->data;
> - if (decode_gost01_algor_params(pk, &p, pval->length) == 0)
> + if (decode_gost01_algor_params(pk, &p, pval->length) == 0) {
> + GOSTerror(GOST_R_BAD_KEY_PARAMETERS_FORMAT);
>   return 0;
> + }
>   p = pkey_buf;
>   if (V_ASN1_OCTET_STRING == *p) {
>   /* New format - Little endian octet string */
> --
> 2.25.1
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] gost: use ECerror to report EC errors

kinichiro inoguchi
In reply to this post by dbaryshkov
I had checked this by portable build and all regresses passed.
I'm ok with this diff.

On Thu, Mar 26, 2020 at 09:28:02PM +0300, [hidden email] wrote:

> From: Dmitry Baryshkov <[hidden email]>
>
> GOST code uses GOSTerror(EC_R_foo) to report several errors. Use
> ECerror(EC_R_foo) instead to make error messages match error code.
>
> Sponsored by ROSA Linux.
>
> Signed-off-by: Dmitry Baryshkov <[hidden email]>
> ---
>  src/lib/libcrypto/gost/gostr341001_ameth.c |  2 +-
>  src/lib/libcrypto/gost/gostr341001_key.c   | 14 +++++++-------
>  src/lib/libcrypto/gost/gostr341001_pmeth.c |  2 +-
>  3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/src/lib/libcrypto/gost/gostr341001_ameth.c b/src/lib/libcrypto/gost/gostr341001_ameth.c
> index be621d0185dd..28ed55e6992f 100644
> --- a/src/lib/libcrypto/gost/gostr341001_ameth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_ameth.c
> @@ -547,7 +547,7 @@ param_decode_gost01(EVP_PKEY *pkey, const unsigned char **pder, int derlen)
>   }
>   group = EC_GROUP_new_by_curve_name(nid);
>   if (group == NULL) {
> - GOSTerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
> + ECerror(EC_R_EC_GROUP_NEW_BY_NAME_FAILURE);
>   GOST_KEY_free(ec);
>   return 0;
>   }
> diff --git a/src/lib/libcrypto/gost/gostr341001_key.c b/src/lib/libcrypto/gost/gostr341001_key.c
> index 0af39f21bf33..74f8cab9d86c 100644
> --- a/src/lib/libcrypto/gost/gostr341001_key.c
> +++ b/src/lib/libcrypto/gost/gostr341001_key.c
> @@ -121,7 +121,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
>   return 0;
>   }
>   if (EC_POINT_is_at_infinity(key->group, key->pub_key) != 0) {
> - GOSTerror(EC_R_POINT_AT_INFINITY);
> + ECerror(EC_R_POINT_AT_INFINITY);
>   goto err;
>   }
>   if ((ctx = BN_CTX_new()) == NULL)
> @@ -131,14 +131,14 @@ GOST_KEY_check_key(const GOST_KEY *key)
>  
>   /* testing whether the pub_key is on the elliptic curve */
>   if (EC_POINT_is_on_curve(key->group, key->pub_key, ctx) == 0) {
> - GOSTerror(EC_R_POINT_IS_NOT_ON_CURVE);
> + ECerror(EC_R_POINT_IS_NOT_ON_CURVE);
>   goto err;
>   }
>   /* testing whether pub_key * order is the point at infinity */
>   if ((order = BN_new()) == NULL)
>   goto err;
>   if (EC_GROUP_get_order(key->group, order, ctx) == 0) {
> - GOSTerror(EC_R_INVALID_GROUP_ORDER);
> + ECerror(EC_R_INVALID_GROUP_ORDER);
>   goto err;
>   }
>   if (EC_POINT_mul(key->group, point, NULL, key->pub_key, order,
> @@ -147,7 +147,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
>   goto err;
>   }
>   if (EC_POINT_is_at_infinity(key->group, point) == 0) {
> - GOSTerror(EC_R_WRONG_ORDER);
> + ECerror(EC_R_WRONG_ORDER);
>   goto err;
>   }
>   /*
> @@ -156,7 +156,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
>   */
>   if (key->priv_key != NULL) {
>   if (BN_cmp(key->priv_key, order) >= 0) {
> - GOSTerror(EC_R_WRONG_ORDER);
> + ECerror(EC_R_WRONG_ORDER);
>   goto err;
>   }
>   if (EC_POINT_mul(key->group, point, key->priv_key, NULL, NULL,
> @@ -165,7 +165,7 @@ GOST_KEY_check_key(const GOST_KEY *key)
>   goto err;
>   }
>   if (EC_POINT_cmp(key->group, point, key->pub_key, ctx) != 0) {
> - GOSTerror(EC_R_INVALID_PRIVATE_KEY);
> + ECerror(EC_R_INVALID_PRIVATE_KEY);
>   goto err;
>   }
>   }
> @@ -212,7 +212,7 @@ GOST_KEY_set_public_key_affine_coordinates(GOST_KEY *key, BIGNUM *x, BIGNUM *y)
>   * out of range.
>   */
>   if (BN_cmp(x, tx) != 0 || BN_cmp(y, ty) != 0) {
> - GOSTerror(EC_R_COORDINATES_OUT_OF_RANGE);
> + ECerror(EC_R_COORDINATES_OUT_OF_RANGE);
>   goto err;
>   }
>   if (GOST_KEY_set_public_key(key, point) == 0)
> diff --git a/src/lib/libcrypto/gost/gostr341001_pmeth.c b/src/lib/libcrypto/gost/gostr341001_pmeth.c
> index 0eb1d873deaf..0e0cae99e3fc 100644
> --- a/src/lib/libcrypto/gost/gostr341001_pmeth.c
> +++ b/src/lib/libcrypto/gost/gostr341001_pmeth.c
> @@ -246,7 +246,7 @@ pkey_gost01_sign(EVP_PKEY_CTX *ctx, unsigned char *sig, size_t *siglen,
>   *siglen = 2 * size;
>   return 1;
>   } else if (*siglen < 2 * size) {
> - GOSTerror(EC_R_BUFFER_TOO_SMALL);
> + ECerror(EC_R_BUFFER_TOO_SMALL);
>   return 0;
>   }
>   if (tbs_len != 32 && tbs_len != 64) {
> --
> 2.25.1
>

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] gost: add missing error reporting

dbaryshkov
In reply to this post by kinichiro inoguchi
Hello,

сб, 28 мар. 2020 г. в 17:20, Kinichiro Inoguchi <[hidden email]>:
>
> I had checked this by portable build and all regresses passed.
> I'm ok with this diff.

Thank you! Any further actions required from my side?

> On Thu, Mar 26, 2020 at 09:28:01PM +0300, [hidden email] wrote:
> > From: Dmitry Baryshkov <[hidden email]>
> >
> > Add few more error reports to help debugging.
> >
> > Sponsored by ROSA Linux.
> >
> > Signed-off-by: Dmitry Baryshkov <[hidden email]>
> > ---
> >  src/lib/libcrypto/gost/gostr341001_ameth.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)

--
With best wishes
Dmitry