| Message ID | 20240124193223.46383-1-enrico.bravi@polito.it (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ima: add crypto agility support for template-hash algorithm | expand |
On Wed, 2024-01-24 at 20:32 +0100, Enrico Bravi wrote: > The template hash showed by the ascii_runtime_measurements and > binary_runtime_measurements is the one calculated using sha1 and there is no > possibility to change this value, despite the fact that the template hash is > calculated using the hash algorithms corresponding to all the PCR banks > configured in the TPM. Hi Enrico the missing part is that we should not modify existing files, to avoid breaking existing applications. > Add the support to retrieve the ima log with the template data hash calculated > with a specific hash algorithm. > Add a new file in the securityfs ima directory for each hash algo configured > for the PCR banks of the TPM. Each new file has the name with the following > structure: > > {binary, ascii}_runtime_measurements_<hash_algo_name> > > The <hash_algo_name> is used to select the template data hash algorithm to show > in ima_ascii_measurements_show() and in ima_measurements_show(). > Legacy files are kept but as sysmbolic links which point to Typo. Please use codespell on the patch. > {binary, ascii}_runtime_measurements_sha1 files. These two files are created > even if a TPM chip is not detected. > > As example, in the case a TPM chip is present and sha1 and sha256 are the > configured PCR banks, the listing of the security/ima directory is the following: > > lr--r--r-- [...] ascii_runtime_measurements -> ascii_runtime_measurements_sha1 > -r--r----- [...] ascii_runtime_measurements_sha1 > -r--r----- [...] ascii_runtime_measurements_sha256 > lr--r--r-- [...] binary_runtime_measurements -> binary_runtime_measurements_sha1 > -r--r----- [...] binary_runtime_measurements_sha1 > -r--r----- [...] binary_runtime_measurements_sha256 > --w------- [...] policy > -r--r----- [...] runtime_measurements_count > -r--r----- [...] violations Ok, great. > Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> > Signed-off-by: Silvia Sisinni <silvia.sisinni@polito.it> > > --- > > v3: > - Added create_measurements_list_files function for measurements files creation. > - Parametrized the remove_measurements_list_files function and add NULL > check before freeing files' list. > - Removed algorithm selection based on file name during ima_measurements_show > and ima_ascii_mesurements_show, and selecting it comparing dentry address. > - Allocate also sha1 file following the schema > {binary, ascii}_runtime_measurements_<hash_algo_name> and keep legacy > files as symbolic links to those files. > - Allocate measurements files lists even if a TPM chip is not detected, > adding only sha1 files. > > v2: > - Changed the behaviour of configuring at boot time the template data hash > algorithm. > - Removed template data hash algo name prefix. > - Removed ima_template_hash command line option. > - Introducing a new file in the securityfs ima subdir for each PCR banks > algorithm configured in the TPM. > (suggested by Roberto) > > security/integrity/ima/ima_fs.c | 145 +++++++++++++++++++++++++++++--- > 1 file changed, 131 insertions(+), 14 deletions(-) > > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index cd1683dad3bf..fb65ba9426a1 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -116,9 +116,12 @@ void ima_putc(struct seq_file *m, void *data, int datalen) > seq_putc(m, *(char *)data++); > } > > +static struct dentry **ima_ascii_measurements_files; > +static struct dentry **ima_binary_measurements_files; > + > /* print format: > * 32bit-le=pcr# > - * char[20]=template digest > + * char[n]=template digest > * 32bit-le=template name size > * char[n]=template name > * [eventdata length] > @@ -130,9 +133,25 @@ int ima_measurements_show(struct seq_file *m, void *v) > struct ima_queue_entry *qe = v; > struct ima_template_entry *e; > char *template_name; > + struct dentry *dentry; > u32 pcr, namelen, template_data_len; /* temporary fields */ > bool is_ima_template = false; > - int i; > + int i, algo_idx; > + enum hash_algo algo; > + > + dentry = m->file->f_path.dentry; I like more file_dentry(m->file). > + algo_idx = ima_sha1_idx; > + algo = HASH_ALGO_SHA1; > + > + if (ima_tpm_chip) { > + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { > + if (dentry == ima_binary_measurements_files[i]) { > + algo_idx = i; > + algo = ima_tpm_chip->allocated_banks[i].crypto_id; > + break; > + } > + } > + } > > /* get entry */ > e = qe->entry; > @@ -151,7 +170,7 @@ int ima_measurements_show(struct seq_file *m, void *v) > ima_putc(m, &pcr, sizeof(e->pcr)); > > /* 2nd: template digest */ > - ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); > + ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]); > > /* 3rd: template name size */ > namelen = !ima_canonical_fmt ? strlen(template_name) : > @@ -220,7 +239,23 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) > struct ima_queue_entry *qe = v; > struct ima_template_entry *e; > char *template_name; > - int i; > + struct dentry *dentry; > + int i, algo_idx; > + enum hash_algo algo; > + > + dentry = m->file->f_path.dentry; Same. > + algo_idx = ima_sha1_idx; > + algo = HASH_ALGO_SHA1; > + > + if (ima_tpm_chip) { > + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { > + if (dentry == ima_ascii_measurements_files[i]) { Uhm, why not iterating over ima_ascii_measurements_files? But I have another comment on this. Have a look at ima_crypto.c: if (ima_sha1_idx < 0) { ima_sha1_idx = NR_BANKS(ima_tpm_chip) + ima_extra_slots++; if (ima_hash_algo == HASH_ALGO_SHA1) ima_hash_algo_idx = ima_sha1_idx; } This is done because not necessarily the TPM has a SHA1 bank. ima_extra_slots is already exported, you could use that to determine if you need more slots than NR_BANKS(ima_tpm_chip). > + algo_idx = i; > + algo = ima_tpm_chip->allocated_banks[i].crypto_id; > + break; > + } > + } > + } > > /* get entry */ > e = qe->entry; > @@ -233,8 +268,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) > /* 1st: PCR used (config option) */ > seq_printf(m, "%2d ", e->pcr); > > - /* 2nd: SHA1 template hash */ > - ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); > + /* 2nd: template hash */ > + ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]); > > /* 3th: template name */ > seq_printf(m, " %s", template_name); > @@ -379,6 +414,84 @@ static const struct seq_operations ima_policy_seqops = { > }; > #endif > > +/* > + * Remove the securityfs files created for each hash algo configured > + * in the TPM, excluded ascii_runtime_measurements and > + * binary_runtime_measurements. > + */ It does not hurt following the kernel-doc format. > +static void remove_measurements_list_files(struct dentry **files) > +{ > + int i, len; > + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; It is much better having a global variable with the number of array elements. > + > + if (files) > + { This bracket should be one line up. > + for (i = 0; i < len; i++) { > + if (files[i]) { > + securityfs_remove(files[i]); > + } > + } > + > + kfree(files); > + } > +} > + > +/* > + * Allocate a file in the securityfs for each hash algo configured > + * in the TPM (for ascii and binary output). In case no TPM chip is > + * detected only sha1 files are created. > + */ > +static int create_measurements_list_files(void) > +{ > + int i, len; > + u16 algo; > + char file_name[NAME_MAX+1]; > + struct dentry *dfile; > + > + /* > + * Allocate NR_BANKS(ima_tpm_chip) files in case a tpm chip is detected, > + * otherwise allocate just one file for sha1. > + */ > + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; See the comment above regarding ima_extra_slots. If you export len (static), you can always use that without extra logic. > + > + ima_ascii_measurements_files = kmalloc_array(len, > + sizeof(struct dentry *), GFP_KERNEL); > + if(!ima_ascii_measurements_files) > + return -ENOMEM; > + > + ima_binary_measurements_files = kmalloc_array(len, > + sizeof(struct dentry *), GFP_KERNEL); > + if(!ima_binary_measurements_files) > + return -ENOMEM; > + > + for (i = 0; i < len; i++) { > + algo = ima_tpm_chip ? ima_tpm_chip->allocated_banks[i].crypto_id : > + HASH_ALGO_SHA1; I'm starting to think that maybe we should export ima_algo_array instead, and follow that. > + > + sprintf(file_name, "ascii_runtime_measurements_%s", > + hash_algo_name[algo]); > + dfile = securityfs_create_file(file_name, > + S_IRUSR | S_IRGRP, ima_dir, NULL, > + &ima_ascii_measurements_ops); > + if (IS_ERR(dfile)) > + return PTR_ERR(dfile); > + > + ima_ascii_measurements_files[i] = dfile; > + > + sprintf(file_name, "binary_runtime_measurements_%s", > + hash_algo_name[algo]); > + dfile = securityfs_create_file(file_name, > + S_IRUSR | S_IRGRP, ima_dir, NULL, > + &ima_measurements_ops); > + if (IS_ERR(dfile)) > + return PTR_ERR(dfile); > + > + ima_binary_measurements_files[i] = dfile; > + } > + > + return 0; > +} > + > /* > * ima_open_policy: sequentialize access to the policy file > */ > @@ -465,19 +578,21 @@ int __init ima_fs_init(void) > goto out; > } > > - binary_runtime_measurements = > - securityfs_create_file("binary_runtime_measurements", > - S_IRUSR | S_IRGRP, ima_dir, NULL, > - &ima_measurements_ops); > + ret = create_measurements_list_files(); > + if (ret != 0) > + goto out; > + > + binary_runtime_measurements = securityfs_create_symlink( > + "binary_runtime_measurements", ima_dir, > + "binary_runtime_measurements_sha1", NULL); > if (IS_ERR(binary_runtime_measurements)) { > ret = PTR_ERR(binary_runtime_measurements); > goto out; > } > > - ascii_runtime_measurements = > - securityfs_create_file("ascii_runtime_measurements", > - S_IRUSR | S_IRGRP, ima_dir, NULL, > - &ima_ascii_measurements_ops); > + ascii_runtime_measurements = securityfs_create_symlink( > + "ascii_runtime_measurements", ima_dir, > + "ascii_runtime_measurements_sha1", NULL); > if (IS_ERR(ascii_runtime_measurements)) { > ret = PTR_ERR(ascii_runtime_measurements); > goto out; > @@ -515,6 +630,8 @@ int __init ima_fs_init(void) > securityfs_remove(runtime_measurements_count); > securityfs_remove(ascii_runtime_measurements); > securityfs_remove(binary_runtime_measurements); > + remove_measurements_list_files(ima_ascii_measurements_files); > + remove_measurements_list_files(ima_binary_measurements_files); The rest is ok. Thanks Roberto > securityfs_remove(ima_symlink); > securityfs_remove(ima_dir); > > base-commit: 88035e5694a86a7167d490bb95e9df97a9bb162b
On Thu, 2024-01-25 at 09:30 +0100, Roberto Sassu wrote: > On Wed, 2024-01-24 at 20:32 +0100, Enrico Bravi wrote: > > The template hash showed by the ascii_runtime_measurements and > > binary_runtime_measurements is the one calculated using sha1 and there is no > > possibility to change this value, despite the fact that the template hash is > > calculated using the hash algorithms corresponding to all the PCR banks > > configured in the TPM. > > Hi Enrico > > the missing part is that we should not modify existing files, to avoid > breaking existing applications. Forgot: please use scripts/get_maintainer.pl to find the correct recipients of the patch (I think you only need to include maintainers and reviewers). Thanks Roberto > > Add the support to retrieve the ima log with the template data hash calculated > > with a specific hash algorithm. > > Add a new file in the securityfs ima directory for each hash algo configured > > for the PCR banks of the TPM. Each new file has the name with the following > > structure: > > > > {binary, ascii}_runtime_measurements_<hash_algo_name> > > > > The <hash_algo_name> is used to select the template data hash algorithm to show > > in ima_ascii_measurements_show() and in ima_measurements_show(). > > Legacy files are kept but as sysmbolic links which point to > > Typo. Please use codespell on the patch. > > > {binary, ascii}_runtime_measurements_sha1 files. These two files are created > > even if a TPM chip is not detected. > > > > As example, in the case a TPM chip is present and sha1 and sha256 are the > > configured PCR banks, the listing of the security/ima directory is the following: > > > > lr--r--r-- [...] ascii_runtime_measurements -> ascii_runtime_measurements_sha1 > > -r--r----- [...] ascii_runtime_measurements_sha1 > > -r--r----- [...] ascii_runtime_measurements_sha256 > > lr--r--r-- [...] binary_runtime_measurements -> binary_runtime_measurements_sha1 > > -r--r----- [...] binary_runtime_measurements_sha1 > > -r--r----- [...] binary_runtime_measurements_sha256 > > --w------- [...] policy > > -r--r----- [...] runtime_measurements_count > > -r--r----- [...] violations > > Ok, great. > > > Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> > > Signed-off-by: Silvia Sisinni <silvia.sisinni@polito.it> > > > > --- > > > > v3: > > - Added create_measurements_list_files function for measurements files creation. > > - Parametrized the remove_measurements_list_files function and add NULL > > check before freeing files' list. > > - Removed algorithm selection based on file name during ima_measurements_show > > and ima_ascii_mesurements_show, and selecting it comparing dentry address. > > - Allocate also sha1 file following the schema > > {binary, ascii}_runtime_measurements_<hash_algo_name> and keep legacy > > files as symbolic links to those files. > > - Allocate measurements files lists even if a TPM chip is not detected, > > adding only sha1 files. > > > > v2: > > - Changed the behaviour of configuring at boot time the template data hash > > algorithm. > > - Removed template data hash algo name prefix. > > - Removed ima_template_hash command line option. > > - Introducing a new file in the securityfs ima subdir for each PCR banks > > algorithm configured in the TPM. > > (suggested by Roberto) > > > > security/integrity/ima/ima_fs.c | 145 +++++++++++++++++++++++++++++--- > > 1 file changed, 131 insertions(+), 14 deletions(-) > > > > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > > index cd1683dad3bf..fb65ba9426a1 100644 > > --- a/security/integrity/ima/ima_fs.c > > +++ b/security/integrity/ima/ima_fs.c > > @@ -116,9 +116,12 @@ void ima_putc(struct seq_file *m, void *data, int datalen) > > seq_putc(m, *(char *)data++); > > } > > > > +static struct dentry **ima_ascii_measurements_files; > > +static struct dentry **ima_binary_measurements_files; > > + > > /* print format: > > * 32bit-le=pcr# > > - * char[20]=template digest > > + * char[n]=template digest > > * 32bit-le=template name size > > * char[n]=template name > > * [eventdata length] > > @@ -130,9 +133,25 @@ int ima_measurements_show(struct seq_file *m, void *v) > > struct ima_queue_entry *qe = v; > > struct ima_template_entry *e; > > char *template_name; > > + struct dentry *dentry; > > u32 pcr, namelen, template_data_len; /* temporary fields */ > > bool is_ima_template = false; > > - int i; > > + int i, algo_idx; > > + enum hash_algo algo; > > + > > + dentry = m->file->f_path.dentry; > > I like more file_dentry(m->file). > > > + algo_idx = ima_sha1_idx; > > + algo = HASH_ALGO_SHA1; > > + > > + if (ima_tpm_chip) { > > + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { > > + if (dentry == ima_binary_measurements_files[i]) { > > + algo_idx = i; > > + algo = ima_tpm_chip->allocated_banks[i].crypto_id; > > + break; > > + } > > + } > > + } > > > > /* get entry */ > > e = qe->entry; > > @@ -151,7 +170,7 @@ int ima_measurements_show(struct seq_file *m, void *v) > > ima_putc(m, &pcr, sizeof(e->pcr)); > > > > /* 2nd: template digest */ > > - ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); > > + ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]); > > > > /* 3rd: template name size */ > > namelen = !ima_canonical_fmt ? strlen(template_name) : > > @@ -220,7 +239,23 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) > > struct ima_queue_entry *qe = v; > > struct ima_template_entry *e; > > char *template_name; > > - int i; > > + struct dentry *dentry; > > + int i, algo_idx; > > + enum hash_algo algo; > > + > > + dentry = m->file->f_path.dentry; > > Same. > > > + algo_idx = ima_sha1_idx; > > + algo = HASH_ALGO_SHA1; > > + > > + if (ima_tpm_chip) { > > + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { > > + if (dentry == ima_ascii_measurements_files[i]) { > > Uhm, why not iterating over ima_ascii_measurements_files? > > But I have another comment on this. > > Have a look at ima_crypto.c: > > if (ima_sha1_idx < 0) { > ima_sha1_idx = NR_BANKS(ima_tpm_chip) + ima_extra_slots++; > if (ima_hash_algo == HASH_ALGO_SHA1) > ima_hash_algo_idx = ima_sha1_idx; > } > > This is done because not necessarily the TPM has a SHA1 bank. > > ima_extra_slots is already exported, you could use that to determine if > you need more slots than NR_BANKS(ima_tpm_chip). > > > + algo_idx = i; > > + algo = ima_tpm_chip->allocated_banks[i].crypto_id; > > + break; > > + } > > + } > > + } > > > > /* get entry */ > > e = qe->entry; > > @@ -233,8 +268,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) > > /* 1st: PCR used (config option) */ > > seq_printf(m, "%2d ", e->pcr); > > > > - /* 2nd: SHA1 template hash */ > > - ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); > > + /* 2nd: template hash */ > > + ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]); > > > > /* 3th: template name */ > > seq_printf(m, " %s", template_name); > > @@ -379,6 +414,84 @@ static const struct seq_operations ima_policy_seqops = { > > }; > > #endif > > > > +/* > > + * Remove the securityfs files created for each hash algo configured > > + * in the TPM, excluded ascii_runtime_measurements and > > + * binary_runtime_measurements. > > + */ > > It does not hurt following the kernel-doc format. > > > +static void remove_measurements_list_files(struct dentry **files) > > +{ > > + int i, len; > > + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; > > It is much better having a global variable with the number of array > elements. > > > + > > + if (files) > > + { > > This bracket should be one line up. > > > + for (i = 0; i < len; i++) { > > + if (files[i]) { > > + securityfs_remove(files[i]); > > + } > > + } > > + > > + kfree(files); > > + } > > +} > > + > > +/* > > + * Allocate a file in the securityfs for each hash algo configured > > + * in the TPM (for ascii and binary output). In case no TPM chip is > > + * detected only sha1 files are created. > > + */ > > +static int create_measurements_list_files(void) > > +{ > > + int i, len; > > + u16 algo; > > + char file_name[NAME_MAX+1]; > > + struct dentry *dfile; > > + > > + /* > > + * Allocate NR_BANKS(ima_tpm_chip) files in case a tpm chip is detected, > > + * otherwise allocate just one file for sha1. > > + */ > > + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; > > See the comment above regarding ima_extra_slots. > > If you export len (static), you can always use that without extra > logic. > > > + > > + ima_ascii_measurements_files = kmalloc_array(len, > > + sizeof(struct dentry *), GFP_KERNEL); > > + if(!ima_ascii_measurements_files) > > + return -ENOMEM; > > + > > + ima_binary_measurements_files = kmalloc_array(len, > > + sizeof(struct dentry *), GFP_KERNEL); > > + if(!ima_binary_measurements_files) > > + return -ENOMEM; > > + > > + for (i = 0; i < len; i++) { > > + algo = ima_tpm_chip ? ima_tpm_chip->allocated_banks[i].crypto_id : > > + HASH_ALGO_SHA1; > > I'm starting to think that maybe we should export ima_algo_array > instead, and follow that. > > > + > > + sprintf(file_name, "ascii_runtime_measurements_%s", > > + hash_algo_name[algo]); > > + dfile = securityfs_create_file(file_name, > > + S_IRUSR | S_IRGRP, ima_dir, NULL, > > + &ima_ascii_measurements_ops); > > + if (IS_ERR(dfile)) > > + return PTR_ERR(dfile); > > + > > + ima_ascii_measurements_files[i] = dfile; > > + > > + sprintf(file_name, "binary_runtime_measurements_%s", > > + hash_algo_name[algo]); > > + dfile = securityfs_create_file(file_name, > > + S_IRUSR | S_IRGRP, ima_dir, NULL, > > + &ima_measurements_ops); > > + if (IS_ERR(dfile)) > > + return PTR_ERR(dfile); > > + > > + ima_binary_measurements_files[i] = dfile; > > + } > > + > > + return 0; > > +} > > + > > /* > > * ima_open_policy: sequentialize access to the policy file > > */ > > @@ -465,19 +578,21 @@ int __init ima_fs_init(void) > > goto out; > > } > > > > - binary_runtime_measurements = > > - securityfs_create_file("binary_runtime_measurements", > > - S_IRUSR | S_IRGRP, ima_dir, NULL, > > - &ima_measurements_ops); > > + ret = create_measurements_list_files(); > > + if (ret != 0) > > + goto out; > > + > > + binary_runtime_measurements = securityfs_create_symlink( > > + "binary_runtime_measurements", ima_dir, > > + "binary_runtime_measurements_sha1", NULL); > > if (IS_ERR(binary_runtime_measurements)) { > > ret = PTR_ERR(binary_runtime_measurements); > > goto out; > > } > > > > - ascii_runtime_measurements = > > - securityfs_create_file("ascii_runtime_measurements", > > - S_IRUSR | S_IRGRP, ima_dir, NULL, > > - &ima_ascii_measurements_ops); > > + ascii_runtime_measurements = securityfs_create_symlink( > > + "ascii_runtime_measurements", ima_dir, > > + "ascii_runtime_measurements_sha1", NULL); > > if (IS_ERR(ascii_runtime_measurements)) { > > ret = PTR_ERR(ascii_runtime_measurements); > > goto out; > > @@ -515,6 +630,8 @@ int __init ima_fs_init(void) > > securityfs_remove(runtime_measurements_count); > > securityfs_remove(ascii_runtime_measurements); > > securityfs_remove(binary_runtime_measurements); > > + remove_measurements_list_files(ima_ascii_measurements_files); > > + remove_measurements_list_files(ima_binary_measurements_files); > > The rest is ok. > > Thanks > > Roberto > > > securityfs_remove(ima_symlink); > > securityfs_remove(ima_dir); > > > > base-commit: 88035e5694a86a7167d490bb95e9df97a9bb162b
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index cd1683dad3bf..fb65ba9426a1 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -116,9 +116,12 @@ void ima_putc(struct seq_file *m, void *data, int datalen) seq_putc(m, *(char *)data++); } +static struct dentry **ima_ascii_measurements_files; +static struct dentry **ima_binary_measurements_files; + /* print format: * 32bit-le=pcr# - * char[20]=template digest + * char[n]=template digest * 32bit-le=template name size * char[n]=template name * [eventdata length] @@ -130,9 +133,25 @@ int ima_measurements_show(struct seq_file *m, void *v) struct ima_queue_entry *qe = v; struct ima_template_entry *e; char *template_name; + struct dentry *dentry; u32 pcr, namelen, template_data_len; /* temporary fields */ bool is_ima_template = false; - int i; + int i, algo_idx; + enum hash_algo algo; + + dentry = m->file->f_path.dentry; + algo_idx = ima_sha1_idx; + algo = HASH_ALGO_SHA1; + + if (ima_tpm_chip) { + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { + if (dentry == ima_binary_measurements_files[i]) { + algo_idx = i; + algo = ima_tpm_chip->allocated_banks[i].crypto_id; + break; + } + } + } /* get entry */ e = qe->entry; @@ -151,7 +170,7 @@ int ima_measurements_show(struct seq_file *m, void *v) ima_putc(m, &pcr, sizeof(e->pcr)); /* 2nd: template digest */ - ima_putc(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); + ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]); /* 3rd: template name size */ namelen = !ima_canonical_fmt ? strlen(template_name) : @@ -220,7 +239,23 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) struct ima_queue_entry *qe = v; struct ima_template_entry *e; char *template_name; - int i; + struct dentry *dentry; + int i, algo_idx; + enum hash_algo algo; + + dentry = m->file->f_path.dentry; + algo_idx = ima_sha1_idx; + algo = HASH_ALGO_SHA1; + + if (ima_tpm_chip) { + for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) { + if (dentry == ima_ascii_measurements_files[i]) { + algo_idx = i; + algo = ima_tpm_chip->allocated_banks[i].crypto_id; + break; + } + } + } /* get entry */ e = qe->entry; @@ -233,8 +268,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v) /* 1st: PCR used (config option) */ seq_printf(m, "%2d ", e->pcr); - /* 2nd: SHA1 template hash */ - ima_print_digest(m, e->digests[ima_sha1_idx].digest, TPM_DIGEST_SIZE); + /* 2nd: template hash */ + ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]); /* 3th: template name */ seq_printf(m, " %s", template_name); @@ -379,6 +414,84 @@ static const struct seq_operations ima_policy_seqops = { }; #endif +/* + * Remove the securityfs files created for each hash algo configured + * in the TPM, excluded ascii_runtime_measurements and + * binary_runtime_measurements. + */ +static void remove_measurements_list_files(struct dentry **files) +{ + int i, len; + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; + + if (files) + { + for (i = 0; i < len; i++) { + if (files[i]) { + securityfs_remove(files[i]); + } + } + + kfree(files); + } +} + +/* + * Allocate a file in the securityfs for each hash algo configured + * in the TPM (for ascii and binary output). In case no TPM chip is + * detected only sha1 files are created. + */ +static int create_measurements_list_files(void) +{ + int i, len; + u16 algo; + char file_name[NAME_MAX+1]; + struct dentry *dfile; + + /* + * Allocate NR_BANKS(ima_tpm_chip) files in case a tpm chip is detected, + * otherwise allocate just one file for sha1. + */ + len = ima_tpm_chip ? NR_BANKS(ima_tpm_chip) : 1; + + ima_ascii_measurements_files = kmalloc_array(len, + sizeof(struct dentry *), GFP_KERNEL); + if(!ima_ascii_measurements_files) + return -ENOMEM; + + ima_binary_measurements_files = kmalloc_array(len, + sizeof(struct dentry *), GFP_KERNEL); + if(!ima_binary_measurements_files) + return -ENOMEM; + + for (i = 0; i < len; i++) { + algo = ima_tpm_chip ? ima_tpm_chip->allocated_banks[i].crypto_id : + HASH_ALGO_SHA1; + + sprintf(file_name, "ascii_runtime_measurements_%s", + hash_algo_name[algo]); + dfile = securityfs_create_file(file_name, + S_IRUSR | S_IRGRP, ima_dir, NULL, + &ima_ascii_measurements_ops); + if (IS_ERR(dfile)) + return PTR_ERR(dfile); + + ima_ascii_measurements_files[i] = dfile; + + sprintf(file_name, "binary_runtime_measurements_%s", + hash_algo_name[algo]); + dfile = securityfs_create_file(file_name, + S_IRUSR | S_IRGRP, ima_dir, NULL, + &ima_measurements_ops); + if (IS_ERR(dfile)) + return PTR_ERR(dfile); + + ima_binary_measurements_files[i] = dfile; + } + + return 0; +} + /* * ima_open_policy: sequentialize access to the policy file */ @@ -465,19 +578,21 @@ int __init ima_fs_init(void) goto out; } - binary_runtime_measurements = - securityfs_create_file("binary_runtime_measurements", - S_IRUSR | S_IRGRP, ima_dir, NULL, - &ima_measurements_ops); + ret = create_measurements_list_files(); + if (ret != 0) + goto out; + + binary_runtime_measurements = securityfs_create_symlink( + "binary_runtime_measurements", ima_dir, + "binary_runtime_measurements_sha1", NULL); if (IS_ERR(binary_runtime_measurements)) { ret = PTR_ERR(binary_runtime_measurements); goto out; } - ascii_runtime_measurements = - securityfs_create_file("ascii_runtime_measurements", - S_IRUSR | S_IRGRP, ima_dir, NULL, - &ima_ascii_measurements_ops); + ascii_runtime_measurements = securityfs_create_symlink( + "ascii_runtime_measurements", ima_dir, + "ascii_runtime_measurements_sha1", NULL); if (IS_ERR(ascii_runtime_measurements)) { ret = PTR_ERR(ascii_runtime_measurements); goto out; @@ -515,6 +630,8 @@ int __init ima_fs_init(void) securityfs_remove(runtime_measurements_count); securityfs_remove(ascii_runtime_measurements); securityfs_remove(binary_runtime_measurements); + remove_measurements_list_files(ima_ascii_measurements_files); + remove_measurements_list_files(ima_binary_measurements_files); securityfs_remove(ima_symlink); securityfs_remove(ima_dir);
The template hash showed by the ascii_runtime_measurements and binary_runtime_measurements is the one calculated using sha1 and there is no possibility to change this value, despite the fact that the template hash is calculated using the hash algorithms corresponding to all the PCR banks configured in the TPM. Add the support to retrieve the ima log with the template data hash calculated with a specific hash algorithm. Add a new file in the securityfs ima directory for each hash algo configured for the PCR banks of the TPM. Each new file has the name with the following structure: {binary, ascii}_runtime_measurements_<hash_algo_name> The <hash_algo_name> is used to select the template data hash algorithm to show in ima_ascii_measurements_show() and in ima_measurements_show(). Legacy files are kept but as sysmbolic links which point to {binary, ascii}_runtime_measurements_sha1 files. These two files are created even if a TPM chip is not detected. As example, in the case a TPM chip is present and sha1 and sha256 are the configured PCR banks, the listing of the security/ima directory is the following: lr--r--r-- [...] ascii_runtime_measurements -> ascii_runtime_measurements_sha1 -r--r----- [...] ascii_runtime_measurements_sha1 -r--r----- [...] ascii_runtime_measurements_sha256 lr--r--r-- [...] binary_runtime_measurements -> binary_runtime_measurements_sha1 -r--r----- [...] binary_runtime_measurements_sha1 -r--r----- [...] binary_runtime_measurements_sha256 --w------- [...] policy -r--r----- [...] runtime_measurements_count -r--r----- [...] violations Signed-off-by: Enrico Bravi <enrico.bravi@polito.it> Signed-off-by: Silvia Sisinni <silvia.sisinni@polito.it> --- v3: - Added create_measurements_list_files function for measurements files creation. - Parametrized the remove_measurements_list_files function and add NULL check before freeing files' list. - Removed algorithm selection based on file name during ima_measurements_show and ima_ascii_mesurements_show, and selecting it comparing dentry address. - Allocate also sha1 file following the schema {binary, ascii}_runtime_measurements_<hash_algo_name> and keep legacy files as symbolic links to those files. - Allocate measurements files lists even if a TPM chip is not detected, adding only sha1 files. v2: - Changed the behaviour of configuring at boot time the template data hash algorithm. - Removed template data hash algo name prefix. - Removed ima_template_hash command line option. - Introducing a new file in the securityfs ima subdir for each PCR banks algorithm configured in the TPM. (suggested by Roberto) security/integrity/ima/ima_fs.c | 145 +++++++++++++++++++++++++++++--- 1 file changed, 131 insertions(+), 14 deletions(-) base-commit: 88035e5694a86a7167d490bb95e9df97a9bb162b