Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

2013-08-27 Thread Jiri Kosina
On Mon, 26 Aug 2013, Pavel Machek wrote:

> > > > Due to RSA_I2OSP is not only used by signature verification path but 
> > > > also used
> > > > in signature generation path. So, separate the length checking of octet 
> > > > string
> > > > because it's not for generate 0x00 0x01 leading string when used in 
> > > > signature
> > > > generation.
> > > > 
> > > > Reviewed-by: Jiri Kosina 
> > > > Signed-off-by: Lee, Chun-Yi 
> > > 
> > > > +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> > > > +{
> > > > +   unsigned x_size;
> > > > +   unsigned X_size;
> > > > +   u8 *X = NULL;
> > > 
> > > Is this kernel code or entry into obfuscated C code contest? This is not 
> > > funny.
> > > 
> > The small "x" is the input integer that will transfer to big "X" that is
> > a octet sting. 
> > 
> > Sorry for I direct give variable name to match with spec, maybe I need
> > use big_X or
> 
> Having variables that differ only in case is confusing. Actually
> RSA_I2OSP is not a good name for function, either.
> 
> If it converts x into X, perhaps you can name one input and second output?

The variable naming is according to spec, and I believe it makes sense to 
keep it so, no matter how stupid the naming in the spec might be.

Otherwise you have to do mental remapping when looking at the code and the 
spec at the same time, which is very inconvenient.

Would a comment next to the variable declaration, that would point this 
fact out, be satisfactory for you?

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

2013-08-26 Thread Pavel Machek
Hi!

> > > Due to RSA_I2OSP is not only used by signature verification path but also 
> > > used
> > > in signature generation path. So, separate the length checking of octet 
> > > string
> > > because it's not for generate 0x00 0x01 leading string when used in 
> > > signature
> > > generation.
> > > 
> > > Reviewed-by: Jiri Kosina 
> > > Signed-off-by: Lee, Chun-Yi 
> > 
> > > +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> > > +{
> > > + unsigned x_size;
> > > + unsigned X_size;
> > > + u8 *X = NULL;
> > 
> > Is this kernel code or entry into obfuscated C code contest? This is not 
> > funny.
> > 
> The small "x" is the input integer that will transfer to big "X" that is
> a octet sting. 
> 
> Sorry for I direct give variable name to match with spec, maybe I need
> use big_X or

Having variables that differ only in case is confusing. Actually
RSA_I2OSP is not a good name for function, either.

If it converts x into X, perhaps you can name one input and second output?

> Do you have good suggest for the naming?

Not really, sorry.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

2013-08-26 Thread joeyli
於 日,2013-08-25 於 18:01 +0200,Pavel Machek 提到:
> On Thu 2013-08-22 19:01:42, Lee, Chun-Yi wrote:
> > Due to RSA_I2OSP is not only used by signature verification path but also 
> > used
> > in signature generation path. So, separate the length checking of octet 
> > string
> > because it's not for generate 0x00 0x01 leading string when used in 
> > signature
> > generation.
> > 
> > Reviewed-by: Jiri Kosina 
> > Signed-off-by: Lee, Chun-Yi 
> 
> > +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> > +{
> > +   unsigned x_size;
> > +   unsigned X_size;
> > +   u8 *X = NULL;
> 
> Is this kernel code or entry into obfuscated C code contest? This is not 
> funny.
> 
>   Pavel

The small "x" is the input integer that will transfer to big "X" that is
a octet sting. 

Sorry for I direct give variable name to match with spec, maybe I need
use big_X or

Do you have good suggest for the naming?


Thanks a lot!
Joey Lee


--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

2013-08-25 Thread Pavel Machek
On Thu 2013-08-22 19:01:42, Lee, Chun-Yi wrote:
> Due to RSA_I2OSP is not only used by signature verification path but also used
> in signature generation path. So, separate the length checking of octet string
> because it's not for generate 0x00 0x01 leading string when used in signature
> generation.
> 
> Reviewed-by: Jiri Kosina 
> Signed-off-by: Lee, Chun-Yi 

> +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> +{
> + unsigned x_size;
> + unsigned X_size;
> + u8 *X = NULL;

Is this kernel code or entry into obfuscated C code contest? This is not funny.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

2013-08-22 Thread Lee, Chun-Yi
Due to RSA_I2OSP is not only used by signature verification path but also used
in signature generation path. So, separate the length checking of octet string
because it's not for generate 0x00 0x01 leading string when used in signature
generation.

Reviewed-by: Jiri Kosina 
Signed-off-by: Lee, Chun-Yi 
---
 crypto/asymmetric_keys/rsa.c |   33 -
 1 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 6996ff7..c26ae77 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -121,12 +121,30 @@ static int RSAVP1(const struct public_key *key, MPI s, 
MPI *_m)
 /*
  * Integer to Octet String conversion [RFC3447 sec 4.1]
  */
-static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
+static int _RSA_I2OSP(MPI x, unsigned *X_size, u8 **_X)
 {
-   unsigned X_size, x_size;
int X_sign;
u8 *X;
 
+   X = mpi_get_buffer(x, X_size, &X_sign);
+   if (!X)
+   return -ENOMEM;
+   if (X_sign < 0) {
+   kfree(X);
+   return -EBADMSG;
+   }
+
+   *_X = X;
+   return 0;
+}
+
+static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
+{
+   unsigned x_size;
+   unsigned X_size;
+   u8 *X = NULL;
+   int ret;
+
/* Make sure the string is the right length.  The number should begin
 * with { 0x00, 0x01, ... } so we have to account for 15 leading zero
 * bits not being reported by MPI.
@@ -136,13 +154,10 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
if (x_size != xLen * 8 - 15)
return -ERANGE;
 
-   X = mpi_get_buffer(x, &X_size, &X_sign);
-   if (!X)
-   return -ENOMEM;
-   if (X_sign < 0) {
-   kfree(X);
-   return -EBADMSG;
-   }
+   ret = _RSA_I2OSP(x, &X_size, &X);
+   if (ret < 0)
+   return ret;
+
if (X_size != xLen - 1) {
kfree(X);
return -EBADMSG;
-- 
1.6.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html