blob: 795ee1ea626f4e89e3a0fb2ae3469123078da926 [file] [log] [blame]
Serge Bazanski72c1f2b2024-06-04 13:42:48 +00001From 98df8cd09ec7a5b91f05c665529ed6f579f231d9 Mon Sep 17 00:00:00 2001
2From: Serge Bazanski <serge@monogon.tech>
3Date: Tue, 4 Jun 2024 13:53:48 +0200
4Subject: [PATCH 5/6] boringssl compat: remove constant time flags (UNSAFE)
5
6OpenSSL has a quirky little API to mark bignums as 'secret' ie.
7'constant time' which is supposed to influence operations performed on
8them to be constant time.
9
10This API was tricky to use and caused security issues, so it was removed
11by BoringSSL.
12
13https://github.com/google/boringssl/commit/0a211dfe91588d2986a8735e1969dd9202a8b025
14
15Ideally we would replace all relevent BN_mod_exp calls with
16constant-time versions, but that's not trivial to do: the constant time
17versions of modular exponentiation and multiplicative inverse operations
18rely on Montgomery modular multiplication which seems to reduce the
19domain of the exponent to |0, N>. Unfortunately libtpms has plenty of
20eg. ModExp operations that work on exponents outside this range. OpenSSL
21seems to not have applied the constant time request to BN_mod_exp if
22that was the case, but BoringSSL refuses to perform constant time
23operations then.
24
25As I'm not a cryptographer and not able to fix this properly (or even
26fully reason about this), I'm just adding a big fat warning to be shown
27whenever potentially unsafe operations are now performed.
28---
29 src/monogon_unsafe.c | 28 ++++++++++++++++++++++++++
30 src/tpm2/crypto/openssl/BnToOsslMath.c | 10 +++++----
31 src/tpm2/crypto/openssl/ExpDCache.c | 5 +++--
32 3 files changed, 37 insertions(+), 6 deletions(-)
33 create mode 100644 src/monogon_unsafe.c
34
35diff --git a/src/monogon_unsafe.c b/src/monogon_unsafe.c
36new file mode 100644
37index 0000000..abaef79
38--- /dev/null
39+++ b/src/monogon_unsafe.c
40@@ -0,0 +1,28 @@
41+#include <stdio.h>
42+#include <stdlib.h>
43+
44+// This library was built against BoringSSL without the BN constant time API,
45+// thus all cryptographic operations are performed timing-unsafe which might
46+// lead to side channel leaks. This is fine for Monogon's usecase (swtpm in
47+// tests) but this code must not end up being used to secure any real systems.
48+//
49+// Note: I am not sure this code was safe from side channels in the first
50+// place. See RsaPrivateKeyOp and compare with BoringSSL's
51+// rsa_default_private_transform implementation... ~q3k
52+
53+static int _warned = 0;
54+
55+void monogon_warn_unsafe_library(void)
56+{
57+ if (getenv("MONOGON_LIBTPMS_ACKNOWLEDGE_UNSAFE") != NULL) {
58+ return;
59+ }
60+ if (_warned) {
61+ return;
62+ }
63+ _warned = 1;
64+ fprintf(stderr, "--------------------------------------------------------------------------------\n");
65+ fprintf(stderr, "WARNING: This fork of libtpms/swtpm contains UNSAFE cryptographic operations and\n");
66+ fprintf(stderr, " MUST NOT be used to secure sensitive data.\n");
67+ fprintf(stderr, "--------------------------------------------------------------------------------\n");
68+}
69diff --git a/src/tpm2/crypto/openssl/BnToOsslMath.c b/src/tpm2/crypto/openssl/BnToOsslMath.c
70index 7d13ce8..54d5916 100644
71--- a/src/tpm2/crypto/openssl/BnToOsslMath.c
72+++ b/src/tpm2/crypto/openssl/BnToOsslMath.c
73@@ -83,6 +83,8 @@
74 //#include "Tpm.h"
75 #include "BnOssl.h"
76
77+extern void monogon_warn_unsafe_library();
78+
79 #ifdef MATH_LIB_OSSL
80 # include "BnToOsslMath_fp.h"
81
82@@ -133,6 +135,7 @@ BOOL OsslToTpmBn(bigNum bn, const BIGNUM* osslBn) // libtpms: added 'const'
83 // function prototype. Instead, use BnNewVariable().
84 BIGNUM* BigInitialized(BIGNUM* toInit, bigConst initializer)
85 {
86+ monogon_warn_unsafe_library();
87 #if 1 // libtpms: added begin
88 BIGNUM *_toInit;
89 unsigned char buffer[LARGEST_NUMBER + 1];
90@@ -147,7 +150,6 @@ BIGNUM* BigInitialized(BIGNUM* toInit, bigConst initializer)
91 #if 1 // libtpms: added begin
92 BnToBytes(initializer, buffer, &buffer_len); /* TPM to bin */
93 _toInit = BN_bin2bn(buffer, buffer_len, NULL); /* bin to ossl */
94- BN_set_flags(_toInit, BN_FLG_CONSTTIME);
95 BN_copy(toInit, _toInit);
96 BN_clear_free(_toInit);
97 #else // libtpms: added end
98@@ -355,13 +357,13 @@ LIB_EXPORT BOOL BnGcd(bigNum gcd, // OUT: the common divisor
99 bigConst number2 // IN:
100 )
101 {
102+ monogon_warn_unsafe_library();
103 OSSL_ENTER();
104 BIGNUM* bnGcd = BN_NEW();
105 BOOL OK = TRUE;
106 BIG_INITIALIZED(bn1, number1);
107 BIG_INITIALIZED(bn2, number2);
108 //
109- BN_set_flags(bn1, BN_FLG_CONSTTIME); // number1 is secret prime number
110 GOTO_ERROR_UNLESS(BN_gcd(bnGcd, bn1, bn2, CTX));
111 GOTO_ERROR_UNLESS(OsslToTpmBn(gcd, bnGcd));
112 goto Exit;
113@@ -387,6 +389,7 @@ LIB_EXPORT BOOL BnModExp(bigNum result, // OUT: the result
114 bigConst modulus // IN:
115 )
116 {
117+ monogon_warn_unsafe_library();
118 OSSL_ENTER();
119 BIGNUM* bnResult = BN_NEW();
120 BOOL OK = TRUE;
121@@ -394,7 +397,6 @@ LIB_EXPORT BOOL BnModExp(bigNum result, // OUT: the result
122 BIG_INITIALIZED(bnE, exponent);
123 BIG_INITIALIZED(bnM, modulus);
124 //
125- BN_set_flags(bnE, BN_FLG_CONSTTIME); // exponent may be private
126 GOTO_ERROR_UNLESS(BN_mod_exp(bnResult, bnN, bnE, bnM, CTX));
127 GOTO_ERROR_UNLESS(OsslToTpmBn(result, bnResult));
128 goto Exit;
129@@ -416,13 +418,13 @@ LIB_EXPORT BOOL BnModExp(bigNum result, // OUT: the result
130 // FALSE(0) failure in operation
131 LIB_EXPORT BOOL BnModInverse(bigNum result, bigConst number, bigConst modulus)
132 {
133+ monogon_warn_unsafe_library();
134 OSSL_ENTER();
135 BIGNUM* bnResult = BN_NEW();
136 BOOL OK = TRUE;
137 BIG_INITIALIZED(bnN, number);
138 BIG_INITIALIZED(bnM, modulus);
139 //
140- BN_set_flags(bnN, BN_FLG_CONSTTIME); // number may be private
141 GOTO_ERROR_UNLESS(BN_mod_inverse(bnResult, bnN, bnM, CTX) != NULL);
142 GOTO_ERROR_UNLESS(OsslToTpmBn(result, bnResult));
143 goto Exit;
144diff --git a/src/tpm2/crypto/openssl/ExpDCache.c b/src/tpm2/crypto/openssl/ExpDCache.c
145index 5aeaf14..133e9ed 100644
146--- a/src/tpm2/crypto/openssl/ExpDCache.c
147+++ b/src/tpm2/crypto/openssl/ExpDCache.c
148@@ -61,6 +61,8 @@
149 #include "Tpm.h"
150 #include "ExpDCache_fp.h"
151
152+extern void monogon_warn_unsafe_library(void);
153+
154 /* Implement a cache for the private exponent D so it doesn't need to be
155 * recalculated every time from P, Q, E and N (modulus). The cache has a
156 * number of entries that cache D and use P, Q, and E for lookup.
157@@ -169,6 +171,7 @@ BIGNUM *ExpDCacheFind(const BIGNUM *P, const BIGNUM *N, const BIGNUM *E, BIGNUM
158 unsigned myage;
159 BIGNUM *D;
160
161+ monogon_warn_unsafe_library();
162 for (i = 0; i < ARRAY_SIZE(ExpDCache); i++) {
163 if (BN_cmp(ExpDCache[i].P, P) == 0 && BN_cmp(ExpDCache[i].N, N) == 0 &&
164 BN_cmp(ExpDCache[i].E, E) == 0) {
165@@ -190,8 +193,6 @@ BIGNUM *ExpDCacheFind(const BIGNUM *P, const BIGNUM *N, const BIGNUM *E, BIGNUM
166 *Q = NULL;
167 return NULL;
168 }
169- BN_set_flags(*Q, BN_FLG_CONSTTIME);
170- BN_set_flags(D, BN_FLG_CONSTTIME);
171 return D;
172 }
173 }
174--
1752.42.0
176