Browse Source

make pkcs#1 decode functions constant-time

as proposed in RFC 3447 only one error return code is used when there are
errors while decoding the pkcs#1 format.
also, all steps are executed and only the "output" is skipped if something
went wrong.

Sorry this could break backwards compatibility, since there's no more
BUFFER_OVERFLOW messaging.
Former error-handling code could also be affected because now there's only
OK as return code in cases where "res" is also set to '1'.
Steffen Jaeckel 11 years ago
parent
commit
1e9e98aa0d
2 changed files with 30 additions and 33 deletions
  1. 19 19
      src/pk/pkcs1/pkcs_1_oaep_decode.c
  2. 11 14
      src/pk/pkcs1/pkcs_1_v1_5_decode.c

+ 19 - 19
src/pk/pkcs1/pkcs_1_oaep_decode.c

@@ -28,7 +28,7 @@
    @param out              [out] Destination of decoding
    @param outlen           [in/out] The max size and resulting size of the decoding
    @param res              [out] Result of decoding, 1==valid, 0==invalid
-   @return CRYPT_OK if successful (even if invalid)
+   @return CRYPT_OK if successful
 */
 int pkcs_1_oaep_decode(const unsigned char *msg,    unsigned long msglen,
                        const unsigned char *lparam, unsigned long lparamlen,
@@ -38,7 +38,7 @@ int pkcs_1_oaep_decode(const unsigned char *msg,    unsigned long msglen,
 {
    unsigned char *DB, *seed, *mask;
    unsigned long hLen, x, y, modulus_len;
-   int           err;
+   int           err, ret;
 
    LTC_ARGCHK(msg    != NULL);
    LTC_ARGCHK(out    != NULL);
@@ -85,10 +85,12 @@ int pkcs_1_oaep_decode(const unsigned char *msg,    unsigned long msglen,
 
     */
 
+   err = CRYPT_OK;
+   ret = CRYPT_OK;
+
    /* must have leading 0x00 byte */
    if (msg[0] != 0x00) {
-      err = CRYPT_OK;
-      goto LBL_ERR;
+      ret = CRYPT_INVALID_PACKET;
    }
 
    /* now read the masked seed */
@@ -137,8 +139,7 @@ int pkcs_1_oaep_decode(const unsigned char *msg,    unsigned long msglen,
 
    /* compare the lhash'es */
    if (mem_neq(seed, DB, hLen) != 0) {
-      err = CRYPT_OK;
-      goto LBL_ERR;
+      ret = CRYPT_INVALID_PACKET;
    }
 
    /* now zeroes before a 0x01 */
@@ -146,28 +147,27 @@ int pkcs_1_oaep_decode(const unsigned char *msg,    unsigned long msglen,
       /* step... */
    }
 
-   /* error out if wasn't 0x01 */
+   /* error if wasn't 0x01 */
    if (x == (modulus_len - hLen - 1) || DB[x] != 0x01) {
-      err = CRYPT_INVALID_PACKET;
-      goto LBL_ERR;
+      ret = CRYPT_INVALID_PACKET;
    }
 
    /* rest is the message (and skip 0x01) */
    if ((modulus_len - hLen - 1 - ++x) > *outlen) {
-      *outlen = modulus_len - hLen - 1 - x;
-      err = CRYPT_BUFFER_OVERFLOW;
-      goto LBL_ERR;
+      ret = CRYPT_INVALID_PACKET;
    }
 
-   /* copy message */
-   *outlen = modulus_len - hLen - 1 - x;
-   XMEMCPY(out, DB + x, modulus_len - hLen - 1 - x);
-   x += modulus_len - hLen - 1;
+   if (ret == CRYPT_OK) {
+      /* copy message */
+      *outlen = modulus_len - hLen - 1 - x;
+      XMEMCPY(out, DB + x, modulus_len - hLen - 1 - x);
+      x += modulus_len - hLen - 1;
 
-   /* valid packet */
-   *res = 1;
+      /* valid packet */
+      *res = 1;
+   }
+   err = ret;
 
-   err = CRYPT_OK;
 LBL_ERR:
 #ifdef LTC_CLEAN_STACK
    zeromem(DB,   modulus_len);

+ 11 - 14
src/pk/pkcs1/pkcs_1_v1_5_decode.c

@@ -27,7 +27,7 @@
  *  @param outlen           [in/out] The max size and resulting size of the decoding
  *  @param is_valid         [out] Boolean whether the padding was valid
  *
- *  @return CRYPT_OK if successful (even if invalid)
+ *  @return CRYPT_OK if successful
  */
 int pkcs_1_v1_5_decode(const unsigned char *msg,
                              unsigned long  msglen,
@@ -51,11 +51,12 @@ int pkcs_1_v1_5_decode(const unsigned char *msg,
     return CRYPT_PK_INVALID_SIZE;
   }
 
+  result = CRYPT_OK;
+
   /* separate encoded message */
 
   if ((msg[0] != 0x00) || (msg[1] != (unsigned char)block_type)) {
     result = CRYPT_INVALID_PACKET;
-    goto bail;
   }
 
   if (block_type == LTC_PKCS_1_EME) {
@@ -69,7 +70,6 @@ int pkcs_1_v1_5_decode(const unsigned char *msg,
       /* There was no octet with hexadecimal value 0x00 to separate ps from m.
        */
       result = CRYPT_INVALID_PACKET;
-      goto bail;
     }
   } else {
     for (i = 2; i < modulus_len - 1; i++) {
@@ -80,7 +80,6 @@ int pkcs_1_v1_5_decode(const unsigned char *msg,
     if (msg[i] != 0) {
       /* There was no octet with hexadecimal value 0x00 to separate ps from m. */
       result = CRYPT_INVALID_PACKET;
-      goto bail;
     }
 
     ps_len = i - 2;
@@ -91,22 +90,20 @@ int pkcs_1_v1_5_decode(const unsigned char *msg,
     /* The length of ps is less than 8 octets.
      */
     result = CRYPT_INVALID_PACKET;
-    goto bail;
   }
 
   if (*outlen < (msglen - (2 + ps_len + 1))) {
-    *outlen = msglen - (2 + ps_len + 1);
-    result = CRYPT_BUFFER_OVERFLOW;
-    goto bail;
+    result = CRYPT_INVALID_PACKET;
   }
 
-  *outlen = (msglen - (2 + ps_len + 1));
-  XMEMCPY(out, &msg[2 + ps_len + 1], *outlen);
+  if (result == CRYPT_OK) {
+     *outlen = (msglen - (2 + ps_len + 1));
+     XMEMCPY(out, &msg[2 + ps_len + 1], *outlen);
+
+     /* valid packet */
+     *is_valid = 1;
+  }
 
-  /* valid packet */
-  *is_valid = 1;
-  result    = CRYPT_OK;
-bail:
   return result;
 } /* pkcs_1_v1_5_decode */