mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: mcgrof@gmail.com
Cc: mjg59@srcf.ucam.org, keyrings@linux-nfs.org,
	gregkh@linuxfoundation.org, kyle@kernel.org,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	seth.forshee@canonical.com,
	linux-security-module@vger.kernel.org, zohar@linux.vnet.ibm.com,
	dwmw2@infradead.org
Subject: [PATCH 18/27] ASN.1: Fix handling of CHOICE in ASN.1 compiler
Date: Fri, 17 Jul 2015 17:18:29 +0100	[thread overview]
Message-ID: <20150717161829.2578.86456.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <20150717161456.2578.95678.stgit@warthog.procyon.org.uk>

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.

Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-By: David Woodhouse <David.Woodhouse@intel.com>
---

 scripts/asn1_compiler.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/scripts/asn1_compiler.c b/scripts/asn1_compiler.c
index 866f85e5fdd2..ac9706d45c6b 100644
--- a/scripts/asn1_compiler.c
+++ b/scripts/asn1_compiler.c
@@ -682,7 +682,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
@@ -895,6 +895,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++;
@@ -1230,9 +1231,10 @@ static void dump_element(const struct element *e, int level)
 			asn1_methods[e->method],
 			e->tag);
 
-	printf("%c%c%c%c %c %*s[*] \e[33m%s\e[m %*.*s %*.*s\n",
+	printf("%c%c%c%c%c %c %*s[*] \e[33m%s\e[m %*.*s %*.*s\n",
 	       e->flags & ELEMENT_IMPLICIT ? 'I' : '-',
 	       e->flags & ELEMENT_EXPLICIT ? 'E' : '-',
+	       e->flags & ELEMENT_TAG_SPECIFIED ? 'T' : '-',
 	       e->flags & ELEMENT_SKIPPABLE ? 'S' : '-',
 	       e->flags & ELEMENT_CONDITIONAL ? 'C' : '-',
 	       "-tTqQcato"[e->compound],
@@ -1438,7 +1440,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;
 
@@ -1497,15 +1499,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 &&
@@ -1601,7 +1605,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)


  parent reply	other threads:[~2015-07-17 16:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-17 16:14 [PATCH 00/27] MODSIGN: Use PKCS#7 for module signatures David Howells
2015-07-17 16:15 ` [PATCH 01/27] X.509: Extract both parts of the AuthorityKeyIdentifier David Howells
2015-07-17 16:15 ` [PATCH 02/27] X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier David Howells
2015-07-17 16:15 ` [PATCH 03/27] PKCS#7: Allow detached data to be supplied for signature checking purposes David Howells
2015-07-17 16:15 ` [PATCH 04/27] MODSIGN: Provide a utility to append a PKCS#7 signature to a module David Howells
2015-07-17 16:15 ` [PATCH 05/27] MODSIGN: Use PKCS#7 messages as module signatures David Howells
2015-07-17 16:16 ` [PATCH 06/27] sign-file: Add option to only create signature file David Howells
2015-07-17 16:16 ` [PATCH 07/27] system_keyring.c doesn't need to #include module-internal.h David Howells
2015-07-17 16:16 ` [PATCH 08/27] MODSIGN: Extract the blob PKCS#7 signature verifier from module signing David Howells
2015-07-17 16:16 ` [PATCH 09/27] modsign: Abort modules_install when signing fails David Howells
2015-07-17 16:16 ` [PATCH 10/27] modsign: Allow password to be specified for signing key David Howells
2015-07-17 16:17 ` [PATCH 11/27] modsign: Allow signing key to be PKCS#11 David Howells
2015-07-17 16:17 ` [PATCH 12/27] modsign: Allow external signing key to be specified David Howells
2015-07-17 16:17 ` [PATCH 13/27] modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed David Howells
2015-07-17 16:17 ` [PATCH 14/27] modsign: Use single PEM file for autogenerated key David Howells
2015-07-17 16:17 ` [PATCH 15/27] modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option David Howells
2015-07-17 16:18 ` [PATCH 16/27] PKCS#7: Check content type and versions David Howells
2015-07-17 16:18 ` [PATCH 17/27] ASN.1: Add an ASN.1 compiler option to dump the element tree David Howells
2015-07-17 16:18 ` David Howells [this message]
2015-07-17 16:18 ` [PATCH 19/27] X.509: Change recorded SKID & AKID to not include Subject or Issuer David Howells
2015-07-17 16:18 ` [PATCH 20/27] PKCS#7: Support CMS messages also [RFC5652] David Howells
2015-07-17 16:19 ` [PATCH 21/27] sign-file: Generate CMS message as signature instead of PKCS#7 David Howells
2015-07-17 16:19 ` [PATCH 22/27] extract-cert: Cope with multiple X.509 certificates in a single file David Howells
2015-07-17 16:19 ` [PATCH 23/27] modsign: Use extract-cert to process CONFIG_SYSTEM_TRUSTED_KEYS David Howells
2015-07-17 16:19 ` [PATCH 24/27] X.509: Restrict the usage of a key based on information in X.509 certificate David Howells
2015-07-17 16:19 ` [PATCH 25/27] X.509: Parse the keyUsage extension to detect key-signing keys David Howells
2015-07-17 16:20 ` [PATCH 26/27] PKCS#7: Add an optional authenticated attribute to hold firmware name David Howells
2015-07-17 16:20 ` [PATCH 27/27] KEYS: Restrict signature verification to keys appropriate to the purpose David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150717161829.2578.86456.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=keyrings@linux-nfs.org \
    --cc=kyle@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mcgrof@gmail.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=seth.forshee@canonical.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®