From patchwork Wed Oct 11 19:12:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Garrett X-Patchwork-Id: 10000379 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 AC7AC602BF for ; Wed, 11 Oct 2017 19:12:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9399428B38 for ; Wed, 11 Oct 2017 19:12:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 885ED28B3B; Wed, 11 Oct 2017 19:12:41 +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=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B266128B36 for ; Wed, 11 Oct 2017 19:12:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753094AbdJKTMj (ORCPT ); Wed, 11 Oct 2017 15:12:39 -0400 Received: from mail-it0-f73.google.com ([209.85.214.73]:60941 "EHLO mail-it0-f73.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752451AbdJKTMf (ORCPT ); Wed, 11 Oct 2017 15:12:35 -0400 Received: by mail-it0-f73.google.com with SMTP id c3so2065902itc.19 for ; Wed, 11 Oct 2017 12:12:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:date:in-reply-to:message-id:references:subject:from:to :cc; bh=39eC2ukCfO7iMZjna567xO6acHXEtpK2WY9bFhfmh64=; b=tZrhWUpI3eieWBKG4VVS3yt59XfThQb71WdYC+PzF7iKUUnmEbviKjVQToZRdkWhwK BYfq8S0O+Q6nQf+FUaa98bUPN0JU97kFf1HRHmTuXtVHY0SawiV32MM/jeDI3XjglIZz nxoecBP+aaPGKUDGQNP1zMIlhJI1aGem+7mPRvflmClIki5ePr+diFruVW4lfQE9Nbj/ D5mqYWLbBBTWK2m8TU8cZE+SsbQY0nJrhAyJ+Lo+30ILKe8RgFRs5Vq8E2ORjOiNX9dJ sFXb2i57UYVflM6f0L1Qc8NxQhEvBBWFDJYkaBXY5mwcSX6r/Zekzfv7AvaqD8mcfKGi ogDQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:date:in-reply-to:message-id :references:subject:from:to:cc; bh=39eC2ukCfO7iMZjna567xO6acHXEtpK2WY9bFhfmh64=; b=b05K+ekVBPUihxxqD5kgy85cyBOQMY6HS1oufZSZ0JTgmsEncmwHOMQR7nU/gsV7gU w9eiBdAwqA3ABmaDrh6oas3EC6U+UTU96UelBSe8wpF0iOUnmIU06zA/Cdl0nOUWxW/G XSpYy0b7jvi3gJwzz74U92lQS/ulU3jfFH0J2t9I+vBw99btHSkPkt+l3wedEu062v46 KkCY6zCGILD18SI++GGjVjqXY3hJ3mU9V2ROXAz3F7SkXAk/ahpnT2FbYj3GBe24vu7U tt8mVcqfThQRHIl11S77tPGE62s77IEUgpkUg4AGdKa1LpQaDXKSc/+YUZvm46uMOOfI eEfw== X-Gm-Message-State: AMCzsaXc+wVjI9oBIqGvUXLLLUQCuZVcpufbRQf0xVP6mrnzCE0gaHkw Jno/7DC6gB9/VvottKGOFapVaiYfFLRxe72MjZlSe7eiIGijIeZwIlUvw/BBrp25+lmJmVMImF8 t2F1zIdB+VyLHsZMnh7ZmRehYEh/Hfsj9zlo= X-Google-Smtp-Source: AOwi7QAUyHtpYcxUyQVjpk5xjuJf4Wb0mvkDssR/zPAtI7rprAh8tloFR/krn5DNpzj1I2O6W4KpoEveWx71FhU8Jvb66A== MIME-Version: 1.0 X-Received: by 10.36.217.196 with SMTP id p187mr41085itg.28.1507749154981; Wed, 11 Oct 2017 12:12:34 -0700 (PDT) Date: Wed, 11 Oct 2017 12:12:18 -0700 In-Reply-To: <20171011191218.5274-1-mjg59@google.com> Message-Id: <20171011191218.5274-2-mjg59@google.com> References: <20171011191218.5274-1-mjg59@google.com> X-Mailer: git-send-email 2.15.0.rc0.271.g36b669edcc-goog Subject: [PATCH V3 2/2] Add a bprm_interpreted security hook and call into IMA from it From: Matthew Garrett To: linux-integrity@vger.kernel.org Cc: zohar@linux.vnet.ibm.com, james.l.morris@oracle.com, keescook@chromium.org, oleg@redhat.com, Matthew Garrett Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP IMA has support for validating files during execution using the bprm_check functionality. However, this is called before new credentials have been applied to the task. The previous patch in this series added an additional IMA check in the bprm_committed_creds hook - however, if a file is handled by binfmt_misc or binfmt_script (or, in an extremely unlikely corner case, binfmt_em86), only the interpreter will be appraised since bprm_committed_creds is never called for the initial executable. This patch adds an additional bprm_interpreted hook and calls it from the end of the relevant binfmt implementations. It is effectively identical to the bprm_committed_creds hook, except that bprm->file points to the initial file rather than to the interpreter. Signed-off-by: Matthew Garrett Cc: James Morris Cc: Kees Cook Cc: Oleg Nesterov --- fs/binfmt_em86.c | 31 ++++++++++++++++++++++--------- fs/binfmt_misc.c | 11 +++++++---- fs/binfmt_script.c | 45 +++++++++++++++++++++++++++++++-------------- include/linux/lsm_hooks.h | 7 +++++++ include/linux/security.h | 1 + security/security.c | 11 +++++++++++ 6 files changed, 79 insertions(+), 27 deletions(-) diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index dd2d3f0cd55d..e954ec123d27 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -26,7 +26,7 @@ static int load_em86(struct linux_binprm *bprm) { const char *i_name, *i_arg; char *interp; - struct file * file; + struct file *file, *old; int retval; struct elfhdr elf_ex; @@ -47,8 +47,7 @@ static int load_em86(struct linux_binprm *bprm) if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) return -ENOENT; - allow_write_access(bprm->file); - fput(bprm->file); + old = bprm->file; bprm->file = NULL; /* Unlike in the script case, we don't have to do any hairy @@ -72,11 +71,13 @@ static int load_em86(struct linux_binprm *bprm) bprm->argc++; if (i_arg) { retval = copy_strings_kernel(1, &i_arg, bprm); - if (retval < 0) return retval; + if (retval < 0) + goto ret; bprm->argc++; } retval = copy_strings_kernel(1, &i_name, bprm); - if (retval < 0) return retval; + if (retval < 0) + goto ret; bprm->argc++; /* @@ -85,16 +86,28 @@ static int load_em86(struct linux_binprm *bprm) * space, and we don't need to copy it. */ file = open_exec(interp); - if (IS_ERR(file)) - return PTR_ERR(file); + if (IS_ERR(file)) { + retval = PTR_ERR(file); + goto ret; + } bprm->file = file; retval = prepare_binprm(bprm); if (retval < 0) - return retval; + goto ret; - return search_binary_handler(bprm); + retval = search_binary_handler(bprm); + if (retval < 0) + goto ret; + + bprm->file = old; + retval = security_bprm_interpreted(bprm); + bprm->file = file; +ret: + allow_write_access(old); + fput(old); + return retval; } static struct linux_binfmt em86_format = { diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 2a46762def31..81e6e02348f9 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -130,7 +130,7 @@ static Node *check_file(struct linux_binprm *bprm) static int load_misc_binary(struct linux_binprm *bprm) { Node *fmt; - struct file *interp_file = NULL; + struct file *interp_file = NULL, *old = bprm->file; int retval; int fd_binary = -1; @@ -175,7 +175,6 @@ static int load_misc_binary(struct linux_binprm *bprm) regardless of the interpreter's permissions */ would_dump(bprm, bprm->file); - allow_write_access(bprm->file); bprm->file = NULL; /* mark the bprm that fd should be passed to interp */ @@ -183,8 +182,6 @@ static int load_misc_binary(struct linux_binprm *bprm) bprm->interp_data = fd_binary; } else { - allow_write_access(bprm->file); - fput(bprm->file); bprm->file = NULL; } /* make argv[1] be the path to the binary */ @@ -236,7 +233,13 @@ static int load_misc_binary(struct linux_binprm *bprm) if (retval < 0) goto error; + bprm->file = old; + retval = security_bprm_interpreted(bprm); + bprm->file = interp_file; ret: + allow_write_access(old); + if (fd_binary < 0) + fput(old); dput(fmt->dentry); return retval; error: diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 7cde3f46ad26..f2bd2aa15702 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -13,12 +13,13 @@ #include #include #include +#include static int load_script(struct linux_binprm *bprm) { const char *i_arg, *i_name; char *cp; - struct file *file; + struct file *file, *old; int retval; if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!')) @@ -38,8 +39,7 @@ static int load_script(struct linux_binprm *bprm) * Sorta complicated, but hopefully it will work. -TYT */ - allow_write_access(bprm->file); - fput(bprm->file); + old = bprm->file; bprm->file = NULL; bprm->buf[BINPRM_BUF_SIZE - 1] = '\0'; @@ -54,8 +54,11 @@ static int load_script(struct linux_binprm *bprm) break; } for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++); - if (*cp == '\0') - return -ENOEXEC; /* No interpreter name found */ + if (*cp == '\0') { + retval = -ENOEXEC; /* No interpreter name found */ + goto ret; + } + i_name = cp; i_arg = NULL; for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++) @@ -76,37 +79,51 @@ static int load_script(struct linux_binprm *bprm) */ retval = remove_arg_zero(bprm); if (retval) - return retval; + goto ret; retval = copy_strings_kernel(1, &bprm->interp, bprm); if (retval < 0) - return retval; + goto ret; bprm->argc++; if (i_arg) { retval = copy_strings_kernel(1, &i_arg, bprm); if (retval < 0) - return retval; + goto ret; bprm->argc++; } retval = copy_strings_kernel(1, &i_name, bprm); if (retval) - return retval; + goto ret; bprm->argc++; retval = bprm_change_interp(i_name, bprm); if (retval < 0) - return retval; + goto ret; /* * OK, now restart the process with the interpreter's dentry. */ file = open_exec(i_name); - if (IS_ERR(file)) - return PTR_ERR(file); + if (IS_ERR(file)) { + retval = PTR_ERR(file); + goto ret; + } bprm->file = file; retval = prepare_binprm(bprm); if (retval < 0) - return retval; - return search_binary_handler(bprm); + goto ret; + retval = search_binary_handler(bprm); + if (retval) + goto ret; + /* + * Allow for validation of the script as well as the interpreter + */ + bprm->file = old; + retval = security_bprm_interpreted(bprm); + bprm->file = file; +ret: + allow_write_access(old); + fput(old); + return retval; } static struct linux_binfmt script_format = { diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index c9258124e417..fd23b4098098 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -75,6 +75,11 @@ * linux_binprm structure. This hook is a good place to perform state * changes on the process such as clearing out non-inheritable signal * state. This is called immediately after commit_creds(). + * @bprm_interpreted: + * Validate the credentials of an executable that was interpreted via + * binfmt_misc or binfmt_script. This occurs after the new credentials + * have been committed to @current->cred, but @bprm->file points to the + * original file rather than the interpreter that was used to execute it. * * Security hooks for filesystem operations. * @@ -1383,6 +1388,7 @@ union security_list_options { int (*bprm_check_security)(struct linux_binprm *bprm); void (*bprm_committing_creds)(struct linux_binprm *bprm); void (*bprm_committed_creds)(struct linux_binprm *bprm); + int (*bprm_interpreted)(struct linux_binprm *bprm); int (*sb_alloc_security)(struct super_block *sb); void (*sb_free_security)(struct super_block *sb); @@ -1703,6 +1709,7 @@ struct security_hook_heads { struct list_head bprm_check_security; struct list_head bprm_committing_creds; struct list_head bprm_committed_creds; + struct list_head bprm_interpreted; struct list_head sb_alloc_security; struct list_head sb_free_security; struct list_head sb_copy_data; diff --git a/include/linux/security.h b/include/linux/security.h index ad970a4f19f6..e48c38379c64 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -233,6 +233,7 @@ int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); void security_bprm_committing_creds(struct linux_binprm *bprm); int security_bprm_committed_creds(struct linux_binprm *bprm); +int security_bprm_interpreted(struct linux_binprm *bprm); int security_sb_alloc(struct super_block *sb); void security_sb_free(struct super_block *sb); int security_sb_copy_data(char *orig, char *copy); diff --git a/security/security.c b/security/security.c index c2c5017e3477..6b9cb108ff61 100644 --- a/security/security.c +++ b/security/security.c @@ -352,6 +352,17 @@ int security_bprm_committed_creds(struct linux_binprm *bprm) return ima_creds_check(bprm); } +int security_bprm_interpreted(struct linux_binprm *bprm) +{ + int ret; + + ret = call_int_hook(bprm_interpreted, 0, bprm); + if (ret) + return ret; + + return ima_creds_check(bprm); +} + int security_sb_alloc(struct super_block *sb) { return call_int_hook(sb_alloc_security, 0, sb);