From patchwork Wed Oct 11 19:12:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Garrett X-Patchwork-Id: 10000377 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 E000B602BF for ; Wed, 11 Oct 2017 19:12:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C6B3528B36 for ; Wed, 11 Oct 2017 19:12:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BB6D628B38; Wed, 11 Oct 2017 19:12:39 +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.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM 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 CA53128B36 for ; Wed, 11 Oct 2017 19:12:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757514AbdJKTMh (ORCPT ); Wed, 11 Oct 2017 15:12:37 -0400 Received: from mail-oi0-f73.google.com ([209.85.218.73]:55794 "EHLO mail-oi0-f73.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757719AbdJKTMd (ORCPT ); Wed, 11 Oct 2017 15:12:33 -0400 Received: by mail-oi0-f73.google.com with SMTP id e123so1993204oig.14 for ; Wed, 11 Oct 2017 12:12:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:date:message-id:subject:from:to:cc; bh=i+Gs0XFcFW5BBQ5AA1I0WXmPXeDNEi/gKQRHjVotnvw=; b=gujbtYEuQ0npWh+pQyp8j+cJVOpH5UNo3nJGAuxepDji4BD5cHEWQEsLdY52ia2v+Q xx1RyHUv5qT78vY+PcmA4vymje9GuyuWD4sFKqSNQA312g4ZwINDYCS0qkQPdRw+gwrw QsWuuk/kNkxqWZHQQIC/IvcibqWdNujLeUIdhVmsPh7w2D35Dow3aL0PycVE5VirB6xm dJ7aDrR8Z1ajafCF+XMP+Bb4iHRpRRXQyKC6zUu4f7M57b3+5oGGswlZdHwkVgNRVQVN s5f2JFMUWdyPiWS/Kjot9F/qeRPlKiMdW/ZGIB9ZOdvaKwJKBcEunqUZunWbSyPDqfL5 qmDw== 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:message-id:subject:from:to:cc; bh=i+Gs0XFcFW5BBQ5AA1I0WXmPXeDNEi/gKQRHjVotnvw=; b=ipWDOIBmoUdWpvKfbtlVG8IoO94+0DsZjOO4BUY53TsdAdnZ69b8BO3wtkdWbduB7P 0lgHcWELMTCYwE9ixUEneVYAEnXL3lYiuy/v6n1+O4pN5VOIyKg+s2IJJfV9ziTZsQ9u 259qbWm1zLdWr1+AETp/tK9m5fjPx9wrTsP8qNhdd1l81rqkqqmduex6ughhlSo4CfQg vLkwWU+HKXWiQt+4ei0X3ZwLUoCqYxuLk6RbN8rALTR+xLFrO8ti0CzB9Sx0Rihtj3p/ 2uUiRZdvqWQ9VfXgqwbVV+zGUyHyFtslHOrFYAOjN9EUvzFQzlDPHUZp6TBiC1zVPYEb r0XQ== X-Gm-Message-State: AMCzsaVsL3rGqtDf/i+vzxnpue6w6N5v3dgLtjDvO72aNasa1TJk9KOt VdvwkSEqg7WTteBzU6y3+pvNgWkiGzlaz0bAQ7V7r7IygzUG8fRNQARqUrXxYOEC1SS1LQwlcli ivjdghO6cSLAkQgn9nO3hnRUx9GoyzIsZ5kE= X-Google-Smtp-Source: AOwi7QA+Sy2Ws56qVw1cg64y/a8vpCM8+XNnA9JbSF1xIp9skpUW7PNwRgO01Ptdqjbug+dU4INFKE2i3GiHTCZs0/nbBg== MIME-Version: 1.0 X-Received: by 10.157.73.27 with SMTP id e27mr2778733otf.121.1507749152675; Wed, 11 Oct 2017 12:12:32 -0700 (PDT) Date: Wed, 11 Oct 2017 12:12:17 -0700 Message-Id: <20171011191218.5274-1-mjg59@google.com> X-Mailer: git-send-email 2.15.0.rc0.271.g36b669edcc-goog Subject: [PATCH V3 1/2] IMA: Add support for IMA validation after credentials are committed 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 It may be desirable to perform appraisal after credentials are committed, for instance in the case where validation is only required if the binary has transitioned into a privileged security context. Add an additional call into IMA in the committed_credentials security hook and abort execution if it fails. This will only be called at install_exec_creds() time, which means it will only be run on binary interpreters rather than on the scripts or files handled by binfmt_misc. The following patch will rectify this. Signed-off-by: Matthew Garrett --- Documentation/ABI/testing/ima_policy | 2 +- arch/x86/ia32/ia32_aout.c | 4 +++- fs/binfmt_aout.c | 4 +++- fs/binfmt_elf.c | 5 ++++- fs/binfmt_elf_fdpic.c | 5 ++++- fs/binfmt_flat.c | 4 +++- fs/exec.c | 8 ++++++-- include/linux/binfmts.h | 2 +- include/linux/ima.h | 6 ++++++ include/linux/security.h | 5 +++-- security/integrity/iint.c | 1 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_appraise.c | 8 ++++++++ security/integrity/ima/ima_main.c | 24 +++++++++++++++++++++++- security/integrity/ima/ima_policy.c | 4 ++++ security/integrity/integrity.h | 9 +++++++-- security/security.c | 3 ++- 18 files changed, 81 insertions(+), 16 deletions(-) diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index e76432b9954d..5dc9eed035fb 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -25,7 +25,7 @@ Description: [obj_user=] [obj_role=] [obj_type=]] option: [[appraise_type=]] [permit_directio] - base: func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK] + base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c index 8e02b30cf08e..0dba3ed8459f 100644 --- a/arch/x86/ia32/ia32_aout.c +++ b/arch/x86/ia32/ia32_aout.c @@ -312,7 +312,9 @@ static int load_aout_binary(struct linux_binprm *bprm) if (retval < 0) return retval; - install_exec_creds(bprm); + retval = install_exec_creds(bprm); + if (retval) + return retval; if (N_MAGIC(ex) == OMAGIC) { unsigned long text_addr, map_size; diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c index ce1824f47ba6..02c7c4320fc8 100644 --- a/fs/binfmt_aout.c +++ b/fs/binfmt_aout.c @@ -256,7 +256,9 @@ static int load_aout_binary(struct linux_binprm * bprm) if (retval < 0) return retval; - install_exec_creds(bprm); + retval = install_exec_creds(bprm); + if (retval) + return retval; if (N_MAGIC(ex) == OMAGIC) { unsigned long text_addr, map_size; diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 73b01e474fdc..a6883a855a14 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -866,7 +866,10 @@ static int load_elf_binary(struct linux_binprm *bprm) current->flags |= PF_RANDOMIZE; setup_new_exec(bprm); - install_exec_creds(bprm); + + retval = install_exec_creds(bprm); + if (retval) + goto out_free_dentry; /* Do this so that we can load the interpreter, if need be. We will change some of these later */ diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index e70c039ac190..d89830095e19 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -433,7 +433,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm) current->mm->start_stack = current->mm->start_brk + stack_size; #endif - install_exec_creds(bprm); + retval = install_exec_creds(bprm); + if (retval) + goto error; + if (create_elf_fdpic_tables(bprm, current->mm, &exec_params, &interp_params) < 0) goto error; diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 475d083f8088..5f59c53ba97f 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -948,7 +948,9 @@ static int load_flat_binary(struct linux_binprm *bprm) } } - install_exec_creds(bprm); + retval = install_exec_creds(bprm); + if (retval) + return retval; set_binfmt(&flat_format); diff --git a/fs/exec.c b/fs/exec.c index 5470d3c1892a..c7e9f84abd36 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1425,8 +1425,10 @@ EXPORT_SYMBOL(bprm_change_interp); /* * install the new credentials for this executable */ -void install_exec_creds(struct linux_binprm *bprm) +int install_exec_creds(struct linux_binprm *bprm) { + int ret = 0; + security_bprm_committing_creds(bprm); commit_creds(bprm->cred); @@ -1445,8 +1447,10 @@ void install_exec_creds(struct linux_binprm *bprm) * ptrace_attach() from altering our determination of the task's * credentials; any time after this it may be unlocked. */ - security_bprm_committed_creds(bprm); + ret = security_bprm_committed_creds(bprm); mutex_unlock(¤t->signal->cred_guard_mutex); + + return ret; } EXPORT_SYMBOL(install_exec_creds); diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 18d05b5491f3..725d5582589f 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -135,7 +135,7 @@ extern int bprm_change_interp(const char *interp, struct linux_binprm *bprm); extern int copy_strings_kernel(int argc, const char *const *argv, struct linux_binprm *bprm); extern int prepare_bprm_creds(struct linux_binprm *bprm); -extern void install_exec_creds(struct linux_binprm *bprm); +extern int install_exec_creds(struct linux_binprm *bprm); extern void set_binfmt(struct linux_binfmt *new); extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t); diff --git a/include/linux/ima.h b/include/linux/ima.h index 0e4647e0eb60..f9a64f94b0d3 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -16,6 +16,7 @@ struct linux_binprm; #ifdef CONFIG_IMA extern int ima_bprm_check(struct linux_binprm *bprm); +extern int ima_creds_check(struct linux_binprm *bprm); extern int ima_file_check(struct file *file, int mask, int opened); extern void ima_file_free(struct file *file); extern int ima_file_mmap(struct file *file, unsigned long prot); @@ -34,6 +35,11 @@ static inline int ima_bprm_check(struct linux_binprm *bprm) return 0; } +static inline int ima_creds_check(struct linux_binprm *bprm) +{ + return 0; +} + static inline int ima_file_check(struct file *file, int mask, int opened) { return 0; diff --git a/include/linux/security.h b/include/linux/security.h index ce6265960d6c..ad970a4f19f6 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -232,7 +232,7 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); 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); -void security_bprm_committed_creds(struct linux_binprm *bprm); +int security_bprm_committed_creds(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); @@ -536,8 +536,9 @@ static inline void security_bprm_committing_creds(struct linux_binprm *bprm) { } -static inline void security_bprm_committed_creds(struct linux_binprm *bprm) +static inline int security_bprm_committed_creds(struct linux_binprm *bprm) { + return 0; } static inline int security_sb_alloc(struct super_block *sb) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 6fc888ca468e..ad30094a58b4 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint) iint->ima_mmap_status = INTEGRITY_UNKNOWN; iint->ima_bprm_status = INTEGRITY_UNKNOWN; iint->ima_read_status = INTEGRITY_UNKNOWN; + iint->ima_creds_status = INTEGRITY_UNKNOWN; iint->evm_status = INTEGRITY_UNKNOWN; iint->measured_pcrs = 0; kmem_cache_free(iint_cache, iint); diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d52b487ad259..547ea832bb1b 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest) hook(FILE_CHECK) \ hook(MMAP_CHECK) \ hook(BPRM_CHECK) \ + hook(CREDS_CHECK) \ hook(POST_SETATTR) \ hook(MODULE_CHECK) \ hook(FIRMWARE_CHECK) \ diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c2edba8de35e..0c19bb423570 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -165,7 +165,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, * The policy is defined in terms of keypairs: * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK + * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK * mask: contains the permission mask * fsmagic: hex value * diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 809ba70fbbbf..edb82e722a0d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -86,6 +86,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, return iint->ima_mmap_status; case BPRM_CHECK: return iint->ima_bprm_status; + case CREDS_CHECK: + return iint->ima_creds_status; case FILE_CHECK: case POST_SETATTR: return iint->ima_file_status; @@ -106,6 +108,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint, case BPRM_CHECK: iint->ima_bprm_status = status; break; + case CREDS_CHECK: + iint->ima_creds_status = status; + break; case FILE_CHECK: case POST_SETATTR: iint->ima_file_status = status; @@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, case BPRM_CHECK: iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED); break; + case CREDS_CHECK: + iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED); + break; case FILE_CHECK: case POST_SETATTR: iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED); diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 2aebb7984437..5be8307a1dd1 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -14,7 +14,7 @@ * * File: ima_main.c * implements the IMA hooks: ima_bprm_check, ima_file_mmap, - * and ima_file_check. + * ima_creds_check and ima_file_check. */ #include #include @@ -306,6 +306,28 @@ int ima_bprm_check(struct linux_binprm *bprm) BPRM_CHECK, 0); } +/** + * ima_creds_check - based on policy, collect/store measurement. + * @bprm: contains the linux_binprm structure + * + * The OS protects against an executable file, already open for write, + * from being executed in deny_write_access() and an executable file, + * already open for execute, from being modified in get_write_access(). + * So we can be certain that what we verify and measure here is actually + * what is being executed. + * + * This is identical to ima_bprm_check, except called after child credentials + * have been committed. + * + * On success return 0. On integrity appraisal error, assuming the file + * is in policy and IMA-appraisal is in enforcing mode, return -EACCES. + */ +int ima_creds_check(struct linux_binprm *bprm) +{ + return process_measurement(bprm->file, NULL, 0, MAY_EXEC, + CREDS_CHECK, 0); +} + /** * ima_path_check - based on policy, collect/store measurement. * @file: pointer to the file to be measured diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 95209a5f8595..a6e14c532627 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -339,6 +339,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) return IMA_MMAP_APPRAISE; case BPRM_CHECK: return IMA_BPRM_APPRAISE; + case CREDS_CHECK: + return IMA_CREDS_APPRAISE; case FILE_CHECK: case POST_SETATTR: return IMA_FILE_APPRAISE; @@ -691,6 +693,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = MMAP_CHECK; else if (strcmp(args[0].from, "BPRM_CHECK") == 0) entry->func = BPRM_CHECK; + else if (strcmp(args[0].from, "CREDS_CHECK") == 0) + entry->func = CREDS_CHECK; else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") == 0) entry->func = KEXEC_KERNEL_CHECK; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 0a721c110e92..5413eb25b440 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -48,10 +48,14 @@ #define IMA_BPRM_APPRAISED 0x00002000 #define IMA_READ_APPRAISE 0x00004000 #define IMA_READ_APPRAISED 0x00008000 +#define IMA_CREDS_APPRAISE 0x00010000 +#define IMA_CREDS_APPRAISED 0x00020000 #define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \ - IMA_BPRM_APPRAISE | IMA_READ_APPRAISE) + IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \ + IMA_CREDS_APPRAISE) #define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \ - IMA_BPRM_APPRAISED | IMA_READ_APPRAISED) + IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \ + IMA_CREDS_APPRAISED) enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01, @@ -109,6 +113,7 @@ struct integrity_iint_cache { enum integrity_status ima_mmap_status:4; enum integrity_status ima_bprm_status:4; enum integrity_status ima_read_status:4; + enum integrity_status ima_creds_status:4; enum integrity_status evm_status:4; struct ima_digest_data *ima_hash; }; diff --git a/security/security.c b/security/security.c index 4bf0f571b4ef..c2c5017e3477 100644 --- a/security/security.c +++ b/security/security.c @@ -346,9 +346,10 @@ void security_bprm_committing_creds(struct linux_binprm *bprm) call_void_hook(bprm_committing_creds, bprm); } -void security_bprm_committed_creds(struct linux_binprm *bprm) +int security_bprm_committed_creds(struct linux_binprm *bprm) { call_void_hook(bprm_committed_creds, bprm); + return ima_creds_check(bprm); } int security_sb_alloc(struct super_block *sb)