This list is closed, nobody may subscribe to it.
2007 |
Jan
|
Feb
(1) |
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
(1) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2009 |
Jan
|
Feb
|
Mar
|
Apr
(1) |
May
(1) |
Jun
(2) |
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
2011 |
Jan
|
Feb
|
Mar
(1) |
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
2013 |
Jan
|
Feb
|
Mar
(7) |
Apr
|
May
(7) |
Jun
(7) |
Jul
(26) |
Aug
|
Sep
(7) |
Oct
(1) |
Nov
(35) |
Dec
(18) |
2014 |
Jan
(1) |
Feb
(2) |
Mar
(3) |
Apr
|
May
(16) |
Jun
(35) |
Jul
(103) |
Aug
(45) |
Sep
(226) |
Oct
(200) |
Nov
(66) |
Dec
(42) |
2015 |
Jan
(47) |
Feb
(3) |
Mar
(6) |
Apr
(14) |
May
(38) |
Jun
(10) |
Jul
(10) |
Aug
(15) |
Sep
(23) |
Oct
(78) |
Nov
(56) |
Dec
(70) |
2016 |
Jan
(9) |
Feb
(8) |
Mar
(15) |
Apr
(18) |
May
(78) |
Jun
(39) |
Jul
(3) |
Aug
(136) |
Sep
(134) |
Oct
(19) |
Nov
(48) |
Dec
(30) |
2017 |
Jan
(33) |
Feb
(35) |
Mar
(100) |
Apr
(87) |
May
(169) |
Jun
(119) |
Jul
(165) |
Aug
(241) |
Sep
(128) |
Oct
(42) |
Nov
|
Dec
|
From: Boshi W. <wan...@hu...> - 2017-10-11 07:47:03
|
The hash_setup function always sets hash_setup_done variable. If an invalid hash algorithm is passed, the default hash algorithm specified by CONFIG_IMA_DEFAULT_HASH could not be used. Signed-off-by: Wang Boshi <wan...@hu...> --- diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 2aebb79..ab70a39 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -51,6 +51,8 @@ static int __init hash_setup(char *str) ima_hash_algo = HASH_ALGO_SHA1; else if (strncmp(str, "md5", 3) == 0) ima_hash_algo = HASH_ALGO_MD5; + else + return 1; goto out; } @@ -60,6 +62,8 @@ static int __init hash_setup(char *str) break; } } + if (i == HASH_ALGO__LAST) + return 1; out: hash_setup_done = 1; return 1; |
From: Herbert Xu <he...@go...> - 2017-10-11 06:29:10
|
On Sat, Oct 07, 2017 at 10:51:42AM +0300, Gilad Ben-Yossef wrote: > On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu <he...@go...> wrote: > > On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote: > >> > >> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c > >> index 5e92bd2..3b3c154 100644 > >> --- a/crypto/algif_hash.c > >> +++ b/crypto/algif_hash.c > >> @@ -39,6 +39,20 @@ struct algif_hash_tfm { > >> bool has_key; > >> }; > >> > >> +/* Previous versions of crypto_* ops used to return -EBUSY > >> + * rather than -EAGAIN to indicate being tied up. The in > >> + * kernel API changed but we don't want to break the user > >> + * space API. As only the hash user interface exposed this > >> + * error ever to the user, do the translation here. > >> + */ > >> +static inline int crypto_user_err(int err) > >> +{ > >> + if (err == -EAGAIN) > >> + return -EBUSY; > >> + > >> + return err; > > > > I don't see the need to carry along this baggage. Does anyone > > in user-space actually rely on EBUSY? > > > I am not aware of anyone who does. I was just trying to avoid > changing the user ABI. > > Shall I roll a new revision without this patch? Yes please. I'd rather not carry this around for eternity unless it was actually required. Thanks, -- Email: Herbert Xu <he...@go...> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt |
From: Matthew G. <mj...@go...> - 2017-10-10 22:24:54
|
On Tue, Oct 10, 2017 at 3:21 PM, Mimi Zohar <zo...@li...> wrote: > I was hoping we could replace the existing bprm_check with the new > creds_check, but not all of the binfmt's registered are covered. Only > those that call install_exec_creds() are covered. This should > probably be reflected in the ima_creds_check() description. > Otherwise, the patch looks good. The semantics are different - bprm_check will check sub_user and co against the pre-exec() credentials, creds_check against the post-exec() credentials. That feels like something that could break existing policies, so I think we need to keep them independent. I'll rewrite the description and resend. |
From: Mimi Z. <zo...@li...> - 2017-10-10 22:22:08
|
On Fri, 2017-09-08 at 10:43 -0700, Matthew Garrett wrote: > 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. > > Signed-off-by: Matthew Garrett <mj...@go...> I was hoping we could replace the existing bprm_check with the new creds_check, but not all of the binfmt's registered are covered. Only those that call install_exec_creds() are covered. This should probably be reflected in the ima_creds_check() description. Otherwise, the patch looks good. Mimi > --- > 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 8d0879f1d42c..5eaedc31661b 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 9be82c4e14a4..5eb778710a46 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 6466153f2bf0..0f0463e8bcb8 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -865,7 +865,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 cf93a4fad012..066f81d31d7b 100644 > --- a/fs/binfmt_elf_fdpic.c > +++ b/fs/binfmt_elf_fdpic.c > @@ -432,7 +432,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 a1e6860b6f46..61cc1099d8a6 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -958,7 +958,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 62175cbcc801..8923f0ce5d57 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1418,8 +1418,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); > @@ -1438,8 +1440,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 3ae9013eeaaa..4d60d2c432d9 100644 > --- a/include/linux/binfmts.h > +++ b/include/linux/binfmts.h > @@ -121,7 +121,7 @@ extern int bprm_change_interp(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 b6ea1dc9cc9d..21763448dc89 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -231,7 +231,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_bprm_secureexec(struct linux_binprm *bprm); > int security_sb_alloc(struct super_block *sb); > void security_sb_free(struct super_block *sb); > @@ -537,8 +537,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_bprm_secureexec(struct linux_binprm *bprm) > 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 <linux/module.h> > #include <linux/file.h> > @@ -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 a53e7e4ab06c..45ba0e4501d6 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, > @@ -108,6 +112,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 30132378d103..bdb5cd5c8859 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_bprm_secureexec(struct linux_binprm *bprm) |
From: Matthew G. <mj...@go...> - 2017-10-09 17:45:54
|
On Fri, Sep 8, 2017 at 10:43 AM, Matthew Garrett <mj...@go...> wrote: > 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. Hi, Any feedback on this? |
From: er_tou <er...@16...> - 2017-10-09 09:23:24
|
hello, I encountered a problem, execute the following command to show the error. $ su -c 'mkdir -p /etc/keys' # To create and save the kernel master key (user type): $ su -c 'modprobe trusted encrypted' $ su -c 'keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u' $ su -c 'keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob' show "keyctl_read_alloc: Permission denied". Problem location is 'keyctl pipe'. [root@localhost ima_key_sign]# cat /proc/keys 029fddf9 I------ 1 perm 1f030000 0 0 asymmetri CentOS Linux kernel signing key: d48863a7c16fcc274123e6298f74f057af19fc54: X509.RSA af19fc54 [] 034d0e68 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid.1000: empty 0a1ab8e3 I--Q--- 1 perm 3f010000 0 0 user kmk-user: 10 This keyring “kmk-user” don't have read permission. Anyone have the question ? I hope get help? Thank, Thank, Thank ---------------------------------------------------------------------------------------- but, below command can execute. $ mkdir -p /etc/keys # To create and save the kernel master key (user type): $ modprobe trusted encrypted $ keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u $ keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob Thanks,Looking forward to your reply! -------------- next part -------------- An HTML attachment was scrubbed... |
From: Thomas M. <th...@m3...> - 2017-10-07 14:24:16
|
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer <th...@m3...> --- diff -u -p a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -32,7 +32,7 @@ bool ima_canonical_fmt; static int __init default_canonical_fmt_setup(char *str) { #ifdef __BIG_ENDIAN - ima_canonical_fmt = 1; + ima_canonical_fmt = true; #endif return 1; } diff -u -p a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -196,9 +196,9 @@ static int __init policy_setup(char *str if ((strcmp(p, "tcb") == 0) && !ima_policy) ima_policy = DEFAULT_TCB; else if (strcmp(p, "appraise_tcb") == 0) - ima_use_appraise_tcb = 1; + ima_use_appraise_tcb = true; else if (strcmp(p, "secure_boot") == 0) - ima_use_secure_boot = 1; + ima_use_secure_boot = true; } return 1; @@ -207,7 +207,7 @@ __setup("ima_policy=", policy_setup); static int __init default_appraise_policy_setup(char *str) { - ima_use_appraise_tcb = 1; + ima_use_appraise_tcb = true; return 1; } __setup("ima_appraise_tcb", default_appraise_policy_setup); |
From: Gilad Ben-Y. <gi...@be...> - 2017-10-07 07:51:50
|
On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu <he...@go...> wrote: > On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote: >> >> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c >> index 5e92bd2..3b3c154 100644 >> --- a/crypto/algif_hash.c >> +++ b/crypto/algif_hash.c >> @@ -39,6 +39,20 @@ struct algif_hash_tfm { >> bool has_key; >> }; >> >> +/* Previous versions of crypto_* ops used to return -EBUSY >> + * rather than -EAGAIN to indicate being tied up. The in >> + * kernel API changed but we don't want to break the user >> + * space API. As only the hash user interface exposed this >> + * error ever to the user, do the translation here. >> + */ >> +static inline int crypto_user_err(int err) >> +{ >> + if (err == -EAGAIN) >> + return -EBUSY; >> + >> + return err; > > I don't see the need to carry along this baggage. Does anyone > in user-space actually rely on EBUSY? I am not aware of anyone who does. I was just trying to avoid changing the user ABI. Shall I roll a new revision without this patch? Thanks, Gilad > > Cheers, > -- > Email: Herbert Xu <he...@go...> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru |
From: Herbert Xu <he...@go...> - 2017-10-07 03:25:00
|
On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote: > > diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c > index 5e92bd2..3b3c154 100644 > --- a/crypto/algif_hash.c > +++ b/crypto/algif_hash.c > @@ -39,6 +39,20 @@ struct algif_hash_tfm { > bool has_key; > }; > > +/* Previous versions of crypto_* ops used to return -EBUSY > + * rather than -EAGAIN to indicate being tied up. The in > + * kernel API changed but we don't want to break the user > + * space API. As only the hash user interface exposed this > + * error ever to the user, do the translation here. > + */ > +static inline int crypto_user_err(int err) > +{ > + if (err == -EAGAIN) > + return -EBUSY; > + > + return err; I don't see the need to carry along this baggage. Does anyone in user-space actually rely on EBUSY? Cheers, -- Email: Herbert Xu <he...@go...> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt |
From: Greg Kroah-H. <gr...@li...> - 2017-10-06 09:26:46
|
3.18-stable review patch. If anyone has any objections, please let me know. ------------------ From: Andrey Ryabinin <ary...@vi...> commit f5527fffff3f002b0a6b376163613b82f69de073 upstream. This fixes CVE-2016-8650. If mpi_powm() is given a zero exponent, it wants to immediately return either 1 or 0, depending on the modulus. However, if the result was initalised with zero limb space, no limbs space is allocated and a NULL-pointer exception ensues. Fix this by allocating a minimal amount of limb space for the result when the 0-exponent case when the result is 1 and not touching the limb space when the result is 0. This affects the use of RSA keys and X.509 certificates that carry them. BUG: unable to handle kernel NULL pointer dereference at (null) IP: [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6 PGD 0 Oops: 0002 [#1] SMP Modules linked in: CPU: 3 PID: 3014 Comm: keyctl Not tainted 4.9.0-rc6-fscache+ #278 Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014 task: ffff8804011944c0 task.stack: ffff880401294000 RIP: 0010:[<ffffffff8138ce5d>] [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6 RSP: 0018:ffff880401297ad8 EFLAGS: 00010212 RAX: 0000000000000000 RBX: ffff88040868bec0 RCX: ffff88040868bba0 RDX: ffff88040868b260 RSI: ffff88040868bec0 RDI: ffff88040868bee0 RBP: ffff880401297ba8 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000047 R11: ffffffff8183b210 R12: 0000000000000000 R13: ffff8804087c7600 R14: 000000000000001f R15: ffff880401297c50 FS: 00007f7a7918c700(0000) GS:ffff88041fb80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000401250000 CR4: 00000000001406e0 Stack: ffff88040868bec0 0000000000000020 ffff880401297b00 ffffffff81376cd4 0000000000000100 ffff880401297b10 ffffffff81376d12 ffff880401297b30 ffffffff81376f37 0000000000000100 0000000000000000 ffff880401297ba8 Call Trace: [<ffffffff81376cd4>] ? __sg_page_iter_next+0x43/0x66 [<ffffffff81376d12>] ? sg_miter_get_next_page+0x1b/0x5d [<ffffffff81376f37>] ? sg_miter_next+0x17/0xbd [<ffffffff8138ba3a>] ? mpi_read_raw_from_sgl+0xf2/0x146 [<ffffffff8132a95c>] rsa_verify+0x9d/0xee [<ffffffff8132acca>] ? pkcs1pad_sg_set_buf+0x2e/0xbb [<ffffffff8132af40>] pkcs1pad_verify+0xc0/0xe1 [<ffffffff8133cb5e>] public_key_verify_signature+0x1b0/0x228 [<ffffffff8133d974>] x509_check_for_self_signed+0xa1/0xc4 [<ffffffff8133cdde>] x509_cert_parse+0x167/0x1a1 [<ffffffff8133d609>] x509_key_preparse+0x21/0x1a1 [<ffffffff8133c3d7>] asymmetric_key_preparse+0x34/0x61 [<ffffffff812fc9f3>] key_create_or_update+0x145/0x399 [<ffffffff812fe227>] SyS_add_key+0x154/0x19e [<ffffffff81001c2b>] do_syscall_64+0x80/0x191 [<ffffffff816825e4>] entry_SYSCALL64_slow_path+0x25/0x25 Code: 56 41 55 41 54 53 48 81 ec a8 00 00 00 44 8b 71 04 8b 42 04 4c 8b 67 18 45 85 f6 89 45 80 0f 84 b4 06 00 00 85 c0 75 2f 41 ff ce <49> c7 04 24 01 00 00 00 b0 01 75 0b 48 8b 41 18 48 83 38 01 0f RIP [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6 RSP <ffff880401297ad8> CR2: 0000000000000000 ---[ end trace d82015255d4a5d8d ]--- Basically, this is a backport of a libgcrypt patch: http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=patch;h=6e1adb05d290aeeb1c230c763970695f4a538526 Fixes: cdec9cb5167a ("crypto: GnuPG based MPI lib - source files (part 1)") Signed-off-by: Andrey Ryabinin <ary...@vi...> Signed-off-by: David Howells <dho...@re...> cc: Dmitry Kasatkin <dmi...@gm...> cc: lin...@li... Signed-off-by: James Morris <jam...@or...> Signed-off-by: Greg Kroah-Hartman <gr...@li...> --- lib/mpi/mpi-pow.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) --- a/lib/mpi/mpi-pow.c +++ b/lib/mpi/mpi-pow.c @@ -64,8 +64,13 @@ int mpi_powm(MPI res, MPI base, MPI exp, if (!esize) { /* Exponent is zero, result is 1 mod MOD, i.e., 1 or 0 * depending on if MOD equals 1. */ - rp[0] = 1; res->nlimbs = (msize == 1 && mod->d[0] == 1) ? 0 : 1; + if (res->nlimbs) { + if (mpi_resize(res, 1) < 0) + goto enomem; + rp = res->d; + rp[0] = 1; + } res->sign = 0; goto leave; } |
From: Jarkko S. <jar...@li...> - 2017-10-04 11:22:41
|
On Mon, Sep 25, 2017 at 01:19:50PM +0200, Roberto Sassu wrote: > In preparation for the patch introducing a function to pass supported TPM > algorithms and digest sizes to TPM users, the crypto algorithm identifier > is added to the active_bank_info structure. > > All members of active_bank_info are necessary: TPM algorithm identifiers > will be used to create an event log (they are included in the Crypto Agile > format defined by TCG); crypto identifiers will be used to calculate > digests using the crypto subsystem; digest sizes will be used to truncate > digests calculated with different algorithms. > > Signed-off-by: Roberto Sassu <rob...@hu...> This should be in the patch set where it makes sense. Please remove it from the next version. /Jarkko > --- > drivers/char/tpm/tpm2-cmd.c | 2 ++ > include/linux/tpm.h | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index b1356be..8c58f6e 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -946,6 +946,7 @@ static int tpm2_init_active_bank_info(struct tpm_chip *chip, u16 alg_id, > int rc, i; > > active_bank->alg_id = alg_id; > + active_bank->crypto_id = HASH_ALGO__LAST; > > for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id; > @@ -954,6 +955,7 @@ static int tpm2_init_active_bank_info(struct tpm_chip *chip, u16 alg_id, > continue; > > active_bank->digest_size = hash_digest_size[crypto_algo]; > + active_bank->crypto_id = crypto_algo; > return 0; > } > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 3ecce21..fc927f3 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -54,6 +54,7 @@ struct tpm_class_ops { > > struct active_bank_info { > u16 alg_id; > + u16 crypto_id; > u16 digest_size; > }; > > -- > 2.9.3 > |
From: Carol W. <in...@te...> - 2017-10-04 11:12:36
|
An HTML attachment was scrubbed... |
From: Jarkko S. <jar...@li...> - 2017-10-04 11:12:08
|
On Mon, Sep 25, 2017 at 01:19:49PM +0200, Roberto Sassu wrote: > PCRs can be extended by providing the TPM algorithm identifier and > the digest. To correctly build the command buffer, the digest size > must be known. Remove the first paragraph. It does not any bring light on what the commit does and/or why the code change is made. In short, by reading this paragraph I did not learn anything about the commit. > The TPM driver cannot determine the digest size if the provided > TPM algorithm is not mapped to any crypto algorithm. In this case, > the PCR bank is not extended and could be used by attackers to protect > measurements made by themselves, which do not reflect the true status > of the platform. You are talking about "mapping" without any context. There is a static mapping inside the driver from crypto IDs to TPM algorithm IDs inside the driver implementation. You should just say it. Writing commit messages is very easy. Just write what you are doing and why you are doing it :-) Do not write anything else. > To avoid this situation, the digest size of unknown algorithms is > determined at TPM initialization time with a PCR read, and stored > in the tpm_chip structure. The array of algorithms (active_banks) > has been replaced with an array of active_pcr_bank_info, a new structure > containing both the TPM algorithm identifier and the digest size. > > Signed-off-by: Roberto Sassu <rob...@hu...> > --- > drivers/char/tpm/tpm-interface.c | 4 +-- > drivers/char/tpm/tpm.h | 2 +- > drivers/char/tpm/tpm2-cmd.c | 55 ++++++++++++++++++++++++++++++++-------- > include/linux/tpm.h | 5 ++++ > 4 files changed, 52 insertions(+), 14 deletions(-) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 1d6729b..2c3d973 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -914,8 +914,8 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash) > memset(digest_list, 0, sizeof(digest_list)); > > for (i = 0; i < ARRAY_SIZE(chip->active_banks) && > - chip->active_banks[i] != TPM2_ALG_ERROR; i++) { > - digest_list[i].alg_id = chip->active_banks[i]; > + chip->active_banks[i].alg_id != TPM2_ALG_ERROR; i++) { > + digest_list[i].alg_id = chip->active_banks[i].alg_id; > memcpy(digest_list[i].digest, hash, TPM_DIGEST_SIZE); > count++; > } > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 2d5466a..fb94bd2 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -225,7 +225,7 @@ struct tpm_chip { > const struct attribute_group *groups[3]; > unsigned int groups_cnt; > > - u16 active_banks[7]; > + struct active_bank_info active_banks[7]; > #ifdef CONFIG_ACPI > acpi_handle acpi_dev_handle; > char ppi_version[TPM_PPI_VERSION_LEN + 1]; > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 0cad0f6..b1356be 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -291,7 +291,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count, > struct tpm2_null_auth_area auth_area; > int rc; > int i; > - int j; > > if (count > ARRAY_SIZE(chip->active_banks)) > return -EINVAL; > @@ -313,14 +312,10 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count, > tpm_buf_append_u32(&buf, count); > > for (i = 0; i < count; i++) { > - for (j = 0; j < ARRAY_SIZE(tpm2_hash_map); j++) { > - if (digests[i].alg_id != tpm2_hash_map[j].tpm_id) > - continue; > - tpm_buf_append_u16(&buf, digests[i].alg_id); > - tpm_buf_append(&buf, (const unsigned char > - *)&digests[i].digest, > - hash_digest_size[tpm2_hash_map[j].crypto_id]); > - } > + /* digests[i].alg_id == chip->active_banks[i].alg_id */ This comment should be removed. > + tpm_buf_append_u16(&buf, digests[i].alg_id); > + tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest, > + chip->active_banks[i].digest_size); > } > > rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, > @@ -943,6 +938,39 @@ int tpm2_probe(struct tpm_chip *chip) > } > EXPORT_SYMBOL_GPL(tpm2_probe); > > +static int tpm2_init_active_bank_info(struct tpm_chip *chip, u16 alg_id, > + struct active_bank_info *active_bank) > +{ > + struct tpm_buf buf; > + struct tpm2_pcr_read_out *out; > + int rc, i; One declaration per line. > + > + active_bank->alg_id = alg_id; > + > + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { > + enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id; > + > + if (active_bank->alg_id != tpm2_hash_map[i].tpm_id) > + continue; > + > + active_bank->digest_size = hash_digest_size[crypto_algo]; > + return 0; > + } > + > + rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ); > + if (rc) > + return rc; > + > + rc = tpm2_pcr_read_common(chip, 0, alg_id, &buf, NULL); > + if (rc == 0) { if (!rc) { > + out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE]; > + active_bank->digest_size = be16_to_cpu(out->digest_size); > + } > + > + tpm_buf_destroy(&buf); > + return 0; > +} > + > struct tpm2_pcr_selection { > __be16 hash_alg; > u8 size_of_select; > @@ -997,7 +1025,12 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) > } > > memcpy(&pcr_selection, marker, sizeof(pcr_selection)); > - chip->active_banks[i] = be16_to_cpu(pcr_selection.hash_alg); > + rc = tpm2_init_active_bank_info(chip, > + be16_to_cpu(pcr_selection.hash_alg), > + &chip->active_banks[i]); > + if (rc) > + break; > + > sizeof_pcr_selection = sizeof(pcr_selection.hash_alg) + > sizeof(pcr_selection.size_of_select) + > pcr_selection.size_of_select; > @@ -1006,7 +1039,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) > > out: > if (i < ARRAY_SIZE(chip->active_banks)) > - chip->active_banks[i] = TPM2_ALG_ERROR; > + chip->active_banks[i].alg_id = TPM2_ALG_ERROR; > > tpm_buf_destroy(&buf); > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 5a090f5..3ecce21 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -52,6 +52,11 @@ struct tpm_class_ops { > void (*relinquish_locality)(struct tpm_chip *chip, int loc); > }; > > +struct active_bank_info { > + u16 alg_id; > + u16 digest_size; > +}; "tpm_" prefix is missing. > + > #if defined(CONFIG_TCG_TPM) || defined(CONFIG_TCG_TPM_MODULE) > > extern int tpm_is_tpm2(u32 chip_num); > -- > 2.9.3 > /Jarkko |
From: Jarkko S. <jar...@li...> - 2017-10-04 10:45:28
|
On Mon, Sep 25, 2017 at 01:19:48PM +0200, Roberto Sassu wrote: > tpm2_pcr_read() copies the digest stored in a PCR to a buffer provided by > the caller. However, it does not return the digest size, included in the > output from the TPM. Retrieving it would be useful when a TPM algorithm > is not known by the crypto subsystem, which the TPM driver currently > depends upon. Remove this paragraph. It is just generic nonsense. > Most of tpm2_pcr_read() code is moved to the static function > tpm2_pcr_read_common(), which writes the output of the PCR read to the > tpm_buf structure passed as input. > > tpm2_pcr_read_common() will be called by tpm2_pcr_read(), and by the new > function tpm2_init_active_bank_info(), which will store the identifier > and the digest size of TPM algorithms in the tpm_chip structure. 1. Export tpm_buf to arch/x86/include/linux/tpm.h 2. Repeal and replace tpm2_pcr_read(). I would just pass one tpm_buf (i.e. no u8* res_buf) that is used both for input and output. Speaking about tpm2_inti_active_bank_info(), which is a *nonexistent* function is questionable. For me a sufficient commit message would be something like: " tpm: refine tpm2_pcr_read() access to all PCR banks Refine tpm2_pcr_read() interface and implementation in order to enable access to all PCR banks for other kernel subsystems such as IMA. " That describes all there is in this commit. /Jarkko |
From: Jarkko S. <jar...@li...> - 2017-10-04 07:33:16
|
Hi And apologies for late review. On Mon, Sep 25, 2017 at 01:19:47PM +0200, Roberto Sassu wrote: > This patch set derives from a larger patch set which modifies the TPM > driver API in order to extend a PCR with multiple digests. It can be > retrieved at the URL: > > https://sourceforge.net/p/tpmdd/mailman/message/35905412/ A patch set should be able to live on its own. Please remove this link. I don't care about that patch set at this point and I'm not going to give any distant promises. > The TPM driver currently relies on the crypto subsystem to determine the > digest size of supported TPM algorithms. In the future, TPM vendors might > implement new algorithms in their chips, and those algorithms might not > be supported by the crypto subsystem. > > Usually, vendors provide patches for the new hardware, and likely > the crypto subsystem will be updated before the new algorithm is > introduced. However, old kernels might be updated later, after patches > are included in the mainline kernel. This would leave the opportunity > for attackers to misuse PCRs, as PCR banks with an unknown algorithm > are not extended. > > This patch set provides a long term solution for this issue. If a TPM > algorithm is not known by the crypto subsystem, the TPM driver retrieves > the digest size from the TPM with a PCR read. All the PCR banks are > extended, even if the algorithm is not yet supported by the crypto > subsystem. This part makes sense to me. /Jarkko |
From: Nasim, K. <Kam...@wi...> - 2017-10-03 21:49:07
|
Looks like the appraisal is hanging in this function: $ ps -eo ppid,pid,user,stat,pcpu,comm,wchan:32 | grep D+ 3613 3614 root D+ 0.0 vim vfs_setxattr <<<<<<<<<<<<<< From: Nasim, Kam Sent: Tuesday, October 3, 2017 5:41 PM To: lin...@li... Cc: Element, Paul-Emile Subject: vim editor gets stuck on CLOSE when running IMA LOG Appraisal Hi Experts, I am testing our IMA appraisal (appraise_type=log), to see how IMA would log appraisal failures. Everything seems fine except that when I close my file (without any edits), vim just hangs ( I need to kill the process manually) My policy: $ cat /etc/ima.policy # EXT4_SUPER_MAGIC measure func=FILE_CHECK uid=0 fsmagic=0xEF53 appraise func=FILE_MMAP mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 appraise func=FILE_CHECK mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 appraise func=BPRM_CHECK mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 My audit log entries (appraise_data / missing-hash): type=SYSCALL msg=audit(1507066020.544:1638): arch=c000003e syscall=9 success=yes exit=139688952565760 a0=0 a1=202110 a2=5 a3=802 items=0 ppid=3613 pid=3614 auid=1875 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=2 comm="vim" exe="/usr/bin/vim" key=(null) type=INTEGRITY_DATA msg=audit(1507066020.544:1639): pid=3614 uid=0 auid=1875 ses=2 op="appraise_data" cause="missing-hash" comm=vim name=/usr/lib64/libfreebl3.so dev=sda3 ino=785508 res=0 type=SYSCALL msg=audit(1507066020.544:1639): arch=c000003e syscall=9 success=yes exit=139688950456320 a0=0 a1=2020c0 a2=5 a3=802 items=0 ppid=3613 pid=3614 auid=1875 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=2 comm="vim" exe="/usr/bin/vim" key=(null) type=INTEGRITY_DATA msg=audit(1507066020.551:1640): pid=3614 uid=0 auid=1875 ses=2 op="appraise_data" cause="missing-hash" comm=vim name=/usr/lib64/libnss_files-2.17.so dev=sda3 ino=785569 res=0 vim stuck after I try closing it with :q localhost:~$ ps -auxf | grep vim root 3613 0.0 0.0 198120 3432 pts/0 S+ 21:26 0:00 | \_ sudo vim /var/log/audit/audit.log root 3614 0.0 0.0 150716 4488 pts/0 D+ 21:26 0:00 | \_ vim /var/log/audit/audit.log <<<<<<<<<<<<<<<<< Looks like my process is in Uninterruptible Sleep. Any ideas why this might be happening? Thanks, Kam -------------- next part -------------- An HTML attachment was scrubbed... |
From: Nasim, K. <Kam...@wi...> - 2017-10-03 21:40:44
|
Hi Experts, I am testing our IMA appraisal (appraise_type=log), to see how IMA would log appraisal failures. Everything seems fine except that when I close my file (without any edits), vim just hangs ( I need to kill the process manually) My policy: $ cat /etc/ima.policy # EXT4_SUPER_MAGIC measure func=FILE_CHECK uid=0 fsmagic=0xEF53 appraise func=FILE_MMAP mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 appraise func=FILE_CHECK mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 appraise func=BPRM_CHECK mask=MAY_EXEC uid=0 appraise_type=imasig fsmagic=0xEF53 My audit log entries (appraise_data / missing-hash): type=SYSCALL msg=audit(1507066020.544:1638): arch=c000003e syscall=9 success=yes exit=139688952565760 a0=0 a1=202110 a2=5 a3=802 items=0 ppid=3613 pid=3614 auid=1875 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=2 comm="vim" exe="/usr/bin/vim" key=(null) type=INTEGRITY_DATA msg=audit(1507066020.544:1639): pid=3614 uid=0 auid=1875 ses=2 op="appraise_data" cause="missing-hash" comm=vim name=/usr/lib64/libfreebl3.so dev=sda3 ino=785508 res=0 type=SYSCALL msg=audit(1507066020.544:1639): arch=c000003e syscall=9 success=yes exit=139688950456320 a0=0 a1=2020c0 a2=5 a3=802 items=0 ppid=3613 pid=3614 auid=1875 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=2 comm="vim" exe="/usr/bin/vim" key=(null) type=INTEGRITY_DATA msg=audit(1507066020.551:1640): pid=3614 uid=0 auid=1875 ses=2 op="appraise_data" cause="missing-hash" comm=vim name=/usr/lib64/libnss_files-2.17.so dev=sda3 ino=785569 res=0 vim stuck after I try closing it with :q localhost:~$ ps -auxf | grep vim root 3613 0.0 0.0 198120 3432 pts/0 S+ 21:26 0:00 | \_ sudo vim /var/log/audit/audit.log root 3614 0.0 0.0 150716 4488 pts/0 D+ 21:26 0:00 | \_ vim /var/log/audit/audit.log <<<<<<<<<<<<<<<<< Looks like my process is in Uninterruptible Sleep. Any ideas why this might be happening? Thanks, Kam -------------- next part -------------- An HTML attachment was scrubbed... |
From: er_tou <er...@16...> - 2017-09-29 09:29:40
|
hello, I encountered a problem, execute the following command to show the error. $ su -c 'mkdir -p /etc/keys' # To create and save the kernel master key (user type): $ su -c 'modprobe trusted encrypted' $ su -c 'keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u' $ su -c 'keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob' show "keyctl_read_alloc: Permission denied". Problem location is 'keyctl pipe'. [root@localhost ima_key_sign]# cat /proc/keys 029fddf9 I------ 1 perm 1f030000 0 0 asymmetri CentOS Linux kernel signing key: d48863a7c16fcc274123e6298f74f057af19fc54: X509.RSA af19fc54 [] 034d0e68 I--Q--- 1 perm 1f3f0000 1000 65534 keyring _uid.1000: empty 0a1ab8e3 I--Q--- 1 perm 3f010000 0 0 user kmk-user: 10 This keyring “kmk-user” don't have read permission. Anyone have the question ? I hope get help? Thank, Thank, Thank ---------------------------------------------------------------------------------------- but, below command can execute. $ mkdir -p /etc/keys # To create and save the kernel master key (user type): $ modprobe trusted encrypted $ keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u $ keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob Thanks,Looking forward to your reply! 【网易自营|30天无忧退货】仅售同款价1/4!MUJI制造商“2017秋冬舒适家居拖鞋系列”限时仅34.9元>> -------------- next part -------------- An HTML attachment was scrubbed... |
From: er_tou <er...@16...> - 2017-09-27 07:44:33
|
Hello,everyone,I encounter this problem on centos 7(kernel 3.10.0-514.el7.x86_64). when execute “$ su -c 'keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob'” command, show "keyctl_read_alloc: Permission denied"[ost ima_key_sign]$ su -c 'cat /proc/keys 'Password: 01e429d6 I--Q--- 149 perm 3f030000 1000 1000 keyring _ses: 1 031b7c53 I--Q--- 1 perm 3f010000 0 0 user kmk-user: 10 Who can help me? Thanks ====================================================================================== " $ su -c 'mkdir -p /etc/keys' # To create and save the kernel master key (user type): $ su -c 'modprobe trusted encrypted' $ su -c 'keyctl add user kmk-user "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u' $ su -c 'keyctl pipe `keyctl search @u user kmk-user` > /etc/keys/kmk-user.blob' # Create the EVM encrypted key $ su -c 'keyctl add encrypted evm-key "new user:kmk-user 32" @u' $ su -c 'keyctl pipe `keyctl search @u encrypted evm-key` >/etc/keys/evm-user.blob' " -------------- next part -------------- An HTML attachment was scrubbed... |
From: Sascha H. <s....@pe...> - 2017-09-27 06:44:02
|
On Tue, Sep 26, 2017 at 12:07:23PM -0400, Mimi Zohar wrote: > [Cc'ing lin...@vg...] > > Hi Sascha, > > Jarkko recently announced a new linux-integrity mailing list that > we'll be using for both TPM and IMA discussions. > > On Thu, 2017-09-21 at 11:44 +0200, Sascha Hauer wrote: > > On Wed, Sep 20, 2017 at 08:06:27AM -0400, Mimi Zohar wrote: > > > Hi Sascha, > > > > > > On Wed, 2017-09-20 at 09:23 +0200, Sascha Hauer wrote: > > > > Mimi, > > > > > > > > On Wed, Sep 13, 2017 at 04:15:13PM +0200, Sascha Hauer wrote: > > > > > IMA uses the inode's i_version field to detect changes on an inode. > > > > > This seems to be an optimization for IMA and not strictly necessary. > > > > > Just ignore the i_version field if it is zero and measure the file > > > > > anyway. On filesystems which do not support i_version this may result > > > > > in an unnecessary re-measurement of a file when it has been opened for > > > > > writing without anything actually being written. For filesystems with > > > > > i_version support the behaviour doesn't change. > > > > > > > > > > Signed-off-by: Sascha Hauer <s....@pe...> > > > > > --- > > > > > security/integrity/ima/ima_main.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > I'm not sure if this patch is appropriate, but even when it's not it > > > > > would be interesting to know why it isn't. > > > > > > > > Any input to this one? > > > > > > Sorry, I'm still thinking about it. For filesystems that > > > automatically enable i_version there would be no difference. For > > > filesystems that require a mount option to enable i_version, this > > > changes the behavior. > > > > > > This is slightly different than not caching the integrity results, in > > > that the cache is only cleared if someone opens the file rw. > > > > > > (Jeff Layton posted a patch that replaces the i_version checks with > > > atime/mtime.) > > > > Looking at that patch I think that using i_version only when > > MS_I_VERSION is set is a useful change because otherwise IMA > > ends up using i_version when it contains no sensible values. > > > > I can't judge whether mtime is fine grained enough on all systems, > > but I don't think it's necessary to use it. > > > > My version of ima_should_update_iint() would look like: > > > > static bool ima_should_update_iint(struct integrity_iint_cache *iint, > > struct inode *inode) > > { > > if (atomic_read(&inode->i_writecount) != 1) > > return false; > > if (iint->flags & IMA_NEW_FILE) > > return true; > > if (IS_I_VERSION(inode) && iint->version == inode->i_version) > > return false; > > return true; > > } > > The only reason for a new function would be to call it from multiple > places. The other place would probably affect caching the integrity > results. This change you're proposing is simple enough to reason > about. If we decide at a later date that we need a new function, > we'll refactor the code then. For now, could you make this change in > ima_check_last_writer()? I liked the new function approach because I think it makes the code slightly easier to read. Anyway, I just sent a new patch to the new list. Thanks Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | |
From: Mimi Z. <zo...@li...> - 2017-09-27 06:29:46
|
[Cc'ing lin...@vg...] Hi Sascha, Jarkko recently announced a new linux-integrity mailing list that we'll be using for both TPM and IMA discussions. On Thu, 2017-09-21 at 11:44 +0200, Sascha Hauer wrote: > On Wed, Sep 20, 2017 at 08:06:27AM -0400, Mimi Zohar wrote: > > Hi Sascha, > > > > On Wed, 2017-09-20 at 09:23 +0200, Sascha Hauer wrote: > > > Mimi, > > > > > > On Wed, Sep 13, 2017 at 04:15:13PM +0200, Sascha Hauer wrote: > > > > IMA uses the inode's i_version field to detect changes on an inode. > > > > This seems to be an optimization for IMA and not strictly necessary. > > > > Just ignore the i_version field if it is zero and measure the file > > > > anyway. On filesystems which do not support i_version this may result > > > > in an unnecessary re-measurement of a file when it has been opened for > > > > writing without anything actually being written. For filesystems with > > > > i_version support the behaviour doesn't change. > > > > > > > > Signed-off-by: Sascha Hauer <s....@pe...> > > > > --- > > > > security/integrity/ima/ima_main.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > I'm not sure if this patch is appropriate, but even when it's not it > > > > would be interesting to know why it isn't. > > > > > > Any input to this one? > > > > Sorry, I'm still thinking about it. For filesystems that > > automatically enable i_version there would be no difference. For > > filesystems that require a mount option to enable i_version, this > > changes the behavior. > > > > This is slightly different than not caching the integrity results, in > > that the cache is only cleared if someone opens the file rw. > > > > (Jeff Layton posted a patch that replaces the i_version checks with > > atime/mtime.) > > Looking at that patch I think that using i_version only when > MS_I_VERSION is set is a useful change because otherwise IMA > ends up using i_version when it contains no sensible values. > > I can't judge whether mtime is fine grained enough on all systems, > but I don't think it's necessary to use it. > > My version of ima_should_update_iint() would look like: > > static bool ima_should_update_iint(struct integrity_iint_cache *iint, > struct inode *inode) > { > if (atomic_read(&inode->i_writecount) != 1) > return false; > if (iint->flags & IMA_NEW_FILE) > return true; > if (IS_I_VERSION(inode) && iint->version == inode->i_version) > return false; > return true; > } The only reason for a new function would be to call it from multiple places. The other place would probably affect caching the integrity results. This change you're proposing is simple enough to reason about. If we decide at a later date that we need a new function, we'll refactor the code then. For now, could you make this change in ima_check_last_writer()? > That is, when we don't know for sure that an inode has not changed, we > must assume that it has changed and remeasure it. When in doubt we must > make sure IMA is working as expected, everything else is performance > optimization. Agreed thanks, Mimi |
From: A2Zbuyonline.us <in...@ve...> - 2017-09-27 05:34:33
|
Your email client cannot read this email. To view it online, please go here: http://lifetornado.com/display.php?M=84678&C=e32c10893092a48b154b20b3aafa2a2c&S=6&L=2&N=5 To stop receiving these emails:http://lifetornado.com/unsubscribe.php?M=84678&C=e32c10893092a48b154b20b3aafa2a2c&L=2&N=6 -------------- next part -------------- An HTML attachment was scrubbed... |
From: Roberto S. <rob...@hu...> - 2017-09-25 11:43:06
|
tpm2_pcr_read() copies the digest stored in a PCR to a buffer provided by the caller. However, it does not return the digest size, included in the output from the TPM. Retrieving it would be useful when a TPM algorithm is not known by the crypto subsystem, which the TPM driver currently depends upon. Most of tpm2_pcr_read() code is moved to the static function tpm2_pcr_read_common(), which writes the output of the PCR read to the tpm_buf structure passed as input. tpm2_pcr_read_common() will be called by tpm2_pcr_read(), and by the new function tpm2_init_active_bank_info(), which will store the identifier and the digest size of TPM algorithms in the tpm_chip structure. Signed-off-by: Roberto Sassu <rob...@hu...> --- drivers/char/tpm/tpm2-cmd.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index e1a41b7..0cad0f6 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -218,6 +218,26 @@ struct tpm2_pcr_read_out { u8 digest[]; } __packed; +static int tpm2_pcr_read_common(struct tpm_chip *chip, int pcr_idx, + enum tpm2_algorithms algo, struct tpm_buf *buf, + char *msg) +{ + u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0}; + + if (pcr_idx >= TPM2_PLATFORM_PCR) + return -EINVAL; + + pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7); + + tpm_buf_append_u32(buf, 1); + tpm_buf_append_u16(buf, algo); + tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN); + tpm_buf_append(buf, (const unsigned char *)pcr_select, + sizeof(pcr_select)); + + return tpm_transmit_cmd(chip, NULL, buf->data, PAGE_SIZE, 0, 0, msg); +} + /** * tpm2_pcr_read() - read a PCR value * @chip: TPM chip to use. @@ -231,24 +251,12 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf) int rc; struct tpm_buf buf; struct tpm2_pcr_read_out *out; - u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0}; - - if (pcr_idx >= TPM2_PLATFORM_PCR) - return -EINVAL; rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ); if (rc) return rc; - pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7); - - tpm_buf_append_u32(&buf, 1); - tpm_buf_append_u16(&buf, TPM2_ALG_SHA1); - tpm_buf_append_u8(&buf, TPM2_PCR_SELECT_MIN); - tpm_buf_append(&buf, (const unsigned char *)pcr_select, - sizeof(pcr_select)); - - rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, + rc = tpm2_pcr_read_common(chip, pcr_idx, TPM2_ALG_SHA1, &buf, res_buf ? "attempting to read a pcr value" : NULL); if (rc == 0 && res_buf) { out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE]; -- 2.9.3 |
From: Roberto S. <rob...@hu...> - 2017-09-25 11:42:31
|
This patch set derives from a larger patch set which modifies the TPM driver API in order to extend a PCR with multiple digests. It can be retrieved at the URL: https://sourceforge.net/p/tpmdd/mailman/message/35905412/ The TPM driver currently relies on the crypto subsystem to determine the digest size of supported TPM algorithms. In the future, TPM vendors might implement new algorithms in their chips, and those algorithms might not be supported by the crypto subsystem. Usually, vendors provide patches for the new hardware, and likely the crypto subsystem will be updated before the new algorithm is introduced. However, old kernels might be updated later, after patches are included in the mainline kernel. This would leave the opportunity for attackers to misuse PCRs, as PCR banks with an unknown algorithm are not extended. This patch set provides a long term solution for this issue. If a TPM algorithm is not known by the crypto subsystem, the TPM driver retrieves the digest size from the TPM with a PCR read. All the PCR banks are extended, even if the algorithm is not yet supported by the crypto subsystem. Roberto Sassu (3): tpm: move PCR read code to static function tpm2_pcr_read_common() tpm: retrieve digest size of unknown algorithms with PCR read tpm: add the crypto algorithm identifier to active_bank_info drivers/char/tpm/tpm-interface.c | 4 +- drivers/char/tpm/tpm.h | 2 +- drivers/char/tpm/tpm2-cmd.c | 91 +++++++++++++++++++++++++++++----------- include/linux/tpm.h | 6 +++ 4 files changed, 76 insertions(+), 27 deletions(-) -- 2.9.3 |
From: Roberto S. <rob...@hu...> - 2017-09-25 11:34:09
|
In preparation for the patch introducing a function to pass supported TPM algorithms and digest sizes to TPM users, the crypto algorithm identifier is added to the active_bank_info structure. All members of active_bank_info are necessary: TPM algorithm identifiers will be used to create an event log (they are included in the Crypto Agile format defined by TCG); crypto identifiers will be used to calculate digests using the crypto subsystem; digest sizes will be used to truncate digests calculated with different algorithms. Signed-off-by: Roberto Sassu <rob...@hu...> --- drivers/char/tpm/tpm2-cmd.c | 2 ++ include/linux/tpm.h | 1 + 2 files changed, 3 insertions(+) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index b1356be..8c58f6e 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -946,6 +946,7 @@ static int tpm2_init_active_bank_info(struct tpm_chip *chip, u16 alg_id, int rc, i; active_bank->alg_id = alg_id; + active_bank->crypto_id = HASH_ALGO__LAST; for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { enum hash_algo crypto_algo = tpm2_hash_map[i].crypto_id; @@ -954,6 +955,7 @@ static int tpm2_init_active_bank_info(struct tpm_chip *chip, u16 alg_id, continue; active_bank->digest_size = hash_digest_size[crypto_algo]; + active_bank->crypto_id = crypto_algo; return 0; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 3ecce21..fc927f3 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -54,6 +54,7 @@ struct tpm_class_ops { struct active_bank_info { u16 alg_id; + u16 crypto_id; u16 digest_size; }; -- 2.9.3 |