summaryrefslogtreecommitdiffstats
path: root/lib/asn1_decoder.c
Commit message (Collapse)AuthorAgeFilesLines
* ASN.1: Handle 'ANY OPTIONAL' in grammarDavid Howells2015-08-051-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | 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 <dhowells@redhat.com> Tested-by: Marcel Holtmann <marcel@holtmann.org> Reviewed-by: David Woodhouse <David.Woodhouse@intel.com>
* ASN.1: Fix non-match detection failure on data overrunDavid Howells2015-08-051-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <marcel@holtmann.org> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Marcel Holtmann <marcel@holtmann.org> Reviewed-by: David Woodhouse <David.Woodhouse@intel.com>
* ASN.1: Fix actions on CHOICE elements with IMPLICIT tagsDavid Howells2015-08-051-1/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <dhowells@redhat.com> Reviewed-by: David Woodhouse <David.Woodhouse@intel.com>
* lib/asn1_decoder.c: kernel-doc warning fixFabian Frederick2014-06-041-1/+1
| | | | | | Signed-off-by: Fabian Frederick <fabf@skynet.be> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
* Merge tag 'modules-next-for-linus' of ↵Linus Torvalds2012-12-191-4/+4
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux Pull module update from Rusty Russell: "Nothing all that exciting; a new module-from-fd syscall for those who want to verify the source of the module (ChromeOS) and/or use standard IMA on it or other security hooks." * tag 'modules-next-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux: MODSIGN: Fix kbuild output when using default extra_certificates MODSIGN: Avoid using .incbin in C source modules: don't hand 0 to vmalloc. module: Remove a extra null character at the top of module->strtab. ASN.1: Use the ASN1_LONG_TAG and ASN1_INDEFINITE_LENGTH constants ASN.1: Define indefinite length marker constant moduleparam: use __UNIQUE_ID() __UNIQUE_ID() MODSIGN: Add modules_sign make target powerpc: add finit_module syscall. ima: support new kernel module syscall add finit_module syscall to asm-generic ARM: add finit_module syscall to ARM security: introduce kernel_module_from_file hook module: add flags arg to sys_finit_module() module: add syscall to load module from fd
| * ASN.1: Use the ASN1_LONG_TAG and ASN1_INDEFINITE_LENGTH constantsDavid Howells2012-12-141-4/+4
| | | | | | | | | | | | | | | | Use the ASN1_LONG_TAG and ASN1_INDEFINITE_LENGTH constants in the ASN.1 general decoder instead of the equivalent numbers. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* | ASN.1: Fix an indefinite length skip errorDavid Howells2012-12-051-1/+1
|/ | | | | | | | | | Fix an error in asn1_find_indefinite_length() whereby small definite length elements of size 0x7f are incorrecly classified as non-small. Without this fix, an error will be given as the length of the length will be perceived as being very much greater than the maximum supported size. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* X.509: Fix indefinite length element skip error handlingDavid Howells2012-10-101-9/+19
| | | | | | | | | | | | | | | | | asn1_find_indefinite_length() returns an error indicator of -1, which the caller asn1_ber_decoder() places in a size_t (which is usually unsigned) and then checks to see whether it is less than 0 (which it can't be). This can lead to the following warning: lib/asn1_decoder.c:320 asn1_ber_decoder() warn: unsigned 'len' is never less than zero. Instead, asn1_find_indefinite_length() update the caller's idea of the data cursor and length separately from returning the error code. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* X.509: Add an ASN.1 decoderDavid Howells2012-10-081-0/+477
Add an ASN.1 BER/DER/CER decoder. This uses the bytecode from the ASN.1 compiler in the previous patch to inform it as to what to expect to find in the encoded byte stream. The output from the compiler also tells it what functions to call on what tags, thus allowing the caller to retrieve information. The decoder is called as follows: int asn1_decoder(const struct asn1_decoder *decoder, void *context, const unsigned char *data, size_t datalen); The decoder argument points to the bytecode from the ASN.1 compiler. context is the caller's context and is passed to the action functions. data and datalen define the byte stream to be decoded. Note that the decoder is currently limited to datalen being less than 64K. This reduces the amount of stack space used by the decoder because ASN.1 is a nested construct. Similarly, the decoder is limited to a maximum of 10 levels of constructed data outside of a leaf node also in an effort to keep stack usage down. These restrictions can be raised if necessary. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>