1. 11 Aug, 2015 1 commit
    • David Howells's avatar
      sign-file: Document dependency on OpenSSL devel libraries · f81977b4
      David Howells authored
      The revised sign-file program is no longer a script that wraps the openssl
      program, but now rather a program that makes use of the OpenSSL's crypto
      library.  This means that to build the sign-file program, the kernel build
      process now has a dependency on openssl-devel in addition to openssl.
      
      Document this in Kconfig and in module-signing.txt.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      f81977b4
  2. 07 Aug, 2015 27 commits
    • David Howells's avatar
      PKCS#7: Appropriately restrict authenticated attributes and content type · 768b3497
      David Howells authored
      A PKCS#7 or CMS message can have per-signature authenticated attributes
      that are digested as a lump and signed by the authorising key for that
      signature.  If such attributes exist, the content digest isn't itself
      signed, but rather it is included in a special authattr which then
      contributes to the signature.
      
      Further, we already require the master message content type to be
      pkcs7_signedData - but there's also a separate content type for the data
      itself within the SignedData object and this must be repeated inside the
      authattrs for each signer [RFC2315 9.2, RFC5652 11.1].
      
      We should really validate the authattrs if they exist or forbid them
      entirely as appropriate.  To this end:
      
       (1) Alter the PKCS#7 parser to reject any message that has more than one
           signature where at least one signature has authattrs and at least one
           that does not.
      
       (2) Validate authattrs if they are present and strongly restrict them.
           Only the following authattrs are permitted and all others are
           rejected:
      
           (a) contentType.  This is checked to be an OID that matches the
           	 content type in the SignedData object.
      
           (b) messageDigest.  This must match the crypto digest of the data.
      
           (c) signingTime.  If present, we check that this is a valid, parseable
           	 UTCTime or GeneralTime and that the date it encodes fits within
           	 the validity window of the matching X.509 cert.
      
           (d) S/MIME capabilities.  We don't check the contents.
      
           (e) Authenticode SP Opus Info.  We don't check the contents.
      
           (f) Authenticode Statement Type.  We don't check the contents.
      
           The message is rejected if (a) or (b) are missing.  If the message is
           an Authenticode type, the message is rejected if (e) is missing; if
           not Authenticode, the message is rejected if (d) - (f) are present.
      
           The S/MIME capabilities authattr (d) unfortunately has to be allowed
           to support kernels already signed by the pesign program.  This only
           affects kexec.  sign-file suppresses them (CMS_NOSMIMECAP).
      
           The message is also rejected if an authattr is given more than once or
           if it contains more than one element in its set of values.
      
       (3) Add a parameter to pkcs7_verify() to select one of the following
           restrictions and pass in the appropriate option from the callers:
      
           (*) VERIFYING_MODULE_SIGNATURE
      
      	 This requires that the SignedData content type be pkcs7-data and
      	 forbids authattrs.  sign-file sets CMS_NOATTR.  We could be more
      	 flexible and permit authattrs optionally, but only permit minimal
      	 content.
      
           (*) VERIFYING_FIRMWARE_SIGNATURE
      
      	 This requires that the SignedData content type be pkcs7-data and
      	 requires authattrs.  In future, this will require an attribute
      	 holding the target firmware name in addition to the minimal set.
      
           (*) VERIFYING_UNSPECIFIED_SIGNATURE
      
      	 This requires that the SignedData content type be pkcs7-data but
      	 allows either no authattrs or only permits the minimal set.
      
           (*) VERIFYING_KEXEC_PE_SIGNATURE
      
      	 This only supports the Authenticode SPC_INDIRECT_DATA content type
      	 and requires at least an SpcSpOpusInfo authattr in addition to the
      	 minimal set.  It also permits an SPC_STATEMENT_TYPE authattr (and
      	 an S/MIME capabilities authattr because the pesign program doesn't
      	 remove these).
      
           (*) VERIFYING_KEY_SIGNATURE
           (*) VERIFYING_KEY_SELF_SIGNATURE
      
      	 These are invalid in this context but are included for later use
      	 when limiting the use of X.509 certs.
      
       (4) The pkcs7_test key type is given a module parameter to select between
           the above options for testing purposes.  For example:
      
      	echo 1 >/sys/module/pkcs7_test_key/parameters/usage
      	keyctl padd pkcs7_test foo @s </tmp/stuff.pkcs7
      
           will attempt to check the signature on stuff.pkcs7 as if it contains a
           firmware blob (1 being VERIFYING_FIRMWARE_SIGNATURE).
      Suggested-by: default avatarAndy Lutomirski <luto@kernel.org>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      768b3497
    • David Howells's avatar
      KEYS: Add a name for PKEY_ID_PKCS7 · 2b3f1d06
      David Howells authored
      Add a name for PKEY_ID_PKCS7 into the pkey_id_type_name array.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      2b3f1d06
    • David Howells's avatar
      PKCS#7: Improve and export the X.509 ASN.1 time object decoder · adf691f8
      David Howells authored
      Make the X.509 ASN.1 time object decoder fill in a time64_t rather than a
      struct tm to make comparison easier (unfortunately, this makes readable
      display less easy) and export it so that it can be used by the PKCS#7 code
      too.
      
      Further, tighten up its parsing to reject invalid dates (eg. weird
      characters, non-existent hour numbers) and unsupported dates (eg. timezones
      other than 'Z' or dates earlier than 1970).
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      adf691f8
    • David Woodhouse's avatar
      modsign: Use extract-cert to process CONFIG_SYSTEM_TRUSTED_KEYS · c6275a94
      David Woodhouse authored
      Fix up the dependencies somewhat too, while we're at it.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      c6275a94
    • David Woodhouse's avatar
      extract-cert: Cope with multiple X.509 certificates in a single file · 084284a8
      David Woodhouse authored
      This is not required for the module signing key, although it doesn't do any
      harm — it just means that any additional certs in the PEM file are also
      trusted by the kernel.
      
      But it does allow us to use the extract-cert tool for processing the extra
      certs from CONFIG_SYSTEM_TRUSTED_KEYS, instead of that horrid awk|base64
      hack.
      
      Also cope with being invoked with no input file, creating an empty output
      file as a result.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      084284a8
    • David Howells's avatar
      sign-file: Generate CMS message as signature instead of PKCS#7 · 7007d5c5
      David Howells authored
      Make sign-file use the OpenSSL CMS routines to generate a message to be
      used as the signature blob instead of the PKCS#7 routines.  This allows us
      to change how the matching X.509 certificate is selected.  With PKCS#7 the
      only option is to match on the serial number and issuer fields of an X.509
      certificate; with CMS, we also have the option of matching by subjectKeyId
      extension.  The new behaviour is selected with the "-k" flag.
      
      Without the -k flag specified, the output is pretty much identical to the
      PKCS#7 output.
      
      Whilst we're at it, don't include the S/MIME capability list in the message
      as it's irrelevant to us.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: David Woodhouse <David.Woodhouse@intel.com
      7007d5c5
    • David Howells's avatar
      PKCS#7: Support CMS messages also [RFC5652] · 4ab65db4
      David Howells authored
      Since CMS is an evolution of PKCS#7, with much of the ASN.1 being
      compatible, add support for CMS signed-data messages also [RFC5652 sec 5].
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      4ab65db4
    • David Howells's avatar
      X.509: Change recorded SKID & AKID to not include Subject or Issuer · 34c6e4cc
      David Howells authored
      The key identifiers fabricated from an X.509 certificate are currently:
      
       (A) Concatenation of serial number and issuer
      
       (B) Concatenation of subject and subjectKeyID (SKID)
      
      When verifying one X.509 certificate with another, the AKID in the target
      can be used to match the authoritative certificate.  The AKID can specify
      the match in one or both of two ways:
      
       (1) Compare authorityCertSerialNumber and authorityCertIssuer from the AKID
           to identifier (A) above.
      
       (2) Compare keyIdentifier from the AKID plus the issuer from the target
           certificate to identifier (B) above.
      
      When verifying a PKCS#7 message, the only available comparison is between
      the IssuerAndSerialNumber field and identifier (A) above.
      
      However, a subsequent patch adds CMS support.  Whilst CMS still supports a
      match on IssuerAndSerialNumber as for PKCS#7, it also supports an
      alternative - which is the SubjectKeyIdentifier field.  This is used to
      match to an X.509 certificate on the SKID alone.  No subject information is
      available to be used.
      
      To this end change the fabrication of (B) above to be from the X.509 SKID
      alone.  The AKID in keyIdentifier form then only matches on that and does
      not include the issuer.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      34c6e4cc
    • David Howells's avatar
      PKCS#7: Check content type and versions · d08cf632
      David Howells authored
      We only support PKCS#7 signed-data [RFC2315 sec 9] content at the top level,
      so reject anything else.  Further, check that the version numbers in
      SignedData and SignerInfo are 1 in both cases.
      
      Note that we don't restrict the inner content type.  In the PKCS#7 code we
      don't parse the data attached there, but merely verify the signature over
      it.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      d08cf632
    • David Woodhouse's avatar
      modsign: Add explicit CONFIG_SYSTEM_TRUSTED_KEYS option · 73314774
      David Woodhouse authored
      Let the user explicitly provide a file containing trusted keys, instead of
      just automatically finding files matching *.x509 in the build tree and
      trusting whatever we find. This really ought to be an *explicit*
      configuration, and the build rules for dealing with the files were
      fairly painful too.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      73314774
    • David Woodhouse's avatar
      modsign: Use single PEM file for autogenerated key · fb117949
      David Woodhouse authored
      The current rule for generating signing_key.priv and signing_key.x509 is
      a classic example of a bad rule which has a tendency to break parallel
      make. When invoked to create *either* target, it generates the other
      target as a side-effect that make didn't predict.
      
      So let's switch to using a single file signing_key.pem which contains
      both key and certificate. That matches what we do in the case of an
      external key specified by CONFIG_MODULE_SIG_KEY anyway, so it's also
      slightly cleaner.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      fb117949
    • David Woodhouse's avatar
      modsign: Extract signing cert from CONFIG_MODULE_SIG_KEY if needed · 1329e8cc
      David Woodhouse authored
      Where an external PEM file or PKCS#11 URI is given, we can get the cert
      from it for ourselves instead of making the user drop signing_key.x509
      in place for us.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      1329e8cc
    • David Woodhouse's avatar
    • David Woodhouse's avatar
      modsign: Allow signing key to be PKCS#11 · 6e3e281f
      David Woodhouse authored
      This is only the key; the corresponding *cert* still needs to be in
      $(topdir)/signing_key.x509. And there's no way to actually use this
      from the build system yet.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      6e3e281f
    • David Woodhouse's avatar
      modsign: Allow password to be specified for signing key · af1eb291
      David Woodhouse authored
      We don't want this in the Kconfig since it might then get exposed in
      /proc/config.gz. So make it a parameter to Kbuild instead. This also
      means we don't have to jump through hoops to strip quotes from it, as
      we would if it was a config option.
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      af1eb291
    • David Woodhouse's avatar
    • David Howells's avatar
      MODSIGN: Extract the blob PKCS#7 signature verifier from module signing · 091f6e26
      David Howells authored
      Extract the function that drives the PKCS#7 signature verification given a
      data blob and a PKCS#7 blob out from the module signing code and lump it with
      the system keyring code as it's generic.  This makes it independent of module
      config options and opens it to use by the firmware loader.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Cc: Luis R. Rodriguez <mcgrof@suse.com>
      Cc: Rusty Russell <rusty@rustcorp.com.au>
      Cc: Ming Lei <ming.lei@canonical.com>
      Cc: Seth Forshee <seth.forshee@canonical.com>
      Cc: Kyle McMartin <kyle@kernel.org>
      091f6e26
    • David Howells's avatar
      system_keyring.c doesn't need to #include module-internal.h · 1c394499
      David Howells authored
      system_keyring.c doesn't need to #include module-internal.h as it doesn't use
      the one thing that exports.  Remove the inclusion.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      1c394499
    • Luis R. Rodriguez's avatar
      sign-file: Add option to only create signature file · 23dfbbab
      Luis R. Rodriguez authored
      Make the -d option (which currently isn't actually wired to anything) write
      out the PKCS#7 message as per the -p option and then exit without either
      modifying the source or writing out a compound file of the source, signature
      and metadata.
      
      This will be useful when firmware signature support is added
      upstream as firmware will be left intact, and we'll only require
      the signature file. The descriptor is implicit by file extension
      and the file's own size.
      Signed-off-by: default avatarLuis R. Rodriguez <mcgrof@suse.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      23dfbbab
    • David Howells's avatar
      MODSIGN: Use PKCS#7 messages as module signatures · 3f1e1bea
      David Howells authored
      Move to using PKCS#7 messages as module signatures because:
      
       (1) We have to be able to support the use of X.509 certificates that don't
           have a subjKeyId set.  We're currently relying on this to look up the
           X.509 certificate in the trusted keyring list.
      
       (2) PKCS#7 message signed information blocks have a field that supplies the
           data required to match with the X.509 certificate that signed it.
      
       (3) The PKCS#7 certificate carries fields that specify the digest algorithm
           used to generate the signature in a standardised way and the X.509
           certificates specify the public key algorithm in a standardised way - so
           we don't need our own methods of specifying these.
      
       (4) We now have PKCS#7 message support in the kernel for signed kexec purposes
           and we can make use of this.
      
      To make this work, the old sign-file script has been replaced with a program
      that needs compiling in a previous patch.  The rules to build it are added
      here.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarVivek Goyal <vgoyal@redhat.com>
      3f1e1bea
    • David Howells's avatar
      MODSIGN: Provide a utility to append a PKCS#7 signature to a module · bc1c373d
      David Howells authored
      Provide a utility that:
      
       (1) Digests a module using the specified hash algorithm (typically sha256).
      
           [The digest can be dumped into a file by passing the '-d' flag]
      
       (2) Generates a PKCS#7 message that:
      
           (a) Has detached data (ie. the module content).
      
           (b) Is signed with the specified private key.
      
           (c) Refers to the specified X.509 certificate.
      
           (d) Has an empty X.509 certificate list.
      
           [The PKCS#7 message can be dumped into a file by passing the '-p' flag]
      
       (3) Generates a signed module by concatenating the old module, the PKCS#7
           message, a descriptor and a magic string.  The descriptor contains the
           size of the PKCS#7 message and indicates the id_type as PKEY_ID_PKCS7.
      
       (4) Either writes the signed module to the specified destination or renames
           it over the source module.
      
      This allows module signing to reuse the PKCS#7 handling code that was added
      for PE file parsing for signed kexec.
      
      Note that the utility is written in C and must be linked against the OpenSSL
      crypto library.
      
      Note further that I have temporarily dropped support for handling externally
      created signatures until we can work out the best way to do those.  Hopefully,
      whoever creates the signature can give me a PKCS#7 certificate.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarVivek Goyal <vgoyal@redhat.com>
      bc1c373d
    • David Howells's avatar
      PKCS#7: Allow detached data to be supplied for signature checking purposes · 4ebdb76f
      David Howells authored
      It is possible for a PKCS#7 message to have detached data.  However, to verify
      the signatures on a PKCS#7 message, we have to be able to digest the data.
      Provide a function to supply that data.  An error is given if the PKCS#7
      message included embedded data.
      
      This is used in a subsequent patch to supply the data to module signing where
      the signature is in the form of a PKCS#7 message with detached data, whereby
      the detached data is the module content that is signed.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarVivek Goyal <vgoyal@redhat.com>
      4ebdb76f
    • David Howells's avatar
      X.509: Support X.509 lookup by Issuer+Serial form AuthorityKeyIdentifier · 4573b64a
      David Howells authored
      If an X.509 certificate has an AuthorityKeyIdentifier extension that provides
      an issuer and serialNumber, then make it so that these are used in preference
      to the keyIdentifier field also held therein for searching for the signing
      certificate.
      
      If both the issuer+serialNumber and the keyIdentifier are supplied, then the
      certificate is looked up by the former but the latter is checked as well.  If
      the latter doesn't match the subjectKeyIdentifier of the parent certificate,
      EKEYREJECTED is returned.
      
      This makes it possible to chain X.509 certificates based on the issuer and
      serialNumber fields rather than on subjectKeyIdentifier.  This is necessary as
      we are having to deal with keys that are represented by X.509 certificates
      that lack a subjectKeyIdentifier.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarVivek Goyal <vgoyal@redhat.com>
      4573b64a
    • David Howells's avatar
      X.509: Extract both parts of the AuthorityKeyIdentifier · b92e6570
      David Howells authored
      Extract both parts of the AuthorityKeyIdentifier, not just the keyIdentifier,
      as the second part can be used to match X.509 certificates by issuer and
      serialNumber.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarVivek Goyal <vgoyal@redhat.com>
      b92e6570
    • David Howells's avatar
      ASN.1: Copy string names to tokens in ASN.1 compiler · c05cae9a
      David Howells authored
      Copy string names to tokens in ASN.1 compiler rather than storing a pointer
      into the source text.  This means we don't have to use "%*.*s" all over the
      place.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      c05cae9a
    • David Howells's avatar
      ASN.1: Add an ASN.1 compiler option to dump the element tree · ae44a2f6
      David Howells authored
      Add an ASN.1 compiler option to dump the element tree to stdout.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      ae44a2f6
    • James Morris's avatar
      Merge tag 'asn1-fixes-20150805' of... · 459c15e5
      James Morris authored
      Merge tag 'asn1-fixes-20150805' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs into next
      459c15e5
  3. 05 Aug, 2015 4 commits
    • David Howells's avatar
      ASN.1: Handle 'ANY OPTIONAL' in grammar · 233ce79d
      David Howells authored
      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: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      233ce79d
    • David Howells's avatar
      ASN.1: Fix non-match detection failure on data overrun · 0d62e9dd
      David Howells authored
      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: default avatarMarcel Holtmann <marcel@holtmann.org>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Tested-by: default avatarMarcel Holtmann <marcel@holtmann.org>
      Reviewed-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      0d62e9dd
    • David Howells's avatar
      ASN.1: Fix actions on CHOICE elements with IMPLICIT tags · 3f3af97d
      David Howells authored
      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>
      3f3af97d
    • David Howells's avatar
      ASN.1: Fix handling of CHOICE in ASN.1 compiler · 8d9b21dc
      David Howells authored
      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: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-By: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      8d9b21dc
  4. 28 Jul, 2015 1 commit
  5. 20 Jul, 2015 1 commit
  6. 19 Jul, 2015 6 commits