From 1e9e98aa0d7801ce7e9429dba805bbf6cab2ed86 Mon Sep 17 00:00:00 2001
From: Steffen Jaeckel <s@jaeckel.eu>
Date: Thu, 13 Nov 2014 22:26:59 +0100
Subject: [PATCH] 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'.
---
 src/pk/pkcs1/pkcs_1_oaep_decode.c | 38 +++++++++++++++----------------
 src/pk/pkcs1/pkcs_1_v1_5_decode.c | 25 +++++++++-----------
 2 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/src/pk/pkcs1/pkcs_1_oaep_decode.c b/src/pk/pkcs1/pkcs_1_oaep_decode.c
index 4801e0e..6c4da54 100644
--- a/src/pk/pkcs1/pkcs_1_oaep_decode.c
+++ b/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);
diff --git a/src/pk/pkcs1/pkcs_1_v1_5_decode.c b/src/pk/pkcs1/pkcs_1_v1_5_decode.c
index 5afbb80..34bb434 100644
--- a/src/pk/pkcs1/pkcs_1_v1_5_decode.c
+++ b/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 */