aports/main/openswan/CVE-2013-2052.patch

347 lines
12 KiB
Diff

-----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-----