Commit 3f3af97d authored by David Howells's avatar David Howells

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: default avatarDavid Howells <dhowells@redhat.com>
Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
parent 8d9b21dc
...@@ -61,7 +61,8 @@ enum asn1_opcode { ...@@ -61,7 +61,8 @@ enum asn1_opcode {
ASN1_OP_COND_FAIL = 0x1b, ASN1_OP_COND_FAIL = 0x1b,
ASN1_OP_COMPLETE = 0x1c, ASN1_OP_COMPLETE = 0x1c,
ASN1_OP_ACT = 0x1d, 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 */ /* The following eight have bit 0 -> SET, 1 -> OF, 2 -> ACT */
ASN1_OP_END_SEQ = 0x20, ASN1_OP_END_SEQ = 0x20,
......
...@@ -33,6 +33,7 @@ static const unsigned char asn1_op_lengths[ASN1_OP__NR] = { ...@@ -33,6 +33,7 @@ static const unsigned char asn1_op_lengths[ASN1_OP__NR] = {
[ASN1_OP_COND_FAIL] = 1, [ASN1_OP_COND_FAIL] = 1,
[ASN1_OP_COMPLETE] = 1, [ASN1_OP_COMPLETE] = 1,
[ASN1_OP_ACT] = 1 + 1, [ASN1_OP_ACT] = 1 + 1,
[ASN1_OP_MAYBE_ACT] = 1 + 1,
[ASN1_OP_RETURN] = 1, [ASN1_OP_RETURN] = 1,
[ASN1_OP_END_SEQ] = 1, [ASN1_OP_END_SEQ] = 1,
[ASN1_OP_END_SEQ_OF] = 1 + 1, [ASN1_OP_END_SEQ_OF] = 1 + 1,
...@@ -177,6 +178,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, ...@@ -177,6 +178,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
unsigned char flags = 0; unsigned char flags = 0;
#define FLAG_INDEFINITE_LENGTH 0x01 #define FLAG_INDEFINITE_LENGTH 0x01
#define FLAG_MATCHED 0x02 #define FLAG_MATCHED 0x02
#define FLAG_LAST_MATCHED 0x04 /* Last tag matched */
#define FLAG_CONS 0x20 /* Corresponds to CONS bit in the opcode tag #define FLAG_CONS 0x20 /* Corresponds to CONS bit in the opcode tag
* - ie. whether or not we are going to parse * - ie. whether or not we are going to parse
* a compound type. * a compound type.
...@@ -211,6 +213,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, ...@@ -211,6 +213,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
if ((op & ASN1_OP_MATCH__COND && if ((op & ASN1_OP_MATCH__COND &&
flags & FLAG_MATCHED) || flags & FLAG_MATCHED) ||
dp == datalen) { dp == datalen) {
flags &= ~FLAG_LAST_MATCHED;
pc += asn1_op_lengths[op]; pc += asn1_op_lengths[op];
goto next_op; goto next_op;
} }
...@@ -422,8 +425,15 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, ...@@ -422,8 +425,15 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
pc += asn1_op_lengths[op]; pc += asn1_op_lengths[op];
goto next_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: case ASN1_OP_ACT:
ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len); ret = actions[machine[pc + 1]](context, hdr, tag, data + tdp, len);
if (ret < 0)
return ret;
pc += asn1_op_lengths[op]; pc += asn1_op_lengths[op];
goto next_op; goto next_op;
...@@ -431,6 +441,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, ...@@ -431,6 +441,7 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
if (unlikely(jsp <= 0)) if (unlikely(jsp <= 0))
goto jump_stack_underflow; goto jump_stack_underflow;
pc = jump_stack[--jsp]; pc = jump_stack[--jsp];
flags |= FLAG_MATCHED | FLAG_LAST_MATCHED;
goto next_op; goto next_op;
default: default:
...@@ -438,7 +449,8 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder, ...@@ -438,7 +449,8 @@ int asn1_ber_decoder(const struct asn1_decoder *decoder,
} }
/* Shouldn't reach here */ /* 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; return -EBADMSG;
data_overrun_error: data_overrun_error:
......
...@@ -1468,7 +1468,8 @@ static void render_element(FILE *out, struct element *e, struct element *tag) ...@@ -1468,7 +1468,8 @@ static void render_element(FILE *out, struct element *e, struct element *tag)
case TYPE_REF: case TYPE_REF:
render_element(out, e->type->type->element, tag); render_element(out, e->type->type->element, tag);
if (e->action) if (e->action)
render_opcode(out, "ASN1_OP_ACT,\n"); render_opcode(out, "ASN1_OP_%sACT,\n",
skippable ? "MAYBE_" : "");
break; break;
case SEQUENCE: case SEQUENCE:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment