From patchwork Mon Nov 7 09:38:15 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Kozina X-Patchwork-Id: 9414571 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CCE1F6048F for ; Mon, 7 Nov 2016 09:40:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BD39C28BE4 for ; Mon, 7 Nov 2016 09:40:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B1F0128BE9; Mon, 7 Nov 2016 09:40:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 47EE728BE4 for ; Mon, 7 Nov 2016 09:40:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uA79dDT1027736; Mon, 7 Nov 2016 04:39:14 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uA79co7O031817 for ; Mon, 7 Nov 2016 04:38:50 -0500 Received: from dhcp131-147.brq.redhat.com (dhcp131-195.brq.redhat.com [10.34.131.195]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uA79chHj026259; Mon, 7 Nov 2016 04:38:48 -0500 From: Ondrej Kozina To: dm-devel@redhat.com Date: Mon, 7 Nov 2016 10:38:15 +0100 Message-Id: <1478511495-20306-4-git-send-email-okozina@redhat.com> In-Reply-To: <1478511495-20306-1-git-send-email-okozina@redhat.com> References: <1478511495-20306-1-git-send-email-okozina@redhat.com> In-Reply-To: <1470750966-13028-1-git-send-email-aryabinin@virtuozzo.com> References: <1470750966-13028-1-git-send-email-aryabinin@virtuozzo.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-loop: dm-devel@redhat.com Cc: Ondrej Kozina , aryabinin@virtuozzo.com, mpatocka@redhat.com, snitzer@redhat.com, mbroz@redhat.com Subject: [dm-devel] [PATCH 3/3] dm-crypt: modifications to previous patch X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Virus-Scanned: ClamAV using ClamSMTP 1) if we load kernel keyring key referenced by key description we should also return same key description via later status call. First the table should remain the same, second it's not reasonable to expose kernel key payload that could already have been be invalidated/expired by kernel keyring API. 2) search 'logon' key type instead of 'user' key type. 'logon' type key payloads can't be read from uspace. Not sure this is _the_ correct way either. Do we want dm-crypt to be able to load arbitrary key type? If so perhaps we should prefix a key description with key type? --- drivers/md/dm-crypt.c | 83 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 23 deletions(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 3b0f2a3..a038942 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -142,6 +142,7 @@ struct crypt_config { char *cipher; char *cipher_string; + char *key_description; struct crypt_iv_operations *iv_gen_ops; union { @@ -1495,13 +1496,20 @@ static int crypt_setkey_allcpus(struct crypt_config *cc) #ifdef CONFIG_KEYS static int crypt_set_keyring_key(struct crypt_config *cc, char *key_desc) { - int ret = 0; + char *new_key_desc; + int ret; struct key *key; const struct user_key_payload *ukp; - key = request_key(&key_type_user, key_desc, NULL); - if (IS_ERR(key)) + new_key_desc = kstrdup(key_desc, GFP_KERNEL); + if (!new_key_desc) + return -ENOMEM; + + key = request_key(&key_type_logon, key_desc, NULL); + if (IS_ERR(key)) { + kzfree(new_key_desc); return PTR_ERR(key); + } rcu_read_lock(); ret = key_validate(key); @@ -1514,9 +1522,30 @@ static int crypt_set_keyring_key(struct crypt_config *cc, char *key_desc) goto out; } memcpy(cc->key, ukp->data, cc->key_size); + + rcu_read_unlock(); + key_put(key); + + /* clear the flag since following operations may invalidate previously valid key */ + clear_bit(DM_CRYPT_KEY_VALID, &cc->flags); + + ret = crypt_setkey_allcpus(cc); + + /* wipe the kernel key payload in each case */ + memset(cc->key, 0, cc->key_size * sizeof(u8)); + + if (!ret) { + set_bit(DM_CRYPT_KEY_VALID, &cc->flags); + kzfree(cc->key_description); + cc->key_description = new_key_desc; + } else + kzfree(new_key_desc); + + return ret; out: rcu_read_unlock(); key_put(key); + kzfree(new_key_desc); return ret; } @@ -1528,17 +1557,14 @@ static int get_key_size(const char *key_desc) if (key_desc[0] != ':') return strlen(key_desc) >> 1; - key = request_key(&key_type_user, key_desc + 1, NULL); + key = request_key(&key_type_logon, key_desc + 1, NULL); if (IS_ERR(key)) return PTR_ERR(key); rcu_read_lock(); ret = key_validate(key); - if (ret < 0) - goto out; - - ret = user_key_payload(key)->datalen; -out: + if (!ret) + ret = user_key_payload(key)->datalen; rcu_read_unlock(); key_put(key); return ret; @@ -1566,25 +1592,30 @@ static int crypt_set_key(struct crypt_config *cc, char *key) /* ':' means that the key is in kernel keyring */ if (key[0] == ':') { - if (crypt_set_keyring_key(cc, key + 1)) + if (key_string_len < 2) goto out; + + r = crypt_set_keyring_key(cc, key + 1); } else { /* The key size may not be changed. */ if (cc->key_size != (key_string_len >> 1)) goto out; - } - /* clear the flag since following operations may invalidate previously valid key */ - clear_bit(DM_CRYPT_KEY_VALID, &cc->flags); + /* clear the flag since following operations may invalidate previously valid key */ + clear_bit(DM_CRYPT_KEY_VALID, &cc->flags); - if (key[0] != ':' && cc->key_size && - crypt_decode_key(cc->key, key, cc->key_size) < 0) - goto out; + /* wipe references to any kernel keyring key */ + kzfree(cc->key_description); + cc->key_description = NULL; - r = crypt_setkey_allcpus(cc); - if (!r) - set_bit(DM_CRYPT_KEY_VALID, &cc->flags); + if (cc->key_size && + crypt_decode_key(cc->key, key, cc->key_size) < 0) + goto out; + r = crypt_setkey_allcpus(cc); + if (!r) + set_bit(DM_CRYPT_KEY_VALID, &cc->flags); + } out: /* Hex key string not needed after here, so wipe it. */ memset(key, '0', key_string_len); @@ -1596,6 +1627,8 @@ static int crypt_wipe_key(struct crypt_config *cc) { clear_bit(DM_CRYPT_KEY_VALID, &cc->flags); memset(&cc->key, 0, cc->key_size * sizeof(u8)); + kzfree(cc->key_description); + cc->key_description = NULL; return crypt_setkey_allcpus(cc); } @@ -1633,6 +1666,7 @@ static void crypt_dtr(struct dm_target *ti) kzfree(cc->cipher); kzfree(cc->cipher_string); + kzfree(cc->key_description); /* Must zero key material before freeing */ kzfree(cc); @@ -2035,10 +2069,13 @@ static void crypt_status(struct dm_target *ti, status_type_t type, case STATUSTYPE_TABLE: DMEMIT("%s ", cc->cipher_string); - if (cc->key_size > 0) - for (i = 0; i < cc->key_size; i++) - DMEMIT("%02x", cc->key[i]); - else + if (cc->key_size > 0) { + if (cc->key_description) + DMEMIT(":%s", cc->key_description); + else + for (i = 0; i < cc->key_size; i++) + DMEMIT("%02x", cc->key[i]); + } else DMEMIT("-"); DMEMIT(" %llu %s %llu", (unsigned long long)cc->iv_offset,