Skip to content

Commit

Permalink
Constant time fixes for C_Decrypt return code handling
Browse files Browse the repository at this point in the history 
Return code handling of C_Decrypt, C_DecryptUpdate, and C_DecryptFinal must
be performed in a constant time manner for RSA mechanisms. Otherwise it
may cause a timing side channel that may be used to perform a Bleichenbacher
style attack.

Handling of error situations with CKR_BUFFER_TOO_SMALL or size-query calls,
where the output buffer is NULL and the required size of the output buffer
is to be returned, do not need to be performed in constant time, since
these cases are shortcut anyway, and the result is only dependent on the
modulus size of the RSA key (which is public information anyway).

Signed-off-by: Ingo Franzki <ifranzki@linux.ibm.com>
  • Loading branch information
@ifranzki
ifranzki committed Jan 19, 2024
1 parent bae58c2 commit 5b7408f
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 31 deletions.
39 changes: 33 additions & 6 deletions usr/lib/common/new_host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#include "trace.h"
#include "slotmgr.h"
#include "attributes.h"
#include "constant_time.h"

#include "../api/apiproto.h"
#include "../api/policy.h"
Expand Down Expand Up @@ -2345,6 +2346,7 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2377,11 +2379,19 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
rc = decr_mgr_decrypt(tokdata, sess, length_only, &sess->decr_ctx,
pEncryptedData, ulEncryptedDataLen, pData,
pulDataLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("decr_mgr_decrypt() failed.\n");

done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -2404,6 +2414,7 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2436,11 +2447,18 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
rc = decr_mgr_decrypt_update(tokdata, sess, length_only,
&sess->decr_ctx, pEncryptedPart,
ulEncryptedPartLen, pPart, pulPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("decr_mgr_decrypt_update() failed.\n");

done:
if (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL && sess != NULL) {
/* (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL */
mask = ~constant_time_eq(rc, CKR_OK);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -2462,6 +2480,7 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2493,11 +2512,19 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,

rc = decr_mgr_decrypt_final(tokdata, sess, length_only, &sess->decr_ctx,
pLastPart, pulLastPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("decr_mgr_decrypt_final() failed.\n");

done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand Down
30 changes: 18 additions & 12 deletions usr/lib/ep11_stdll/ep11_specific.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9596,10 +9596,12 @@ CK_RV ep11tok_decrypt_final(STDLL_TokData_t * tokdata, SESSION * session,
rc = constant_time_select(constant_time_eq(rc, CKR_OK),
ep11_error_to_pkcs11_error(rc, session),
rc);
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
if (!is_rsa_mechanism(ctx->mech.mechanism)) {
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
}
}

done:
Expand Down Expand Up @@ -9655,10 +9657,12 @@ CK_RV ep11tok_decrypt(STDLL_TokData_t * tokdata, SESSION * session,
rc = constant_time_select(constant_time_eq(rc, CKR_OK),
ep11_error_to_pkcs11_error(rc, session),
rc);
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
if (!is_rsa_mechanism(ctx->mech.mechanism)) {
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
}
}

done:
Expand Down Expand Up @@ -9720,10 +9724,12 @@ CK_RV ep11tok_decrypt_update(STDLL_TokData_t * tokdata, SESSION * session,
rc = constant_time_select(constant_time_eq(rc, CKR_OK),
ep11_error_to_pkcs11_error(rc, session),
rc);
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
if (!is_rsa_mechanism(ctx->mech.mechanism)) {
if (rc != CKR_OK) {
TRACE_ERROR("%s rc=0x%lx\n", __func__, rc);
} else {
TRACE_INFO("%s rc=0x%lx\n", __func__, rc);
}
}

done:
Expand Down
45 changes: 38 additions & 7 deletions usr/lib/ep11_stdll/new_host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "slotmgr.h"
#include "attributes.h"
#include "ep11_specific.h"
#include "constant_time.h"

#include "../api/apiproto.h"
#include "../api/policy.h"
Expand Down Expand Up @@ -2466,6 +2467,7 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2513,17 +2515,29 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
length_only, sess->decr_ctx.key,
pEncryptedData, ulEncryptedDataLen,
pData, pulDataLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("ep11tok_decrypt_single() failed.\n");
} else {
rc = ep11tok_decrypt(tokdata, sess, pEncryptedData, ulEncryptedDataLen,
pData, pulDataLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("ep11tok_decrypt() failed.\n");
}

done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -2545,6 +2559,7 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
{
SESSION *sess = NULL;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2596,11 +2611,18 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,

rc = ep11tok_decrypt_update(tokdata, sess, pEncryptedPart,
ulEncryptedPartLen, pPart, pulPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("ep11tok_decrypt_update() failed.\n");

done:
if (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL && sess != NULL) {
/* (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL */
mask = ~constant_time_eq(rc, CKR_OK);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -2622,6 +2644,7 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -2670,10 +2693,18 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
}

rc = ep11tok_decrypt_final(tokdata, sess, pLastPart, pulLastPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("ep11tok_decrypt_final() failed.\n");
done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand Down
40 changes: 34 additions & 6 deletions usr/lib/icsf_stdll/new_host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#include "slotmgr.h"
#include "attributes.h"
#include "icsf_specific.h"
#include "constant_time.h"

#include "../api/apiproto.h"
#include "../api/policy.h"

Expand Down Expand Up @@ -1768,6 +1770,7 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -1801,11 +1804,19 @@ CK_RV SC_Decrypt(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,

rc = icsftok_decrypt(tokdata, sess, pEncryptedData, ulEncryptedDataLen,
pData, pulDataLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("icsftok_decrypt() failed.\n");

done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -1827,6 +1838,7 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
{
SESSION *sess = NULL;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -1857,11 +1869,18 @@ CK_RV SC_DecryptUpdate(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,

rc = icsftok_decrypt_update(tokdata, sess, pEncryptedPart,
ulEncryptedPartLen, pPart, pulPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("icsftok_decrypt_update() failed.\n");

done:
if (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL && sess != NULL) {
/* (rc != CKR_OK && rc != CKR_BUFFER_TOO_SMALL */
mask = ~constant_time_eq(rc, CKR_OK);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand All @@ -1883,6 +1902,7 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
SESSION *sess = NULL;
CK_BBOOL length_only = FALSE;
CK_RV rc = CKR_OK;
unsigned int mask;

if (tokdata->initialized == FALSE) {
TRACE_ERROR("%s\n", ock_err(ERR_CRYPTOKI_NOT_INITIALIZED));
Expand Down Expand Up @@ -1915,10 +1935,18 @@ CK_RV SC_DecryptFinal(STDLL_TokData_t *tokdata, ST_SESSION_HANDLE *sSession,
length_only = TRUE;

rc = icsftok_decrypt_final(tokdata, sess, pLastPart, pulLastPartLen);
if (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK)
/* (!is_rsa_mechanism(sess->decr_ctx.mech.mechanism) && rc != CKR_OK) */
mask = ~constant_time_is_zero(
is_rsa_mechanism(sess->decr_ctx.mech.mechanism));
mask &= ~constant_time_eq(rc, CKR_OK);
if (mask)
TRACE_DEVEL("icsftok_decrypt_final() failed.\n");
done:
if (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) {
/* (rc != CKR_BUFFER_TOO_SMALL && (rc != CKR_OK || length_only != TRUE)) */
mask = ~constant_time_eq(rc, CKR_OK);
mask |= constant_time_is_zero(length_only);
mask &= ~constant_time_eq(rc, CKR_BUFFER_TOO_SMALL);
if (mask) {
if (sess)
decr_mgr_cleanup(tokdata, sess, &sess->decr_ctx);
}
Expand Down

0 comments on commit 5b7408f

Please sign in to comment.