1. 07 Oct, 2014 4 commits
  2. 06 Oct, 2014 5 commits
  3. 03 Oct, 2014 2 commits
    • David Howells's avatar
      X.509: If available, use the raw subjKeyId to form the key description · dd2f6c44
      David Howells authored
      Module signing matches keys by comparing against the key description exactly.
      However, the way the key description gets constructed got changed to be
      composed of the subject name plus the certificate serial number instead of the
      subject name and the subjectKeyId.  I changed this to avoid problems with
      certificates that don't *have* a subjectKeyId.
      
      Instead, if available, use the raw subjectKeyId to form the key description
      and only use the serial number if the subjectKeyId doesn't exist.
      Reported-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      dd2f6c44
    • Dmitry Kasatkin's avatar
      KEYS: handle error code encoded in pointer · 40b50e80
      Dmitry Kasatkin authored
      If hexlen is odd then function returns an error.
      Use IS_ERR to check for error, otherwise invalid pointer
      is used and kernel gives oops:
      
      [  132.816522] BUG: unable to handle kernel paging request at
      ffffffffffffffea
      [  132.819902] IP: [<ffffffff812bfc20>] asymmetric_key_id_same+0x14/0x36
      [  132.820302] PGD 1a12067 PUD 1a14067 PMD 0
      [  132.820302] Oops: 0000 [#1] SMP
      [  132.820302] Modules linked in: bridge(E) stp(E) llc(E) evdev(E)
      serio_raw(E) i2c_piix4(E) button(E) fuse(E)
      [  132.820302] CPU: 0 PID: 2993 Comm: cat Tainted: G            E
      3.16.0-kds+ #2847
      [  132.820302] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
      [  132.820302] task: ffff88004249a430 ti: ffff880056640000 task.ti:
      ffff880056640000
      [  132.820302] RIP: 0010:[<ffffffff812bfc20>]  [<ffffffff812bfc20>]
      asymmetric_key_id_same+0x14/0x36
      [  132.820302] RSP: 0018:ffff880056643930  EFLAGS: 00010246
      [  132.820302] RAX: 0000000000000000 RBX: ffffffffffffffea RCX:
      ffff880056643ae0
      [  132.820302] RDX: 000000000000005e RSI: ffffffffffffffea RDI:
      ffff88005bac9300
      [  132.820302] RBP: ffff880056643948 R08: 0000000000000003 R09:
      00000007504aa01a
      [  132.820302] R10: 0000000000000000 R11: 0000000000000000 R12:
      ffff88005d68ca40
      [  132.820302] R13: 0000000000000101 R14: 0000000000000000 R15:
      ffff88005bac5280
      [  132.820302] FS:  00007f67a153c740(0000) GS:ffff88005da00000(0000)
      knlGS:0000000000000000
      [  132.820302] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
      [  132.820302] CR2: ffffffffffffffea CR3: 000000002e663000 CR4:
      00000000000006f0
      [  132.820302] Stack:
      [  132.820302]  ffffffff812bfc66 ffff880056643ae0 ffff88005bac5280
      ffff880056643958
      [  132.820302]  ffffffff812bfc9d ffff880056643980 ffffffff812971d9
      ffff88005ce930c1
      [  132.820302]  ffff88005ce930c0 0000000000000000 ffff8800566439c8
      ffffffff812fb753
      [  132.820302] Call Trace:
      [  132.820302]  [<ffffffff812bfc66>] ? asymmetric_match_key_ids+0x24/0x42
      [  132.820302]  [<ffffffff812bfc9d>] asymmetric_key_cmp+0x19/0x1b
      [  132.820302]  [<ffffffff812971d9>] keyring_search_iterator+0x74/0xd7
      [  132.820302]  [<ffffffff812fb753>] assoc_array_subtree_iterate+0x67/0xd2
      [  132.820302]  [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
      [  132.820302]  [<ffffffff812fbaa1>] assoc_array_iterate+0x19/0x1e
      [  132.820302]  [<ffffffff81297332>] search_nested_keyrings+0xf6/0x2b6
      [  132.820302]  [<ffffffff810728da>] ? sched_clock_cpu+0x91/0xa2
      [  132.820302]  [<ffffffff810860d2>] ? mark_held_locks+0x58/0x6e
      [  132.820302]  [<ffffffff810a137d>] ? current_kernel_time+0x77/0xb8
      [  132.820302]  [<ffffffff81297871>] keyring_search_aux+0xe1/0x14c
      [  132.820302]  [<ffffffff812977fc>] ? keyring_search_aux+0x6c/0x14c
      [  132.820302]  [<ffffffff8129796b>] keyring_search+0x8f/0xb6
      [  132.820302]  [<ffffffff812bfc84>] ? asymmetric_match_key_ids+0x42/0x42
      [  132.820302]  [<ffffffff81297165>] ? key_default_cmp+0x20/0x20
      [  132.820302]  [<ffffffff812ab9e3>] asymmetric_verify+0xa4/0x214
      [  132.820302]  [<ffffffff812ab90e>] integrity_digsig_verify+0xb1/0xe2
      [  132.820302]  [<ffffffff812abe41>] ? evm_verifyxattr+0x6a/0x7a
      [  132.820302]  [<ffffffff812b0390>] ima_appraise_measurement+0x160/0x370
      [  132.820302]  [<ffffffff81161db2>] ? d_absolute_path+0x5b/0x7a
      [  132.820302]  [<ffffffff812ada30>] process_measurement+0x322/0x404
      Reported-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      40b50e80
  4. 02 Oct, 2014 1 commit
  5. 30 Sep, 2014 2 commits
  6. 22 Sep, 2014 3 commits
  7. 21 Sep, 2014 1 commit
    • David Howells's avatar
      KEYS: Check hex2bin()'s return when generating an asymmetric key ID · d1ac5540
      David Howells authored
      As it stands, the code to generate an asymmetric key ID prechecks the hex
      string it is given whilst determining the length, before it allocates the
      buffer for hex2bin() to translate into - which mean that checking the result of
      hex2bin() is redundant.
      
      Unfortunately, hex2bin() is marked as __must_check, which means that the
      following warning may be generated if the return value isn't checked:
      
      	crypto/asymmetric_keys/asymmetric_type.c: In function
      	asymmetric_key_hex_to_key_id:
      	crypto/asymmetric_keys/asymmetric_type.c:110: warning: ignoring return
      	value of hex2bin, declared with attribute warn_unused_result
      
      The warning can't be avoided by casting the result to void.
      
      Instead, use strlen() to check the length of the string and ignore the fact
      that the string might not be entirely valid hex until after the allocation has
      been done - in which case we can use the result of hex2bin() for this.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      d1ac5540
  8. 18 Sep, 2014 3 commits
    • Roberto Sassu's avatar
      ima: detect violations for mmaped files · 1b68bdf9
      Roberto Sassu authored
      This patch fixes the detection of the 'open_writers' violation for mmaped
      files.
      
      before) an 'open_writers' violation is detected if the policy contains
              a rule with the criteria: func=FILE_CHECK mask=MAY_READ
      
      after) an 'open_writers' violation is detected if the current event
             matches one of the policy rules.
      
      With the old behaviour, the 'open_writers' violation is not detected
      in the following case:
      
      policy:
      measure func=FILE_MMAP mask=MAY_EXEC
      
      steps:
      1) open a shared library for writing
      2) execute a binary that links that shared library
      3) during the binary execution, modify the shared library and save
         the change
      
      result:
      the 'open_writers' violation measurement is not present in the IMA list.
      
      Only binaries executed are protected from writes. For libraries mapped
      in memory there is the flag MAP_DENYWRITE for this purpose, but according
      to the output of 'man mmap', the mmap flag is ignored.
      
      Since ima_rdwr_violation_check() is now called by process_measurement()
      the information about if the inode must be measured is already provided
      by ima_get_action(). Thus the unnecessary function ima_must_measure()
      has been removed.
      
      Changes in v3 (Dmitry Kasatkin):
      - Violation for MMAP_CHECK function are verified since this patch
      - Changed patch description a bit
      Signed-off-by: default avatarRoberto Sassu <roberto.sassu@polito.it>
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      1b68bdf9
    • Roberto Sassu's avatar
      ima: fix race condition on ima_rdwr_violation_check and process_measurement · f7a859ff
      Roberto Sassu authored
      This patch fixes a race condition between two functions that try to access
      the same inode. Since the i_mutex lock is held and released separately
      in the two functions, there may be the possibility that a violation is
      not correctly detected.
      
      Suppose there are two processes, A (reader) and B (writer), if the
      following sequence happens:
      
      A: ima_rdwr_violation_check()
      B: ima_rdwr_violation_check()
      B: process_measurement()
      B: starts writing the inode
      A: process_measurement()
      
      the ToMToU violation (a reader may be accessing a content different from
      that measured, due to a concurrent modification by a writer) will not be
      detected. To avoid this issue, the violation check and the measurement
      must be done atomically.
      
      This patch fixes the problem by moving the violation check inside
      process_measurement() when the i_mutex lock is held. Differently from
      the old code, the violation check is executed also for the MMAP_CHECK
      hook (other than for FILE_CHECK). This allows to detect ToMToU violations
      that are possible because shared libraries can be opened for writing
      while they are in use (according to the output of 'man mmap', the mmap()
      flag MAP_DENYWRITE is ignored).
      
      Changes in v5 (Roberto Sassu):
      * get iint if action is not zero
      * exit process_measurement() after the violation check if action is zero
      * reverse order process_measurement() exit cleanup (Mimi)
      
      Changes in v4 (Dmitry Kasatkin):
      * iint allocation is done before calling ima_rdrw_violation_check()
        (Suggested-by Mimi)
      * do not check for violations if the policy does not contain 'measure'
        rules (done by Roberto Sassu)
      
      Changes in v3 (Dmitry Kasatkin):
      * no violation checking for MMAP_CHECK function in this patch
      * remove use of filename from violation
      * removes checking if ima is enabled from ima_rdrw_violation_check
      * slight style change
      Suggested-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarRoberto Sassu <roberto.sassu@polito.it>
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      f7a859ff
    • James Morris's avatar
  9. 17 Sep, 2014 4 commits
    • Roberto Sassu's avatar
      ima: added ima_policy_flag variable · a756024e
      Roberto Sassu authored
      This patch introduces the new variable 'ima_policy_flag', whose bits
      are set depending on the action of the current policy rules. Only the
      flags IMA_MEASURE, IMA_APPRAISE and IMA_AUDIT are set.
      
      The new variable will be used to improve performance by skipping the
      unnecessary execution of IMA code if the policy does not contain rules
      with the above actions.
      
      Changes in v6 (Roberto Sassu)
      * do not check 'ima_initialized' before calling ima_update_policy_flag()
        in ima_update_policy() (suggested by Dmitry)
      * calling ima_update_policy_flag() moved to init_ima to co-locate with
        ima_initialized (Dmitry)
      * add/revise comments (Mimi)
      
      Changes in v5 (Roberto Sassu)
      * reset IMA_APPRAISE flag in 'ima_policy_flag' if 'ima_appraise' is set
        to zero (reported by Dmitry)
      * update 'ima_policy_flag' only if IMA initialization is successful
        (suggested by Mimi and Dmitry)
      * check 'ima_policy_flag' instead of 'ima_initialized'
        (suggested by Mimi and Dmitry)
      Signed-off-by: default avatarRoberto Sassu <roberto.sassu@polito.it>
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      a756024e
    • Roberto Sassu's avatar
      ima: return an error code from ima_add_boot_aggregate() · be39ffc2
      Roberto Sassu authored
      This patch modifies ima_add_boot_aggregate() to return an error code.
      This way we can determine if all the initialization procedures have
      been executed successfully.
      Signed-off-by: default avatarRoberto Sassu <roberto.sassu@polito.it>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      be39ffc2
    • Dmitry Kasatkin's avatar
      ima: provide 'ima_appraise=log' kernel option · 2faa6ef3
      Dmitry Kasatkin authored
      The kernel boot parameter "ima_appraise" currently defines 'off',
      'enforce' and 'fix' modes.  When designing a policy and labeling
      the system, access to files are either blocked in the default
      'enforce' mode or automatically fixed in the 'fix' mode.  It is
      beneficial to be able to run the system in a logging only mode,
      without fixing it, in order to properly analyze the system. This
      patch adds a 'log' mode to run the system in a permissive mode and
      log the appraisal results.
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      2faa6ef3
    • Dmitry Kasatkin's avatar
      ima: move keyring initialization to ima_init() · 31b70f66
      Dmitry Kasatkin authored
      ima_init() is used as a single place for all initializations.
      Experimental keyring patches used the 'late_initcall' which was
      co-located with the late_initcall(init_ima). When the late_initcall
      for the keyring initialization was abandoned, initialization moved
      to init_ima, though it would be more logical to move it to ima_init,
      where the rest of the initialization is done. This patch moves the
      keyring initialization to ima_init() as a preparatory step for
      loading the keys which will be added to ima_init() in following
      patches.
      Signed-off-by: default avatarDmitry Kasatkin <d.kasatkin@samsung.com>
      Signed-off-by: default avatarMimi Zohar <zohar@linux.vnet.ibm.com>
      31b70f66
  10. 16 Sep, 2014 15 commits
    • David Howells's avatar
      Merge tag 'keys-pkcs7-20140916' into keys-next · d3e4f419
      David Howells authored
      Changes for next to improve the matching of asymmetric keys and to improve the
      handling of PKCS#7 certificates:
      
       (1) Provide a method to preparse the data supplied for matching a key.  This
           permits they key type to extract out the bits it needs for matching once
           only.
      
           Further, the type of search (direct lookup or iterative) can be set and
           the function used to actually check the match can be set by preparse
           rather than being hard coded for the type.
      
       (2) Improves asymmetric keys identification.
      
           Keys derived from X.509 certs now get labelled with IDs derived from their
           issuer and certificate number (required to match PKCS#7) and from their
           SKID and subject (required to match X.509).
      
           IDs are now binary and match criterion preparsing is provided so that
           criteria can be turned into binary blobs to make matching faster.
      
       (3) Improves PKCS#7 message handling to permit PKCS#7 messages without X.509
           cert lists to be matched to trusted keys, thereby allowing minimally sized
           PKCS#7 certs to be used.
      
       (4) Improves PKCS#7 message handling to better handle certificate chains that
           are broken due to unsupported crypto that can otherwise by used to
           intersect a trust keyring.
      
      These must go on top of the PKCS#7 parser cleanup fixes.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      d3e4f419
    • David Howells's avatar
      PKCS#7: Handle PKCS#7 messages that contain no X.509 certs · 757932e6
      David Howells authored
      The X.509 certificate list in a PKCS#7 message is optional.  To save space, we
      can omit the inclusion of any X.509 certificates if we are sure that we can
      look the relevant public key up by the serial number and issuer given in a
      signed info block.
      
      This also supports use of a signed info block for which we can't find a
      matching X.509 cert in the certificate list, though it be populated.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      757932e6
    • David Howells's avatar
      PKCS#7: Better handling of unsupported crypto · 41559420
      David Howells authored
      Provide better handling of unsupported crypto when verifying a PKCS#7 message.
      If we can't bridge the gap between a pair of X.509 certs or between a signed
      info block and an X.509 cert because it involves some crypto we don't support,
      that's not necessarily the end of the world as there may be other ways points
      at which we can intersect with a ring of trusted keys.
      
      Instead, only produce ENOPKG immediately if all the signed info blocks in a
      PKCS#7 message require unsupported crypto to bridge to the first X.509 cert.
      Otherwise, we defer the generation of ENOPKG until we get ENOKEY during trust
      validation.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      41559420
    • David Howells's avatar
      KEYS: Overhaul key identification when searching for asymmetric keys · 46963b77
      David Howells authored
      Make use of the new match string preparsing to overhaul key identification
      when searching for asymmetric keys.  The following changes are made:
      
       (1) Use the previously created asymmetric_key_id struct to hold the following
           key IDs derived from the X.509 certificate or PKCS#7 message:
      
      	id: serial number + issuer
      	skid: subjKeyId + subject
      	authority: authKeyId + issuer
      
       (2) Replace the hex fingerprint attached to key->type_data[1] with an
           asymmetric_key_ids struct containing the id and the skid (if present).
      
       (3) Make the asymmetric_type match data preparse select one of two searches:
      
           (a) An iterative search for the key ID given if prefixed with "id:".  The
           	 prefix is expected to be followed by a hex string giving the ID to
           	 search for.  The criterion key ID is checked against all key IDs
           	 recorded on the key.
      
           (b) A direct search if the key ID is not prefixed with "id:".  This will
           	 look for an exact match on the key description.
      
       (4) Make x509_request_asymmetric_key() take a key ID.  This is then converted
           into "id:<hex>" and passed into keyring_search() where match preparsing
           will turn it back into a binary ID.
      
       (5) X.509 certificate verification then takes the authority key ID and looks
           up a key that matches it to find the public key for the certificate
           signature.
      
       (6) PKCS#7 certificate verification then takes the id key ID and looks up a
           key that matches it to find the public key for the signed information
           block signature.
      
      Additional changes:
      
       (1) Multiple subjKeyId and authKeyId values on an X.509 certificate cause the
           cert to be rejected with -EBADMSG.
      
       (2) The 'fingerprint' ID is gone.  This was primarily intended to convey PGP
           public key fingerprints.  If PGP is supported in future, this should
           generate a key ID that carries the fingerprint.
      
       (3) Th ca_keyid= kernel command line option is now converted to a key ID and
           used to match the authority key ID.  Possibly this should only match the
           actual authKeyId part and not the issuer as well.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      46963b77
    • David Howells's avatar
      KEYS: Implement binary asymmetric key ID handling · 7901c1a8
      David Howells authored
      Implement the first step in using binary key IDs for asymmetric keys rather
      than hex string keys.
      
      The previously added match data preparsing will be able to convert hex
      criterion strings into binary which can then be compared more rapidly.
      
      Further, we actually want more then one ID string per public key.  The problem
      is that X.509 certs refer to other X.509 certs by matching Issuer + AuthKeyId
      to Subject + SubjKeyId, but PKCS#7 messages match against X.509 Issuer +
      SerialNumber.
      
      This patch just provides facilities for a later patch to make use of.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      7901c1a8
    • David Howells's avatar
    • David Howells's avatar
      KEYS: Make the key matching functions return bool · 0c903ab6
      David Howells authored
      Make the key matching functions pointed to by key_match_data::cmp return bool
      rather than int.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      0c903ab6
    • David Howells's avatar
      KEYS: Remove key_type::match in favour of overriding default by match_preparse · c06cfb08
      David Howells authored
      A previous patch added a ->match_preparse() method to the key type.  This is
      allowed to override the function called by the iteration algorithm.
      Therefore, we can just set a default that simply checks for an exact match of
      the key description with the original criterion data and allow match_preparse
      to override it as needed.
      
      The key_type::match op is then redundant and can be removed, as can the
      user_match() function.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      c06cfb08
    • David Howells's avatar
      KEYS: Remove key_type::def_lookup_type · 614d8c39
      David Howells authored
      Remove key_type::def_lookup_type as it's no longer used.  The information now
      defaults to KEYRING_SEARCH_LOOKUP_DIRECT but may be overridden by
      type->match_preparse().
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      614d8c39
    • David Howells's avatar
      KEYS: Preparse match data · 46291959
      David Howells authored
      Preparse the match data.  This provides several advantages:
      
       (1) The preparser can reject invalid criteria up front.
      
       (2) The preparser can convert the criteria to binary data if necessary (the
           asymmetric key type really wants to do binary comparison of the key IDs).
      
       (3) The preparser can set the type of search to be performed.  This means
           that it's not then a one-off setting in the key type.
      
       (4) The preparser can set an appropriate comparator function.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      46291959
    • David Howells's avatar
      Provide a binary to hex conversion function · 53d91c5c
      David Howells authored
      Provide a function to convert a buffer of binary data into an unterminated
      ascii hex string representation of that data.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      53d91c5c
    • David Howells's avatar
      Merge tag 'keys-next-fixes-20140916' into keys-next · 1c9c115c
      David Howells authored
      Merge in keyrings fixes for next:
      
       (1) Insert some missing 'static' annotations.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      1c9c115c
    • David Howells's avatar
      Merge tag 'keys-fixes-20140916' into keys-next · 68c45c7f
      David Howells authored
      Merge in keyrings fixes, at least some of which later patches depend on:
      
       (1) Reinstate the production of EPERM for key types beginning with '.' in
           requests from userspace.
      
       (2) Tidy up the cleanup of PKCS#7 message signed information blocks and fix a
           bug this made more obvious.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.coM>
      68c45c7f
    • David Howells's avatar
      PKCS#7: Fix the parser cleanup to drain parsed out X.509 certs · cecf5d2e
      David Howells authored
      Fix the parser cleanup code to drain parsed out X.509 certs in the case that
      the decode fails and we jump to error_decode.
      
      The function is rearranged so that the same cleanup code is used in the success
      case as the error case - just that the message descriptor under construction is
      only released if it is still pointed to by the context struct at that point.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      cecf5d2e
    • David Howells's avatar
      PKCS#7: Provide a single place to do signed info block freeing · 3cd0920c
      David Howells authored
      The code to free a signed info block is repeated several times, so move the
      code to do it into a function of its own.  This gives us a place to add clean
      ups for stuff that gets added to pkcs7_signed_info.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Acked-by: default avatarVivek Goyal <vgoyal@redhat.com>
      3cd0920c