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
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
|
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
commit 7d0ca355a5c7f8337130d4b0b3e7686f2fa4d4c2
Author: Paul Wouters <pwouters@redhat.com>
Date: Thu Apr 25 12:44:55 2013 -0400
* security: atodn() / atoid() buffer overflow
lib/libswan/x509dn.c:atodn() does not perform any length checking
whatsoever on the output buffer.
Affected:
- Libreswan 3.0 and 3.1 (3.2 disabled the oe= option)
- Openswan versions up to and including 2.6.38
- Possibly certain strongswan 3.x/4.x versions
This overflow is exposed (pre-authentication) only in opportunistic
encryption mode. When it is called via receiving a certificate
via IKEv1 or IKEv2, and when it is loaded from disk, the buffers
passed to atodn() are big enough.
This means this vulnerability can only be triggered when:
- Opportunistic Encryption is enabled (oe=yes)
- The attacker is local in the same network and adds a malicious
reverse DNS record to the client's IP, or
- The attacker can trigger an OE DNS lookup to a client fully
configured with OE and their own key.
Libreswan and openswan versions do not enable Opportunistic Encryption
per default. Most distributions like RHEL, Fedora, Debian and Ubuntu
also do not enable OE per default.
This patch addresses the vulnerability in atodn() and further limits the
atoid() call not to traverse into the ASN1 case when triggered by non-cert
cases such as opportunistic encryption.
Vulnerability discoverd by Florian Weimer <fweimer@redhat.com> of the
Red Hat Product Security Team.
Patch by D. Hugh Redelmeier <hugh@mimosa.com> and Paul Wouters <pwouters@redhat.com>
diff --git a/include/asn1.h b/include/asn1.h
index d69ebf9..b812488 100644
- --- a/include/asn1.h
+++ b/include/asn1.h
@@ -84,8 +84,10 @@ typedef enum {
#define ASN1_BODY 0x20
#define ASN1_RAW 0x40
- -#define ASN1_INVALID_LENGTH 0xffffffff
+#define ASN1_INVALID_LENGTH (~(size_t) 0) /* largest size_t */
+#define ASN1_MAX_LEN (1U << (8*3)) /* don't handle objects with length greater than this */
+#define ASN1_MAX_LEN_LEN 4 /* no coded length takes more than 4 bytes. */
/* definition of an ASN.1 object */
diff --git a/include/id.h b/include/id.h
index d1825b4..b440a11 100644
- --- a/include/id.h
+++ b/include/id.h
@@ -47,7 +47,7 @@ extern const struct id *resolve_myid(const struct id *id);
extern void set_myFQDN(void);
extern void free_myFQDN(void);
- -extern err_t atoid(char *src, struct id *id, bool myid_ok);
+extern err_t atoid(char *src, struct id *id, bool myid_ok, bool oe_only);
extern void iptoid(const ip_address *ip, struct id *id);
extern unsigned char* temporary_cyclic_buffer(void);
extern int idtoa(const struct id *id, char *dst, size_t dstlen);
diff --git a/lib/libswan/id.c b/lib/libswan/id.c
index 4442971..31ca7e5 100644
- --- a/lib/libswan/id.c
+++ b/lib/libswan/id.c
@@ -58,27 +58,29 @@ temporary_cyclic_buffer(void)
/* Convert textual form of id into a (temporary) struct id.
* Note that if the id is to be kept, unshare_id_content will be necessary.
+ * This function should be split into parts so the boolean arguments can be
+ * removed -- Paul
*/
err_t
- -atoid(char *src, struct id *id, bool myid_ok)
+atoid(char *src, struct id *id, bool myid_ok, bool oe_only)
{
err_t ugh = NULL;
*id = empty_id;
- - if (myid_ok && streq("%myid", src))
+ if (!oe_only && myid_ok && streq("%myid", src))
{
id->kind = ID_MYID;
}
- - else if (streq("%fromcert", src))
+ else if (!oe_only && streq("%fromcert", src))
{
id->kind = ID_FROMCERT;
}
- - else if (streq("%none", src))
+ else if (!oe_only && streq("%none", src))
{
id->kind = ID_NONE;
}
- - else if (strchr(src, '=') != NULL)
+ else if (!oe_only && strchr(src, '=') != NULL)
{
/* we interpret this as an ASCII X.501 ID_DER_ASN1_DN */
id->kind = ID_DER_ASN1_DN;
@@ -112,7 +114,7 @@ atoid(char *src, struct id *id, bool myid_ok)
{
if (*src == '@')
{
- - if (*(src+1) == '#')
+ if (!oe_only && *(src+1) == '#')
{
/* if there is a second specifier (#) on the line
* we interprete this as ID_KEY_ID
@@ -123,7 +125,7 @@ atoid(char *src, struct id *id, bool myid_ok)
ugh = ttodata(src+2, 0, 16, (char *)id->name.ptr
, strlen(src), &id->name.len);
}
- - else if (*(src+1) == '~')
+ else if (!oe_only && *(src+1) == '~')
{
/* if there is a second specifier (~) on the line
* we interprete this as a binary ID_DER_ASN1_DN
@@ -134,7 +136,7 @@ atoid(char *src, struct id *id, bool myid_ok)
ugh = ttodata(src+2, 0, 16, (char *)id->name.ptr
, strlen(src), &id->name.len);
}
- - else if (*(src+1) == '[')
+ else if (!oe_only && *(src+1) == '[')
{
/* if there is a second specifier ([) on the line
* we interprete this as a text ID_KEY_ID, and we remove
diff --git a/lib/libswan/secrets.c b/lib/libswan/secrets.c
index 6e9466b..8ff80e0 100644
- --- a/lib/libswan/secrets.c
+++ b/lib/libswan/secrets.c
@@ -1223,7 +1223,7 @@ lsw_process_secret_records(struct secret **psecrets, int verbose,
}
else
{
- - ugh = atoid(flp->tok, &id, FALSE);
+ ugh = atoid(flp->tok, &id, FALSE, FALSE);
}
if (ugh != NULL)
diff --git a/lib/libswan/x509dn.c b/lib/libswan/x509dn.c
index 61407e5..7731856 100644
- --- a/lib/libswan/x509dn.c
+++ b/lib/libswan/x509dn.c
@@ -472,7 +472,7 @@ static const x501rdn_t x501rdns[] = {
{"TCGID" , {oid_TCGID, 12}, ASN1_PRINTABLESTRING}
};
- -#define X501_RDN_ROOF 24
+#define X501_RDN_ROOF elemsof(x501rdns)
/* Maximum length of ASN.1 distinquished name */
#define ASN1_BUF_LEN 512
@@ -775,11 +775,11 @@ atodn(char *src, chunk_t *dn)
UNKNOWN_OID = 4
} state_t;
- - u_char oid_len_buf[3];
- - u_char name_len_buf[3];
- - u_char rdn_seq_len_buf[3];
- - u_char rdn_set_len_buf[3];
- - u_char dn_seq_len_buf[3];
+ u_char oid_len_buf[ASN1_MAX_LEN_LEN];
+ u_char name_len_buf[ASN1_MAX_LEN_LEN];
+ u_char rdn_seq_len_buf[ASN1_MAX_LEN_LEN];
+ u_char rdn_set_len_buf[ASN1_MAX_LEN_LEN];
+ u_char dn_seq_len_buf[ASN1_MAX_LEN_LEN];
chunk_t asn1_oid_len = { oid_len_buf, 0 };
chunk_t asn1_name_len = { name_len_buf, 0 };
@@ -797,7 +797,7 @@ atodn(char *src, chunk_t *dn)
err_t ugh = NULL;
- - u_char *dn_ptr = dn->ptr + 4;
+ u_char *dn_ptr = dn->ptr + 1 + ASN1_MAX_LEN_LEN; /* leave room for prefix */
state_t state = SEARCH_OID;
@@ -885,25 +885,37 @@ atodn(char *src, chunk_t *dn)
code_asn1_length(rdn_set_len, &asn1_rdn_set_len);
/* encode the relative distinguished name */
- - *dn_ptr++ = ASN1_SET;
- - chunkcpy(dn_ptr, asn1_rdn_set_len);
- - *dn_ptr++ = ASN1_SEQUENCE;
- - chunkcpy(dn_ptr, asn1_rdn_seq_len);
- - *dn_ptr++ = ASN1_OID;
- - chunkcpy(dn_ptr, asn1_oid_len);
- - chunkcpy(dn_ptr, x501rdns[pos].oid);
- - /* encode the ASN.1 character string type of the name */
- - *dn_ptr++ = (x501rdns[pos].type == ASN1_PRINTABLESTRING
- - && !is_printablestring(name))? ASN1_T61STRING : x501rdns[pos].type;
- - chunkcpy(dn_ptr, asn1_name_len);
- - chunkcpy(dn_ptr, name);
- -
- - /* accumulate the length of the distinguished name sequence */
- - dn_seq_len += 1 + asn1_rdn_set_len.len + rdn_set_len;
- -
- - /* reset name and change state */
- - name = empty_chunk;
- - state = SEARCH_OID;
+ if (IDTOA_BUF < dn_ptr - dn->ptr
+ + 1 + asn1_rdn_set_len.len /* set */
+ + 1 + asn1_rdn_seq_len.len /* sequence */
+ + 1 + asn1_oid_len.len + x501rdns[pos].oid.len /* oid len, oid */
+ + 1 + asn1_name_len.len + name.len /* type name */
+ ) {
+ /* no room! */
+ ugh = "DN is too big";
+ state = UNKNOWN_OID;
+ /* I think that it is safe to continue (but perhaps pointless) */
+ } else {
+ *dn_ptr++ = ASN1_SET;
+ chunkcpy(dn_ptr, asn1_rdn_set_len);
+ *dn_ptr++ = ASN1_SEQUENCE;
+ chunkcpy(dn_ptr, asn1_rdn_seq_len);
+ *dn_ptr++ = ASN1_OID;
+ chunkcpy(dn_ptr, asn1_oid_len);
+ chunkcpy(dn_ptr, x501rdns[pos].oid);
+ /* encode the ASN.1 character string type of the name */
+ *dn_ptr++ = (x501rdns[pos].type == ASN1_PRINTABLESTRING
+ && !is_printablestring(name))? ASN1_T61STRING : x501rdns[pos].type;
+ chunkcpy(dn_ptr, asn1_name_len);
+ chunkcpy(dn_ptr, name);
+
+ /* accumulate the length of the distinguished name sequence */
+ dn_seq_len += 1 + asn1_rdn_set_len.len + rdn_set_len;
+
+ /* reset name and change state */
+ name = empty_chunk;
+ state = SEARCH_OID;
+ }
}
break;
case UNKNOWN_OID:
@@ -911,9 +923,9 @@ atodn(char *src, chunk_t *dn)
}
} while (*src++ != '\0');
- - /* complete the distinguished name sequence*/
- - code_asn1_length(dn_seq_len, &asn1_dn_seq_len);
- - dn->ptr += 3 - asn1_dn_seq_len.len;
+ /* complete the distinguished name sequence: prefix it with ASN1_SEQUENCE and length */
+ code_asn1_length((size_t)dn_seq_len, &asn1_dn_seq_len);
+ dn->ptr += ASN1_MAX_LEN_LEN + 1 - 1 - asn1_dn_seq_len.len;
dn->len = 1 + asn1_dn_seq_len.len + dn_seq_len;
dn_ptr = dn->ptr;
*dn_ptr++ = ASN1_SEQUENCE;
diff --git a/programs/pluto/connections.c b/programs/pluto/connections.c
index e8d326b..f08521b 100644
- --- a/programs/pluto/connections.c
+++ b/programs/pluto/connections.c
@@ -911,7 +911,7 @@ extract_end(struct end *dst, const struct whack_end *src, const char *which)
}
else
{
- - err_t ugh = atoid(src->id, &dst->id, TRUE);
+ err_t ugh = atoid(src->id, &dst->id, TRUE, FALSE);
if (ugh != NULL)
{
diff --git a/programs/pluto/dnskey.c b/programs/pluto/dnskey.c
index 5525d12..78f1d0a 100644
- --- a/programs/pluto/dnskey.c
+++ b/programs/pluto/dnskey.c
@@ -277,8 +277,12 @@ decode_iii(char **pp, struct id *gw_id)
if (*p == '@')
{
/* gateway specification in this record is @FQDN */
- - err_t ugh = atoid(p, gw_id, FALSE);
+ if(strspn(p,' ') >= IDTOA_BUF) {
+ return builddiag("malformed FQDN in TXT " our_TXT_attr_string ": ID too large for IDTOA_BUF");
+ }
+
+ err_t ugh = atoid(p, gw_id, FALSE, TRUE); /* only run OE related parts of atoid() */
if (ugh != NULL)
return builddiag("malformed FQDN in TXT " our_TXT_attr_string ": %s"
, ugh);
diff --git a/programs/pluto/myid.c b/programs/pluto/myid.c
index bdd0e12..2e92f25 100644
- --- a/programs/pluto/myid.c
+++ b/programs/pluto/myid.c
@@ -103,7 +103,7 @@ set_myid(enum myid_state s, char *idstr)
if (idstr != NULL)
{
struct id id;
- - err_t ugh = atoid(idstr, &id, FALSE);
+ err_t ugh = atoid(idstr, &id, FALSE, FALSE);
if (ugh != NULL)
{
diff --git a/programs/pluto/rcv_whack.c b/programs/pluto/rcv_whack.c
index 1725357..7d5072c 100644
- --- a/programs/pluto/rcv_whack.c
+++ b/programs/pluto/rcv_whack.c
@@ -259,7 +259,7 @@ static void
key_add_request(const struct whack_message *msg)
{
struct id keyid;
- - err_t ugh = atoid(msg->keyid, &keyid, FALSE);
+ err_t ugh = atoid(msg->keyid, &keyid, FALSE, FALSE);
if (ugh != NULL)
{
diff --git a/programs/showhostkey/showhostkey.c b/programs/showhostkey/showhostkey.c
index c9fe9cf..bf87080 100644
- --- a/programs/showhostkey/showhostkey.c
+++ b/programs/showhostkey/showhostkey.c
@@ -203,7 +203,7 @@ struct secret *pick_key(struct secret *host_secrets
struct secret *s;
err_t e;
- - e = atoid(idname, &id, FALSE);
+ e = atoid(idname, &id, FALSE, FALSE);
if(e) {
printf("%s: key '%s' is invalid\n", progname, idname);
exit(4);
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
iQIcBAEBCAAGBQJRkWmnAAoJEIX/S0OzD8b5EZIP+wb5LyvL4jXGYJzvalkCjWL3
1cZp5H672jGdVvW/G3bJ5unhjpRt9ASxebHR/4LfWZuWG5U4gdPRjcz1YcuNwVnB
xOXZ4ELWYRFFblkkHz+GO5rSRwmWhFnyGvDdN5Oh6VBcmegHvaKk6uVLPXZJpVdg
2U1+s+x3EkrcP6IJyTa9pyhZiDWcdYVn3seyHcFCNa3R/Xkwefi3HwA2w8+L18NX
NvIMUx2aXj70cBE5VAg+XJWIZ2Rrlf2zHDM96GUUfGIIH1mzpuxYCFbpGqISmOYI
AAumQ9I4kQGy0ZkWn41Et3ppJvcRFoMlAz70Ay+nbZ/+eqQH9B3KfplfX2UrsXAn
SVvMPypkMfjhUbPG8AWr//6+a0uZxa0PyibNXhhdr+3ocANaZ8ty+ehFmVl0DIBM
rc582erQ8s4Bj8v+4vy1TzkR5HXWhwWhCjD0EnU8zGGjZ2u+1BAYgzTUG4Nqo+/Q
ziJdc71vy+OqyLXTFMdekUuRl40BXuFHHUv6jWeslgIh2/1Z/A0NZzxs2sMFCkEW
anTG32ridJSCqQhSXZ4xW07O5F45csH6qgze2jQdYEizATYsDqeKazEZhmakUsow
v5gj85f5VYGWjoYjKr/HbrueEbeGpV3Twf4tZ6XyCxAjJEt6N8XWidSiMeL3gNIm
cgXmYH+ak4nDLJGyaYDt
=5y9o
-----END PGP SIGNATURE-----
|