From patchwork Thu Dec 1 17:20:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ondrej Kozina X-Patchwork-Id: 9456621 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 3CD5160585 for ; Thu, 1 Dec 2016 17:22:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2A6612851E for ; Thu, 1 Dec 2016 17:22:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1EFDD28527; Thu, 1 Dec 2016 17:22:44 +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 B11162851E for ; Thu, 1 Dec 2016 17:22:43 +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 uB1HLH5I019808; Thu, 1 Dec 2016 12:21:18 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id uB1HLGv0031899 for ; Thu, 1 Dec 2016 12:21:16 -0500 Received: from dhcp131-147.brq.redhat.com (dhcp131-195.brq.redhat.com [10.34.131.195]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id uB1HLEI5003116; Thu, 1 Dec 2016 12:21:15 -0500 From: Ondrej Kozina To: dm-devel@redhat.com Date: Thu, 1 Dec 2016 18:20:52 +0100 Message-Id: <1480612852-19878-1-git-send-email-okozina@redhat.com> In-Reply-To: References: X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-loop: dm-devel@redhat.com Cc: snitzer@redhat.com, mpatocka@redhat.com, Ondrej Kozina , aryabinin@virtuozzo.com, agk@redhat.com, mbroz@redhat.com Subject: [dm-devel] [PATCH] dm-crypt: reject key strings containing whitespace chars 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 Well, I was wrong. Unfortunately key_string may theoreticaly contain whitespace even after it's processed by dm_split_args(). The dm core driver supports escaping of almost all chars including any whitespace. if uspace passes key in the kernel in format ":32:logon:my_prefix:my\ key" the dm-crypt will look up key "my_prefix:my key" in kernel keyring service. So far everything's fine. Unfortunately if uspace later calls DM_TABLE_STATUS ioctl, it will not receive back expected ":32:logon:my_prefix:my\ key" but the unescaped version instead. Also uspace (most notably cryptsetup) is not ready to parse single target argument containing (even escaped) whitespace chars and any whitespace is simply taken as delimiter of another argument. This efect is mitigated by the fact libdevmapper curently performs double escaping of '\' char. Any user input in format "x\ x" is transformed into "x\\ x" before being passed to the kernel. Nonetheless dm-crypt may be used without go-between libdevmapper. Therefore I propose following patch to reject any key string containing whitespace. --- drivers/md/dm-crypt.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 78ab5e8..11df7a9 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -1489,6 +1490,14 @@ static int crypt_setkey(struct crypt_config *cc) #ifdef CONFIG_KEYS +static int contains_whitespace(const char *str) +{ + while (*str) + if (isspace(*str++)) + return 1; + return 0; +} + static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string) { char *new_key_string, *key_desc; @@ -1496,6 +1505,15 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string struct key *key; const struct user_key_payload *ukp; + /* + * Reject key_string with whitespace. dm core currently lacks code for + * proper whitespace escaping in arguments on DM_TABLE_STATUS path. + */ + if (contains_whitespace(key_string)) { + DMERR("whitespace chars currently not allowed in key string"); + return -EINVAL; + } + /* look for next ':' separating key_type from key_description */ key_desc = strpbrk(key_string, ":"); if (!key_desc || key_desc == key_string || !strlen(key_desc + 1))