From fa8466d190f0e5ff0270cd758aacb71160603055 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 8 Oct 2021 15:08:38 +0200 Subject: [PATCH 1/5] add constant time comparison function --- src/sancus_support/sm_support.h | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index aca2378..2a5cb2e 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -312,6 +312,29 @@ sm_id sancus_enable_wrapped(struct SancusModule* sm, unsigned nonce, void* tag); #define always_inline static inline __attribute__((always_inline)) /** + * Performs constant time comparison between to buffers + * Returns 0 if the first `n` bytes of the two buffers are equal, -1 otherwise + */ +always_inline int constant_time_cmp(const unsigned char *x_, + const unsigned char *y_, + const unsigned int n) +{ + const volatile unsigned char *volatile x = + (const volatile unsigned char *volatile) x_; + const volatile unsigned char *volatile y = + (const volatile unsigned char *volatile) y_; + volatile unsigned int d = 0U; + int i; + + for (i = 0; i < n; i++) { + d |= x[i] ^ y[i]; + } + + return (1 & ((d - 1) >> 8)) - 1; +} + +/** + * Disable the protection of the calling module. * DANGEROUS: Disable the protection of the calling module. * * NOTE: On Sancus cores that support interruptible enclaves, the @@ -582,7 +605,7 @@ always_inline sm_id sancus_get_self_id(void) * The calling module is defined as the previously executing module. That is, * the module that entered the currently executing module. * - * @note This function is implemented as a compiler intrinsic to + * @note This function is implemented as a compiler intrinsic to * be able to read it from the SM's SSA frame. */ sm_id sancus_get_caller_id(void); From 7b26b352335dee85764602f3fcd9f12d091602f8 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 8 Oct 2021 15:16:46 +0200 Subject: [PATCH 2/5] move `sancus_tag` functions up, add `sancus_untag` functions I changed the order of these functions so that we could call `sancus_untag_with_key` inside sancus_`unwrap_with_key`. To do so, the former needs to be declared before the latter --- src/sancus_support/sm_support.h | 63 +++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 2a5cb2e..9d17595 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -504,6 +504,51 @@ always_inline int sancus_wrap(const void* ad, size_t ad_len, return sancus_wrap_with_key(NULL, ad, ad_len, body, body_len, cipher, tag); } +/** + * The same as sancus_wrap_with_key() but only produces the MAC of the message. + */ +always_inline int sancus_tag_with_key(const void* key, + const void* body, size_t body_len, + void* tag) +{ + return sancus_wrap_with_key(key, body, body_len, NULL, 0, NULL, tag); +} + +/** + * The same as sancus_wrap() but only produces the MAC of the message. + */ +always_inline int sancus_tag(const void* body, size_t body_len, void* tag) +{ + return sancus_wrap(body, body_len, NULL, 0, NULL, tag); +} + +/** + * Verify if the MAC computed over `body` matches with the content of `tag` + */ +always_inline int sancus_untag_with_key(const void* key, const void* body, + size_t body_len, const void* tag) +{ + unsigned char computed_tag[SANCUS_TAG_SIZE]; + + // compute MAC over `body` + if ( !sancus_tag_with_key(key, body, body_len, computed_tag) ) { + return 0; + } + + // compare MAC with provided reference `tag` + return constant_time_cmp(tag, computed_tag, SANCUS_TAG_SIZE) == 0; +} + +/** +* Verify if the MAC computed over `body` matches with the content of `tag` +* +* This is the same as sancus_untag_with_key using the key of the caller module. +*/ +always_inline int sancus_untag(const void* body, size_t body_len, const void* tag) +{ + return sancus_untag_with_key(NULL, body, body_len, tag); +} + /** * Unwrap a message using the Sancus authenticated encryption features. * @@ -555,24 +600,6 @@ always_inline int sancus_unwrap(const void* ad, size_t ad_len, tag, body); } -/** - * The same as sancus_wrap() but only produces the MAC of the message. - */ -always_inline int sancus_tag(const void* body, size_t body_len, void* tag) -{ - return sancus_wrap(body, body_len, NULL, 0, NULL, tag); -} - -/** - * The same as sancus_wrap_with_key() but only produces the MAC of the message. - */ -always_inline int sancus_tag_with_key(const void* key, - const void* body, size_t body_len, - void* tag) -{ - return sancus_wrap_with_key(key, body, body_len, NULL, 0, NULL, tag); -} - /** * Get the Sancus ID of the module loaded at @p addr. */ From 666f960a66a30d2b3e899ee97add2ab6071cfe39 Mon Sep 17 00:00:00 2001 From: Gianluca Scopelliti Date: Fri, 8 Oct 2021 15:19:25 +0200 Subject: [PATCH 3/5] add check on cipher_len inside `sancus_unwrap_with_key` Normally, `sancus_unwrap_with_key` always fails if cipher_len is zero. This commit solves this issue by leveraging `sancus_untag_with_key` --- src/sancus_support/sm_support.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 9d17595..2b36635 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -560,6 +560,11 @@ always_inline int sancus_unwrap_with_key(const void* key, size_t cipher_len, const void* tag, void* body) { + // fix: if cipher_len is zero, just compare the MACs using sancus_untag + if(cipher_len == 0) { + return sancus_untag_with_key(key, ad, ad_len, tag); + } + void* ad_end = (char*)ad + ad_len; void* cipher_end = (char*)cipher + cipher_len; int ret; From 368dc00801bd39ba3fe37921e4a18849bc3b5f70 Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Wed, 20 Oct 2021 11:58:02 +0200 Subject: [PATCH 4/5] Apply suggestions from code review --- src/sancus_support/sm_support.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index 2b36635..f5010cb 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -314,6 +314,7 @@ sm_id sancus_enable_wrapped(struct SancusModule* sm, unsigned nonce, void* tag); /** * Performs constant time comparison between to buffers * Returns 0 if the first `n` bytes of the two buffers are equal, -1 otherwise + * NOTE: Function copied from NaCL's libsodium, re-use allowed under ISC license. */ always_inline int constant_time_cmp(const unsigned char *x_, const unsigned char *y_, @@ -536,7 +537,7 @@ always_inline int sancus_untag_with_key(const void* key, const void* body, } // compare MAC with provided reference `tag` - return constant_time_cmp(tag, computed_tag, SANCUS_TAG_SIZE) == 0; + return constant_time_cmp(tag, computed_tag, SANCUS_TAG_SIZE); } /** From 840d532d86a3bf4313c11d66ead662960b77484e Mon Sep 17 00:00:00 2001 From: Jo Van Bulck Date: Wed, 20 Oct 2021 12:07:00 +0200 Subject: [PATCH 5/5] Update src/sancus_support/sm_support.h --- src/sancus_support/sm_support.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sancus_support/sm_support.h b/src/sancus_support/sm_support.h index f5010cb..9881973 100644 --- a/src/sancus_support/sm_support.h +++ b/src/sancus_support/sm_support.h @@ -537,7 +537,7 @@ always_inline int sancus_untag_with_key(const void* key, const void* body, } // compare MAC with provided reference `tag` - return constant_time_cmp(tag, computed_tag, SANCUS_TAG_SIZE); + return (constant_time_cmp(tag, computed_tag, SANCUS_TAG_SIZE) == 0); } /**