From patchwork Sat Mar 18 00:31:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Richey X-Patchwork-Id: 740567 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vlP3q0nQHz9ryj for ; Sat, 18 Mar 2017 11:59:03 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="HV3j/jXw"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751421AbdCRA7C (ORCPT ); Fri, 17 Mar 2017 20:59:02 -0400 Received: from mail-pg0-f50.google.com ([74.125.83.50]:32867 "EHLO mail-pg0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751101AbdCRA7B (ORCPT ); Fri, 17 Mar 2017 20:59:01 -0400 Received: by mail-pg0-f50.google.com with SMTP id n190so50712286pga.0 for ; Fri, 17 Mar 2017 17:59:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=PJXS8OuuqRYZ8GATqyEe+12A5cXAgKle22GcBqPK4Gk=; b=HV3j/jXwCayiS6PIkUdifz/GF4kWqotWHqWPesk3DQu9nwSYqCQfwO97sLPKE4WXi/ QEnzSzJVOfLN17ac+/J4Slw0rruBRaQYAVWn/XaFF6mCTCikIPR0mAavfA98KI8MLNwI 3BPzu2k9zGpYrxUxVveOOZQ8uy01+js4FoQumCTagLmlmEkkdwFFFamKjnWOviB19VP8 miPu6WKe6dof//cPvN3NMdtAD783Q7ZbT+JK5dPjOrHDx+lAklGM+71PkjIGGoUipBr/ pfr9Pu4qNzchHKRIVbgDczSh+yYuLPLmib9soaVSAsc3Kr5xzrqgem5JBtF9fWehbceW n0Hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=PJXS8OuuqRYZ8GATqyEe+12A5cXAgKle22GcBqPK4Gk=; b=q6CNUKtVZDvV0f4sWsNrhsonk7FM+pDScRs3W7LLS3E4yW9Jsv6Z5T4gOtwLDD2Zkr qRIENLYRjRZD0CHn9hnMndcZdAlYrvFvG3WdAAuzF/aXWu2/UblEOyCRmpOG70dmORcB qGyLMJAtGPUz2DDkfgZe38WFzG7VIm97H87/pjlelm3QOS8q48TCqajHQWiqTLHo+uLk oS7UQbriUQx48yGmaR1GoUxVdXx0FRINRYp+Z35WE5jOb6zmtzfzEqt8Vjc7uYr46fae 91drLW/eziHWrZsE7ZUsB5g1yeRYKUIyThmfazkKs+VZkxUgz3V52anSV/zE85scqjr2 Fj6A== X-Gm-Message-State: AFeK/H3D/gOAunBap6W7C6HRj2N1mq2lyvwI7oPfyDMfz8AnDBhpa5LBYBlyXojUa/h9/NcQ X-Received: by 10.84.217.215 with SMTP id d23mr23947058plj.33.1489797078592; Fri, 17 Mar 2017 17:31:18 -0700 (PDT) Received: from goog-fscrypt-dev.c.goog-dpa-dev.internal (56.114.199.104.bc.googleusercontent.com. [104.199.114.56]) by smtp.gmail.com with ESMTPSA id b83sm18935681pfe.12.2017.03.17.17.31.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 17 Mar 2017 17:31:17 -0700 (PDT) From: Joe Richey To: linux-ext4@vger.kernel.org Cc: Theodore Ts'o , Michael Halcrow , Joe Richey Subject: [PATCH] misc: fixed error handling in e4crypt Date: Sat, 18 Mar 2017 00:31:03 +0000 Message-Id: <1489797063-19169-1-git-send-email-joerichey@google.com> X-Mailer: git-send-email 2.7.4 Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org Due to some interesting behaviour in keyctl (as described in the comments), we use KEYCTL_GET_KEYRING_ID to translate the special value of KEY_SPEC_SESSION_KEYRING to a real keyring id. However, how we currently do this is flawed in two ways. First, if KEYCTL_GET_KEYRING_ID fails, we don't detect it as it returns -1 and zero is used for an error value in get_keyring_id. Second, if the user specifies "-k @s" the translation never runs and the undesireable behavior occurs. These are both fixed by doing the translation outside of get_keyring_id. Signed-off-by: Joe Richey --- misc/e4crypt.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/misc/e4crypt.c b/misc/e4crypt.c index 04faca3..fd2fc8f 100644 --- a/misc/e4crypt.c +++ b/misc/e4crypt.c @@ -507,24 +507,12 @@ static int get_keyring_id(const char *keyring) /* * If no keyring is specified, by default use either the user - * session key ring or the session keyring. Fetching the + * session keyring or the session keyring. Fetching the * session keyring will return the user session keyring if no * session keyring has been set. - * - * We need to do this instead of simply adding the key to - * KEY_SPEC_SESSION_KEYRING since trying to add a key to a - * session keyring that does not yet exist will cause the - * kernel to create a session keyring --- which wil then get - * garbage collected as soon as e4crypt exits. - * - * The fact that the keyctl system call and the add_key system - * call treats KEY_SPEC_SESSION_KEYRING differently when a - * session keyring does not exist is very unfortunate and - * confusing, but so it goes... */ if (keyring == NULL) - return keyctl(KEYCTL_GET_KEYRING_ID, - KEY_SPEC_SESSION_KEYRING, 0); + return KEY_SPEC_SESSION_KEYRING; for (x = 0; x < (sizeof(keyrings) / sizeof(keyrings[0])); ++x) { if (strcmp(keyring, keyrings[x].name) == 0) { return keyrings[x].code; @@ -585,6 +573,24 @@ static void insert_key_into_keyring(const char *keyring, struct salt *salt) key.mode = EXT4_ENCRYPTION_MODE_AES_256_XTS; memcpy(key.raw, salt->key, EXT4_MAX_KEY_SIZE); key.size = EXT4_MAX_KEY_SIZE; + + /* + * We need to do this instead of simply adding the key to + * KEY_SPEC_SESSION_KEYRING since trying to add a key to a + * session keyring that does not yet exist will cause the + * kernel to create a session keyring --- which wil then get + * garbage collected as soon as e4crypt exits. + * + * The fact that the keyctl system call and the add_key system + * call treats KEY_SPEC_SESSION_KEYRING differently when a + * session keyring does not exist is very unfortunate and + * confusing, but so it goes... + */ + if (keyring_id == KEY_SPEC_SESSION_KEYRING) { + keyring_id = keyctl(KEYCTL_GET_KEYRING_ID, keyring_id, 0); + if (keyring_id < 0) + printf("Could not get session keyring.\n"); + } rc = add_key(EXT2FS_KEY_TYPE_LOGON, key_ref_full, (void *)&key, sizeof(key), keyring_id); if (rc == -1) {