1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
|
From c176c35e17d0add934785cb8db1a6c2d14ae9659 Mon Sep 17 00:00:00 2001
From: Daiki Ueno <ueno@gnu.org>
Date: Mon, 23 Oct 2023 09:26:57 +0900
Subject: [PATCH 11/29] auth/rsa_psk: side-step potential side-channel
This removes branching that depends on secret data, porting changes
for regular RSA key exchange from
4804febddc2ed958e5ae774de2a8f85edeeff538 and
80a6ce8ddb02477cd724cd5b2944791aaddb702a. This also removes the
allow_wrong_pms as it was used sorely to control debug output
depending on the branching.
Signed-off-by: Daiki Ueno <ueno@gnu.org>
---
lib/auth/rsa.c | 2 +-
lib/auth/rsa_psk.c | 93 +++++++++++++++++-----------------------------
lib/gnutls_int.h | 4 --
lib/priority.c | 1 -
4 files changed, 35 insertions(+), 65 deletions(-)
--- a/lib/auth/rsa.c
+++ b/lib/auth/rsa.c
@@ -205,11 +205,11 @@ proc_rsa_client_kx(gnutls_session_t sess
gnutls_privkey_decrypt_data2(session->internals.selected_key,
0, &ciphertext, session->key.key.data,
session->key.key.size);
/* After this point, any conditional on failure that cause differences
* in execution may create a timing or cache access pattern side
- * channel that can be used as an oracle, so treat very carefully */
+ * channel that can be used as an oracle, so tread carefully */
/* Error handling logic:
* In case decryption fails then don't inform the peer. Just use the
* random key previously generated. (in order to avoid attack against
* pkcs-1 formatting).
--- a/lib/auth/rsa_psk.c
+++ b/lib/auth/rsa_psk.c
@@ -262,18 +262,17 @@ static int
_gnutls_proc_rsa_psk_client_kx(gnutls_session_t session, uint8_t * data,
size_t _data_size)
{
gnutls_datum_t username;
psk_auth_info_t info;
- gnutls_datum_t plaintext;
gnutls_datum_t ciphertext;
gnutls_datum_t pwd_psk = { NULL, 0 };
int ret, dsize;
- int randomize_key = 0;
ssize_t data_size = _data_size;
gnutls_psk_server_credentials_t cred;
gnutls_datum_t premaster_secret = { NULL, 0 };
+ volatile uint8_t ver_maj, ver_min;
cred = (gnutls_psk_server_credentials_t)
_gnutls_get_cred(session, GNUTLS_CRD_PSK);
if (cred == NULL) {
@@ -327,75 +326,51 @@ _gnutls_proc_rsa_psk_client_kx(gnutls_se
gnutls_assert();
return GNUTLS_E_UNEXPECTED_PACKET_LENGTH;
}
ciphertext.size = dsize;
- ret =
- gnutls_privkey_decrypt_data(session->internals.selected_key, 0,
- &ciphertext, &plaintext);
- if (ret < 0 || plaintext.size != GNUTLS_MASTER_SIZE) {
- /* In case decryption fails then don't inform
- * the peer. Just use a random key. (in order to avoid
- * attack against pkcs-1 formatting).
- */
- gnutls_assert();
- _gnutls_debug_log
- ("auth_rsa_psk: Possible PKCS #1 format attack\n");
- if (ret >= 0) {
- gnutls_free(plaintext.data);
- }
- randomize_key = 1;
- } else {
- /* If the secret was properly formatted, then
- * check the version number.
- */
- if (_gnutls_get_adv_version_major(session) !=
- plaintext.data[0]
- || (session->internals.allow_wrong_pms == 0
- && _gnutls_get_adv_version_minor(session) !=
- plaintext.data[1])) {
- /* No error is returned here, if the version number check
- * fails. We proceed normally.
- * That is to defend against the attack described in the paper
- * "Attacking RSA-based sessions in SSL/TLS" by Vlastimil Klima,
- * Ondej Pokorny and Tomas Rosa.
- */
- gnutls_assert();
- _gnutls_debug_log
- ("auth_rsa: Possible PKCS #1 version check format attack\n");
- }
- }
+ ver_maj = _gnutls_get_adv_version_major(session);
+ ver_min = _gnutls_get_adv_version_minor(session);
+ premaster_secret.data = gnutls_malloc(GNUTLS_MASTER_SIZE);
+ if (premaster_secret.data == NULL) {
+ gnutls_assert();
+ return GNUTLS_E_MEMORY_ERROR;
+ }
+ premaster_secret.size = GNUTLS_MASTER_SIZE;
- if (randomize_key != 0) {
- premaster_secret.size = GNUTLS_MASTER_SIZE;
- premaster_secret.data =
- gnutls_malloc(premaster_secret.size);
- if (premaster_secret.data == NULL) {
- gnutls_assert();
- return GNUTLS_E_MEMORY_ERROR;
- }
-
- /* we do not need strong random numbers here.
- */
- ret = gnutls_rnd(GNUTLS_RND_NONCE, premaster_secret.data,
- premaster_secret.size);
- if (ret < 0) {
- gnutls_assert();
- goto cleanup;
- }
- } else {
- premaster_secret.data = plaintext.data;
- premaster_secret.size = plaintext.size;
+ /* Fallback value when decryption fails. Needs to be unpredictable. */
+ ret = gnutls_rnd(GNUTLS_RND_NONCE, premaster_secret.data,
+ premaster_secret.size);
+ if (ret < 0) {
+ gnutls_assert();
+ goto cleanup;
}
+ gnutls_privkey_decrypt_data2(session->internals.selected_key, 0,
+ &ciphertext, premaster_secret.data,
+ premaster_secret.size);
+ /* After this point, any conditional on failure that cause differences
+ * in execution may create a timing or cache access pattern side
+ * channel that can be used as an oracle, so tread carefully */
+
+ /* Error handling logic:
+ * In case decryption fails then don't inform the peer. Just use the
+ * random key previously generated. (in order to avoid attack against
+ * pkcs-1 formatting).
+ *
+ * If we get version mismatches no error is returned either. We
+ * proceed normally. This is to defend against the attack described
+ * in the paper "Attacking RSA-based sessions in SSL/TLS" by
+ * Vlastimil Klima, Ondej Pokorny and Tomas Rosa.
+ */
+
/* This is here to avoid the version check attack
* discussed above.
*/
-
- premaster_secret.data[0] = _gnutls_get_adv_version_major(session);
- premaster_secret.data[1] = _gnutls_get_adv_version_minor(session);
+ premaster_secret.data[0] = ver_maj;
+ premaster_secret.data[1] = ver_min;
/* find the key of this username
*/
ret =
_gnutls_psk_pwd_find_entry(session, info->username, strlen(info->username), &pwd_psk);
--- a/lib/gnutls_int.h
+++ b/lib/gnutls_int.h
@@ -983,11 +983,10 @@ struct gnutls_priority_st {
bool _allow_large_records;
bool _allow_small_records;
bool _no_etm;
bool _no_ext_master_secret;
bool _allow_key_usage_violation;
- bool _allow_wrong_pms;
bool _dumbfw;
unsigned int _dh_prime_bits; /* old (deprecated) variable */
DEF_ATOMIC_INT(usage_cnt);
};
@@ -1001,20 +1000,18 @@ struct gnutls_priority_st {
(x)->allow_large_records = 1; \
(x)->allow_small_records = 1; \
(x)->no_etm = 1; \
(x)->no_ext_master_secret = 1; \
(x)->allow_key_usage_violation = 1; \
- (x)->allow_wrong_pms = 1; \
(x)->dumbfw = 1
#define ENABLE_PRIO_COMPAT(x) \
(x)->_allow_large_records = 1; \
(x)->_allow_small_records = 1; \
(x)->_no_etm = 1; \
(x)->_no_ext_master_secret = 1; \
(x)->_allow_key_usage_violation = 1; \
- (x)->_allow_wrong_pms = 1; \
(x)->_dumbfw = 1
/* DH and RSA parameters types.
*/
typedef struct gnutls_dh_params_int {
@@ -1135,11 +1132,10 @@ typedef struct {
bool allow_large_records;
bool allow_small_records;
bool no_etm;
bool no_ext_master_secret;
bool allow_key_usage_violation;
- bool allow_wrong_pms;
bool dumbfw;
/* old (deprecated) variable. This is used for both srp_prime_bits
* and dh_prime_bits as they don't overlap */
/* For SRP: minimum bits to allow for SRP
--- a/lib/priority.c
+++ b/lib/priority.c
@@ -699,11 +699,10 @@ gnutls_priority_set(gnutls_session_t ses
COPY_TO_INTERNALS(allow_large_records);
COPY_TO_INTERNALS(allow_small_records);
COPY_TO_INTERNALS(no_etm);
COPY_TO_INTERNALS(no_ext_master_secret);
COPY_TO_INTERNALS(allow_key_usage_violation);
- COPY_TO_INTERNALS(allow_wrong_pms);
COPY_TO_INTERNALS(dumbfw);
COPY_TO_INTERNALS(dh_prime_bits);
return 0;
}
|
