2015-08-05 13:18:18

by David Howells

[permalink] [raw]
Subject: [PATCH 1/4] ASN.1: Fix handling of CHOICE in ASN.1 compiler

Fix the handling of CHOICE types in the ASN.1 compiler to make SEQUENCE and
SET elements in a CHOICE be correctly rendered as skippable and conditional
as appropriate.

For example, in the following ASN.1:

Foo ::= SEQUENCE { w1 INTEGER, w2 Bar, w3 OBJECT IDENTIFIER }
Bar ::= CHOICE {
x1 Seq1,
x2 [0] IMPLICIT OCTET STRING,
x3 Seq2,
x4 SET OF INTEGER
}
Seq1 ::= SEQUENCE { y1 INTEGER, y2 INTEGER, y3 INTEGER }
Seq2 ::= SEQUENCE { z1 BOOLEAN, z2 BOOLEAN, z3 BOOLEAN }

the output in foo.c generated by:

./scripts/asn1_compiler foo.asn1 foo.c foo.h

included:

// Bar
// Seq1
[ 4] = ASN1_OP_MATCH,
[ 5] = _tag(UNIV, CONS, SEQ),
...
[ 13] = ASN1_OP_COND_MATCH_OR_SKIP, // x2
[ 14] = _tagn(CONT, PRIM, 0),
// Seq2
[ 15] = ASN1_OP_MATCH,
[ 16] = _tag(UNIV, CONS, SEQ),
...
[ 24] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x4
[ 25] = _tag(UNIV, CONS, SET),
...
[ 27] = ASN1_OP_COND_FAIL,

as a result of the CHOICE - but this is wrong on lines 4 and 15 because
both of these should be skippable (one and only one of the four can be
picked) and the one on line 15 should also be conditional so that it is
ignored if anything before it matches.

After the patch, it looks like:

// Bar
// Seq1
[ 4] = ASN1_OP_MATCH_JUMP_OR_SKIP, // x1
[ 5] = _tag(UNIV, CONS, SEQ),
...
[ 7] = ASN1_OP_COND_MATCH_OR_SKIP, // x2
[ 8] = _tagn(CONT, PRIM, 0),
// Seq2
[ 9] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x3
[ 10] = _tag(UNIV, CONS, SEQ),
...
[ 12] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // x4
[ 13] = _tag(UNIV, CONS, SET),
...
[ 15] = ASN1_OP_COND_FAIL,

where all four options are skippable and the second, third and fourth are
all conditional, as is the backstop at the end.

This hasn't been a problem so far because in the ASN.1 specs we have are
either using primitives or are using SET OF and SEQUENCE OF which are
handled correctly.

Whilst we're at it, also make sure that element labels get included in
comments in the output for elements that have complex types.

This cannot be tested with the code as it stands, but rather affects future
code.

Signed-off-by: David Howells <[email protected]>
Reviewed-By: David Woodhouse <[email protected]>
---

scripts/asn1_compiler.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
index 7750e9c31483..e87359cd23c0 100644
--- a/scripts/asn1_compiler.c
+++ b/scripts/asn1_compiler.c
@@ -666,7 +666,7 @@ struct element {
unsigned flags;
#define ELEMENT_IMPLICIT 0x0001
#define ELEMENT_EXPLICIT 0x0002
-#define ELEMENT_MARKED 0x0004
+#define ELEMENT_TAG_SPECIFIED 0x0004
#define ELEMENT_RENDERED 0x0008
#define ELEMENT_SKIPPABLE 0x0010
#define ELEMENT_CONDITIONAL 0x0020
@@ -879,6 +879,7 @@ static struct element *parse_type(struct token **_cursor, struct token *end,

element->tag &= ~0x1f;
element->tag |= strtoul(cursor->value, &p, 10);
+ element->flags |= ELEMENT_TAG_SPECIFIED;
if (p - cursor->value != cursor->size)
abort();
cursor++;
@@ -1376,7 +1377,7 @@ static void render_out_of_line_list(FILE *out)
*/
static void render_element(FILE *out, struct element *e, struct element *tag)
{
- struct element *ec;
+ struct element *ec, *x;
const char *cond, *act;
int entry, skippable = 0, outofline = 0;

@@ -1435,15 +1436,17 @@ static void render_element(FILE *out, struct element *e, struct element *tag)
break;
}

- if (e->name)
+ x = tag ?: e;
+ if (x->name)
render_more(out, "\t\t// %*.*s",
- (int)e->name->size, (int)e->name->size,
- e->name->value);
+ (int)x->name->size, (int)x->name->size,
+ x->name->value);
render_more(out, "\n");

/* Render the tag */
- if (!tag)
+ if (!tag || !(tag->flags & ELEMENT_TAG_SPECIFIED))
tag = e;
+
if (tag->class == ASN1_UNIV &&
tag->tag != 14 &&
tag->tag != 15 &&
@@ -1539,7 +1542,7 @@ dont_render_tag:

case CHOICE:
for (ec = e->children; ec; ec = ec->next)
- render_element(out, ec, NULL);
+ render_element(out, ec, ec);
if (!skippable)
render_opcode(out, "ASN1_OP_COND_FAIL,\n");
if (e->action)


2015-08-05 13:18:29

by David Howells

[permalink] [raw]
Subject: [PATCH 2/4] ASN.1: Fix actions on CHOICE elements with IMPLICIT tags

In an ASN.1 description where there is a CHOICE construct that contains
elements with IMPLICIT tags that refer to constructed types, actions to be
taken on those elements should be conditional on the corresponding element
actually being matched. Currently, however, such actions are performed
unconditionally in the middle of processing the CHOICE.

For example, look at elements 'b' and 'e' here:

A ::= SEQUENCE {
CHOICE {
b [0] IMPLICIT B ({ do_XXXXXXXXXXXX_b }),
c [1] EXPLICIT C ({ do_XXXXXXXXXXXX_c }),
d [2] EXPLICIT B ({ do_XXXXXXXXXXXX_d }),
e [3] IMPLICIT C ({ do_XXXXXXXXXXXX_e }),
f [4] IMPLICIT INTEGER ({ do_XXXXXXXXXXXX_f })
}
} ({ do_XXXXXXXXXXXX_A })

B ::= SET OF OBJECT IDENTIFIER ({ do_XXXXXXXXXXXX_oid })

C ::= SET OF INTEGER ({ do_XXXXXXXXXXXX_int })

They each have an action (do_XXXXXXXXXXXX_b and do_XXXXXXXXXXXX_e) that
should only be processed if that element is matched.

The problem is that there's no easy place to hang the action off in the
subclause (type B for element 'b' and type C for element 'e') because
subclause opcode sequences can be shared.

To fix this, introduce a conditional action opcode(ASN1_OP_MAYBE_ACT) that
the decoder only processes if the preceding match was successful. This can
be seen in an excerpt from the output of the fixed ASN.1 compiler for the
above ASN.1 description:

[ 13] = ASN1_OP_COND_MATCH_JUMP_OR_SKIP, // e
[ 14] = _tagn(CONT, CONS, 3),
[ 15] = _jump_target(45), // --> C
[ 16] = ASN1_OP_MAYBE_ACT,
[ 17] = _action(ACT_do_XXXXXXXXXXXX_e),

In this, if the op at [13] is matched (ie. element 'e' above) then the
action at [16] will be performed. However, if the op at [13] doesn't match
or is skipped because it is conditional and some previous op matched, then
the action at [16] will be ignored.

Note that to make this work in the decoder, the ASN1_OP_RETURN op must set
the flag to indicate that a match happened. This is necessary because the
_jump_target() seen above introduces a subclause (in this case an object of
type 'C') which is likely to alter the flag. Setting the flag here is okay
because to process a subclause, a match must have happened and caused a
jump.

This cannot be tested with the code as it stands, but rather affects future
code.

Signed-off-by: David Howells <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
---

include/linux/asn1_ber_bytecode.h | 3 ++-
lib/asn1_decoder.c | 14 +++++++++++++-
scripts/asn1_compiler.c | 3 ++-
3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/include/linux/asn1_ber_bytecode.h b/include/linux/asn1_ber_bytecode.h
index 945d44ae529c..27f35780aecf 100644
--- a/include/linux/asn1_ber_bytecode.h
+++ b/include/linux/asn1_ber_bytecode.h
@@ -61,7 +61,8 @@ enum asn1_opcode {
ASN1_OP_COND_FAIL = 0x1b,
ASN1_OP_COMPLETE = 0x1c,
ASN1_OP_ACT = 0x1d,
- ASN1_OP_RETURN = 0x1e,
+ ASN1_OP_MAYBE_ACT = 0x1e,
+ ASN1_OP_RETURN = 0x1f,

/* The following eight have bit 0 -> SET, 1 -> OF, 2 -> ACT */
ASN1_OP_END_SEQ = 0x20,
diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 1a000bb050f9..55980d7e1ac0 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -33,6 +33,7 @@ static const unsigned char asn1_op_lengths[ASN1_OP__NR] = {
[ASN1_OP_COND_FAIL] = 1,
[ASN1_OP_COMPLETE] = 1,
[ASN1_OP_ACT] = 1 + 1,
+ [ASN1_OP_MAYBE_ACT] = 1 + 1,
[ASN1_OP_RETURN] = 1,
[ASN1_OP_END_SEQ] = 1,
[ASN1_OP_END_SEQ_OF] = 1 + 1,
@@ -177,6 +178,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
unsigned char flags = 0;
#define FLAG_INDEFINITE_LENGTH 0x01
#define FLAG_MATCHED 0x02
+#define FLAG_LAST_MATCHED 0x04 /* Last tag matched */
#define FLAG_CONS 0x20 /* Corresponds to CONS bit in the opcode tag
* - ie. whether or not we are going to parse
* a compound type.
@@ -211,6 +213,7 @@ next_op:
if ((op & ASN1_OP_MATCH__COND &&
flags & FLAG_MATCHED) ||
dp == datalen) {
+ flags &= ~FLAG_LAST_MATCHED;
pc += asn1_op_lengths[op];
goto next_op;
}
@@ -422,8 +425,15 @@ next_op:
pc += asn1_op_lengths[op];
goto next_op;

+ case ASN1_OP_MAYBE_ACT:
+ if (!(flags & FLAG_LAST_MATCHED)) {
+ pc += asn1_op_lengths[op];
+ goto next_op;
+ }
case ASN1_OP_ACT:
ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len);
+ if (ret < 0)
+ return ret;
pc += asn1_op_lengths[op];
goto next_op;

@@ -431,6 +441,7 @@ next_op:
if (unlikely(jsp <= 0))
goto jump_stack_underflow;
pc = jump_stack[--jsp];
+ flags |= FLAG_MATCHED | FLAG_LAST_MATCHED;
goto next_op;

default:
@@ -438,7 +449,8 @@ next_op:
}

/* Shouldn't reach here */
- pr_err("ASN.1 decoder error: Found reserved opcode (%u)\n", op);
+ pr_err("ASN.1 decoder error: Found reserved opcode (%u) pc=%zu\n",
+ op, pc);
return -EBADMSG;

data_overrun_error:
diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
index e87359cd23c0..0515bced929a 100644
--- a/scripts/asn1_compiler.c
+++ b/scripts/asn1_compiler.c
@@ -1468,7 +1468,8 @@ dont_render_tag:
case TYPE_REF:
render_element(out, e->type->type->element, tag);
if (e->action)
- render_opcode(out, "ASN1_OP_ACT,\n");
+ render_opcode(out, "ASN1_OP_%sACT,\n",
+ skippable ? "MAYBE_" : "");
break;

case SEQUENCE:

2015-08-05 13:18:39

by David Howells

[permalink] [raw]
Subject: [PATCH 3/4] ASN.1: Fix non-match detection failure on data overrun

If the ASN.1 decoder is asked to parse a sequence of objects, non-optional
matches get skipped if there's no more data to be had rather than a
data-overrun error being reported.

This is due to the code segment that decides whether to skip optional
matches (ie. matches that could get ignored because an element is marked
OPTIONAL in the grammar) due to a lack of data also skips non-optional
elements if the data pointer has reached the end of the buffer.

This can be tested with the data decoder for the new RSA akcipher algorithm
that takes three non-optional integers. Currently, it skips the last
integer if there is insufficient data.

Without the fix, #defining DEBUG in asn1_decoder.c will show something
like:

next_op: pc=0/13 dp=0/270 C=0 J=0
- match? 30 30 00
- TAG: 30 266 CONS
next_op: pc=2/13 dp=4/270 C=1 J=0
- match? 02 02 00
- TAG: 02 257
- LEAF: 257
next_op: pc=5/13 dp=265/270 C=1 J=0
- match? 02 02 00
- TAG: 02 3
- LEAF: 3
next_op: pc=8/13 dp=270/270 C=1 J=0
next_op: pc=11/13 dp=270/270 C=1 J=0
- end cons t=4 dp=270 l=270/270

The next_op line for pc=8/13 should be followed by a match line.

This is not exploitable for X.509 certificates by means of shortening the
message and fixing up the ASN.1 CONS tags because:

(1) The relevant records being built up are cleared before use.

(2) If the message is shortened sufficiently to remove the public key, the
ASN.1 parse of the RSA key will fail quickly due to a lack of data.

(3) Extracted signature data is either turned into MPIs (which cope with a
0 length) or is simpler integers specifying algoritms and suchlike
(which can validly be 0); and

(4) The AKID and SKID extensions are optional and their removal is handled
without risking passing a NULL to asymmetric_key_generate_id().

(5) If the certificate is truncated sufficiently to remove the subject,
issuer or serialNumber then the ASN.1 decoder will fail with a 'Cons
stack underflow' return.

This is not exploitable for PKCS#7 messages by means of removal of elements
from such a message from the tail end of a sequence:

(1) Any shortened X.509 certs embedded in the PKCS#7 message are survivable
as detailed above.

(2) The message digest content isn't used if it shows a NULL pointer,
similarly, the authattrs aren't used if that shows a NULL pointer.

(3) A missing signature results in a NULL MPI - which the MPI routines deal
with.

(4) If data is NULL, it is expected that the message has detached content and
that is handled appropriately.

(5) If the serialNumber is excised, the unconditional action associated
with it will pick up the containing SEQUENCE instead, so no NULL
pointer will be seen here.

If both the issuer and the serialNumber are excised, the ASN.1 decode
will fail with an 'Unexpected tag' return.

In either case, there's no way to get to asymmetric_key_generate_id()
with a NULL pointer.

(6) Other fields are decoded to simple integers. Shortening the message
to omit an algorithm ID field will cause checks on this to fail early
in the verification process.


This can also be tested by snipping objects off of the end of the ASN.1 stream
such that mandatory tags are removed - or even from the end of internal
SEQUENCEs. If any mandatory tag is missing, the error EBADMSG *should* be
produced. Without this patch ERANGE or ENOPKG might be produced or the parse
may apparently succeed, perhaps with ENOKEY or EKEYREJECTED being produced
later, depending on what gets snipped.

Just snipping off the final BIT_STRING or OCTET_STRING from either sample
should be a start since both are mandatory and neither will cause an EBADMSG
without the patches

Reported-by: Marcel Holtmann <[email protected]>
Signed-off-by: David Howells <[email protected]>
Tested-by: Marcel Holtmann <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
---

lib/asn1_decoder.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 55980d7e1ac0..3f74dd3e2910 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -210,9 +210,8 @@ next_op:
unsigned char tmp;

/* Skip conditional matches if possible */
- if ((op & ASN1_OP_MATCH__COND &&
- flags & FLAG_MATCHED) ||
- dp == datalen) {
+ if ((op & ASN1_OP_MATCH__COND && flags & FLAG_MATCHED) ||
+ (op & ASN1_OP_MATCH__SKIP && dp == datalen)) {
flags &= ~FLAG_LAST_MATCHED;
pc += asn1_op_lengths[op];
goto next_op;

2015-08-05 13:18:49

by David Howells

[permalink] [raw]
Subject: [PATCH 4/4] ASN.1: Handle 'ANY OPTIONAL' in grammar

An ANY object in an ASN.1 grammar that is marked OPTIONAL should be skipped
if there is no more data to be had.

This can be tested by editing X.509 certificates or PKCS#7 messages to
remove the NULL from subobjects that look like the following:

SEQUENCE {
OBJECT(2a864886f70d01010b);
NULL();
}

This is an algorithm identifier plus an optional parameter.

The modified DER can be passed to one of:

keyctl padd asymmetric "" @s </tmp/modified.x509
keyctl padd pkcs7_test foo @s </tmp/modified.pkcs7

It should work okay with the patch and produce EBADMSG without.

Signed-off-by: David Howells <[email protected]>
Tested-by: Marcel Holtmann <[email protected]>
Reviewed-by: David Woodhouse <[email protected]>
---

include/linux/asn1_ber_bytecode.h | 17 +++++++++++------
lib/asn1_decoder.c | 8 ++++++++
scripts/asn1_compiler.c | 3 ++-
3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/linux/asn1_ber_bytecode.h b/include/linux/asn1_ber_bytecode.h
index 27f35780aecf..ab3a6c002f7b 100644
--- a/include/linux/asn1_ber_bytecode.h
+++ b/include/linux/asn1_ber_bytecode.h
@@ -45,24 +45,27 @@ enum asn1_opcode {
ASN1_OP_MATCH_JUMP = 0x04,
ASN1_OP_MATCH_JUMP_OR_SKIP = 0x05,
ASN1_OP_MATCH_ANY = 0x08,
+ ASN1_OP_MATCH_ANY_OR_SKIP = 0x09,
ASN1_OP_MATCH_ANY_ACT = 0x0a,
+ ASN1_OP_MATCH_ANY_ACT_OR_SKIP = 0x0b,
/* Everything before here matches unconditionally */

ASN1_OP_COND_MATCH_OR_SKIP = 0x11,
ASN1_OP_COND_MATCH_ACT_OR_SKIP = 0x13,
ASN1_OP_COND_MATCH_JUMP_OR_SKIP = 0x15,
ASN1_OP_COND_MATCH_ANY = 0x18,
+ ASN1_OP_COND_MATCH_ANY_OR_SKIP = 0x19,
ASN1_OP_COND_MATCH_ANY_ACT = 0x1a,
+ ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP = 0x1b,

/* Everything before here will want a tag from the data */
-#define ASN1_OP__MATCHES_TAG ASN1_OP_COND_MATCH_ANY_ACT
+#define ASN1_OP__MATCHES_TAG ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP

/* These are here to help fill up space */
- ASN1_OP_COND_FAIL = 0x1b,
- ASN1_OP_COMPLETE = 0x1c,
- ASN1_OP_ACT = 0x1d,
- ASN1_OP_MAYBE_ACT = 0x1e,
- ASN1_OP_RETURN = 0x1f,
+ ASN1_OP_COND_FAIL = 0x1c,
+ ASN1_OP_COMPLETE = 0x1d,
+ ASN1_OP_ACT = 0x1e,
+ ASN1_OP_MAYBE_ACT = 0x1f,

/* The following eight have bit 0 -> SET, 1 -> OF, 2 -> ACT */
ASN1_OP_END_SEQ = 0x20,
@@ -77,6 +80,8 @@ enum asn1_opcode {
#define ASN1_OP_END__OF 0x02
#define ASN1_OP_END__ACT 0x04

+ ASN1_OP_RETURN = 0x28,
+
ASN1_OP__NR
};

diff --git a/lib/asn1_decoder.c b/lib/asn1_decoder.c
index 3f74dd3e2910..2b3f46c049d4 100644
--- a/lib/asn1_decoder.c
+++ b/lib/asn1_decoder.c
@@ -24,12 +24,16 @@ static const unsigned char asn1_op_lengths[ASN1_OP__NR] = {
[ASN1_OP_MATCH_JUMP] = 1 + 1 + 1,
[ASN1_OP_MATCH_JUMP_OR_SKIP] = 1 + 1 + 1,
[ASN1_OP_MATCH_ANY] = 1,
+ [ASN1_OP_MATCH_ANY_OR_SKIP] = 1,
[ASN1_OP_MATCH_ANY_ACT] = 1 + 1,
+ [ASN1_OP_MATCH_ANY_ACT_OR_SKIP] = 1 + 1,
[ASN1_OP_COND_MATCH_OR_SKIP] = 1 + 1,
[ASN1_OP_COND_MATCH_ACT_OR_SKIP] = 1 + 1 + 1,
[ASN1_OP_COND_MATCH_JUMP_OR_SKIP] = 1 + 1 + 1,
[ASN1_OP_COND_MATCH_ANY] = 1,
+ [ASN1_OP_COND_MATCH_ANY_OR_SKIP] = 1,
[ASN1_OP_COND_MATCH_ANY_ACT] = 1 + 1,
+ [ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP] = 1 + 1,
[ASN1_OP_COND_FAIL] = 1,
[ASN1_OP_COMPLETE] = 1,
[ASN1_OP_ACT] = 1 + 1,
@@ -304,7 +308,9 @@ next_op:
/* Decide how to handle the operation */
switch (op) {
case ASN1_OP_MATCH_ANY_ACT:
+ case ASN1_OP_MATCH_ANY_ACT_OR_SKIP:
case ASN1_OP_COND_MATCH_ANY_ACT:
+ case ASN1_OP_COND_MATCH_ANY_ACT_OR_SKIP:
ret = actions[machine[pc + 1]](context, hdr, tag, data + dp, len);
if (ret < 0)
return ret;
@@ -321,8 +327,10 @@ next_op:
case ASN1_OP_MATCH:
case ASN1_OP_MATCH_OR_SKIP:
case ASN1_OP_MATCH_ANY:
+ case ASN1_OP_MATCH_ANY_OR_SKIP:
case ASN1_OP_COND_MATCH_OR_SKIP:
case ASN1_OP_COND_MATCH_ANY:
+ case ASN1_OP_COND_MATCH_ANY_OR_SKIP:
skip_data:
if (!(flags & FLAG_CONS)) {
if (flags & FLAG_INDEFINITE_LENGTH) {
diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
index 0515bced929a..1c75e22b6385 100644
--- a/scripts/asn1_compiler.c
+++ b/scripts/asn1_compiler.c
@@ -1401,7 +1401,8 @@ static void render_element(FILE *out, struct element *e, struct element *tag)
act = e->action ? "_ACT" : "";
switch (e->compound) {
case ANY:
- render_opcode(out, "ASN1_OP_%sMATCH_ANY%s,", cond, act);
+ render_opcode(out, "ASN1_OP_%sMATCH_ANY%s%s,",
+ cond, act, skippable ? "_OR_SKIP" : "");
if (e->name)
render_more(out, "\t\t// %*.*s",
(int)e->name->size, (int)e->name->size,

2015-08-06 01:46:41

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 1/4] ASN.1: Fix handling of CHOICE in ASN.1 compiler

On Wed, 5 Aug 2015, David Howells wrote:

> Fix the handling of CHOICE types in the ASN.1 compiler to make SEQUENCE and
> SET elements in a CHOICE be correctly rendered as skippable and conditional
> as appropriate.

What are the security implications of these bugs?

It's pretty late in the -rc cycle.


--
James Morris
<[email protected]>

2015-08-06 05:33:45

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 1/4] ASN.1: Fix handling of CHOICE in ASN.1 compiler

James Morris <[email protected]> wrote:

> What are the security implications of these bugs?

I've fed them various bits of butchered ASN.1 and observed the effects as well
as checking what happens in the code. I don't think there are any security
implications. I've outlined my reasoning in each patch description.

David

2015-08-07 00:20:56

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 1/4] ASN.1: Fix handling of CHOICE in ASN.1 compiler

James Morris <[email protected]> wrote:

> It's pretty late in the -rc cycle.

If you'd rather just pull the patches into your next tree, and wait for the
next merge window, I'm okay with this. I've rebased my asn1-fixes branch on
your next branch and created a new tag there:

asn1-fixes-20150806

The old tag is still extant if you'd rather pull that. There are no other
changes to the two sets of commits.

David
---
The following changes since commit 730daa164e7c7e31c08fab940549f4acc3329432:

Yama: remove needless CONFIG_SECURITY_YAMA_STACKED (2015-07-28 13:18:19 +1000)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/asn1-fixes-20150806

for you to fetch changes up to 1e04d986c8de3b85748addf24e04f165836ff8a0:

ASN.1: Handle 'ANY OPTIONAL' in grammar (2015-08-06 12:59:07 +0100)

----------------------------------------------------------------
ASN.1 fixes

----------------------------------------------------------------
David Howells (4):
ASN.1: Fix handling of CHOICE in ASN.1 compiler
ASN.1: Fix actions on CHOICE elements with IMPLICIT tags
ASN.1: Fix non-match detection failure on data overrun
ASN.1: Handle 'ANY OPTIONAL' in grammar

include/linux/asn1_ber_bytecode.h | 16 +++++++++++-----
lib/asn1_decoder.c | 27 +++++++++++++++++++++++----
scripts/asn1_compiler.c | 23 ++++++++++++++---------
3 files changed, 48 insertions(+), 18 deletions(-)