The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot.
The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.
This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set* for actually carrying the
serialized measurement list across the kexec.
Mimi
*https://lists.infradead.org/pipermail/kexec/2016-June/016157.html
Mimi Zohar (6):
ima: on soft reboot, restore the measurement list
ima: permit duplicate measurement list entries
ima: maintain memory size needed for serializing the measurement list
ima: serialize the binary_runtime_measurements
ima: store the builtin/custom template definitions in a list
ima: support restoring multiple template formats
Thiago Jung Bauermann (1):
ima: on soft reboot, save the measurement list
include/linux/ima.h | 15 ++
kernel/kexec_file.c | 3 +
security/integrity/ima/Kconfig | 12 ++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 14 ++
security/integrity/ima/ima_fs.c | 2 +-
security/integrity/ima/ima_init.c | 2 +
security/integrity/ima/ima_kexec.c | 189 ++++++++++++++++++++++++
security/integrity/ima/ima_main.c | 1 +
security/integrity/ima/ima_queue.c | 72 +++++++++-
security/integrity/ima/ima_template.c | 262 ++++++++++++++++++++++++++++++++--
11 files changed, 556 insertions(+), 17 deletions(-)
create mode 100644 security/integrity/ima/ima_kexec.c
--
2.1.0
@@ -202,6 +202,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,returnret;image->kernel_buf_len=size;+/* IMA needs to pass the measurement list to the next kernel. */+ima_add_kexec_buffer(image);+/* Call arch image probe handlers */ret=arch_kexec_kernel_image_probe(image,image->kernel_buf,image->kernel_buf_len);
@@ -23,6 +23,11 @@#include"ima.h"+#ifdef CONFIG_IMA_KEXEC+/* Physical address of the measurement buffer in the next kernel. */+staticunsignedlongkexec_buffer_load_addr;+staticsize_tkexec_segment_size;+staticintima_dump_measurement_list(unsignedlong*buffer_size,void**buffer,unsignedlongsegment_size){
@@ -75,6 +80,84 @@ out:}/*+*CalledduringkexecexecutesothatIMAcansavethemeasurementlist.+*/+staticintima_update_kexec_buffer(structnotifier_block*self,+unsignedlongaction,void*data)+{+void*kexec_buffer=NULL;+size_tkexec_buffer_size;+intret;++if(!kexec_in_progress)+returnNOTIFY_OK;++kexec_buffer_size=ima_get_binary_runtime_size();+if(kexec_buffer_size>+(kexec_segment_size-sizeof(structima_kexec_hdr))){+pr_err("Binary measurement list grew too large.\n");+gotoout;+}++ima_dump_measurement_list(&kexec_buffer_size,&kexec_buffer,+kexec_segment_size);+if(!kexec_buffer){+pr_err("Not enough memory for the kexec measurement buffer.\n");+gotoout;+}+ret=kexec_update_segment(kexec_buffer,kexec_buffer_size,+kexec_buffer_load_addr,kexec_segment_size);+if(ret)+pr_err("Error updating kexec buffer: %d\n",ret);+out:+returnNOTIFY_OK;+}++structnotifier_blockupdate_buffer_nb={+.notifier_call=ima_update_kexec_buffer,+};++/*+*Calledduringkexec_file_loadsothatIMAcanaddasegmenttothekexec+*imageforthemeasurementlistforthenextkernel.+*/+voidima_add_kexec_buffer(structkimage*image)+{+structkexec_bufkbuf={.image=image,.buf_align=PAGE_SIZE,+.buf_min=0,.buf_max=ULONG_MAX,+.top_down=true};+intret;++if(!kexec_can_hand_over_buffer())+return;++kexec_segment_size=ALIGN(ima_get_binary_runtime_size()+PAGE_SIZE,+PAGE_SIZE);++if(kexec_segment_size>=(ULONG_MAX-sizeof(long))){+pr_err("Binary measurement list too large.\n");+return;+}++/* Ask not to checksum the segment, we will update it later. */+kbuf.buffer=NULL;+kbuf.bufsz=0;+kbuf.memsz=kexec_segment_size;+ret=kexec_add_handover_buffer(&kbuf,false);+if(ret){+pr_err("Error passing over kexec measurement buffer.\n");+return;+}+kexec_buffer_load_addr=kbuf.mem;++register_reboot_notifier(&update_buffer_nb);++pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",+kexec_buffer_load_addr);+}+#endif /* IMA_KEXEC */++/**Restorethemeasurementlistfromthepreviouskernel.*/voidima_load_kexec_buffer(void)
Measurements carried across kexec need to be added to the IMA
measurement list, but should not prevent measurements of the newly
booted kernel from being added to the measurement list. This patch
adds support for allowing duplicate measurements.
The "boot_aggregate" measurement entry is the delimiter between soft
boots.
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/ima_queue.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
@@ -85,8 +86,10 @@ static int ima_add_digest_entry(struct ima_template_entry *entry)list_add_tail_rcu(&qe->later,&ima_measurements);atomic_long_inc(&ima_htable.len);-key=ima_hash_key(entry->digest);-hlist_add_head_rcu(&qe->hnext,&ima_htable.queue[key]);+if(flags){+key=ima_hash_key(entry->digest);+hlist_add_head_rcu(&qe->hnext,&ima_htable.queue[key]);+}return0;}
@@ -126,7 +129,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,}}-result=ima_add_digest_entry(entry);+result=ima_add_digest_entry(entry,1);if(result<0){audit_cause="ENOMEM";audit_info=0;
@@ -155,7 +158,7 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)intresult=0;mutex_lock(&ima_extend_list_mutex);-result=ima_add_digest_entry(entry);+result=ima_add_digest_entry(entry,0);mutex_unlock(&ima_extend_list_mutex);returnresult;}
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot. This patch
restores the measurement list.
Changelog:
- call ima_load_kexec_buffer() (Thiago)
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 10 ++
security/integrity/ima/ima_init.c | 2 +
security/integrity/ima/ima_kexec.c | 55 +++++++++++
security/integrity/ima/ima_queue.c | 10 ++
security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++
6 files changed, 249 insertions(+)
create mode 100644 security/integrity/ima/ima_kexec.c
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;+/* Internal IMA function definitions */intima_init(void);intima_fs_init(void);
@@ -122,6 +129,9 @@ int ima_init_crypto(void);voidima_putc(structseq_file*m,void*data,intdatalen);voidima_print_digest(structseq_file*m,u8*digest,u32size);structima_template_desc*ima_template_desc_current(void);+voidima_load_kexec_buffer(void);+intima_restore_measurement_entry(structima_template_entry*entry);+intima_restore_measurement_list(loff_tbufsize,void*buf);intima_init_template(void);/*
@@ -128,6 +128,8 @@ int __init ima_init(void)if(rc!=0)returnrc;+ima_load_kexec_buffer();+rc=ima_add_boot_aggregate();/* boot aggregate must be first entry */if(rc!=0)returnrc;
@@ -0,0 +1,55 @@+/*+*Copyright(C)2016IBMCorporation+*+*Authors:+*ThiagoJungBauermann<bauerman@linux.vnet.ibm.com>+*MimiZohar<zohar@linux.vnet.ibm.com>+*+*Thisprogramisfreesoftware;youcanredistributeitand/ormodify+*itunderthetermsoftheGNUGeneralPublicLicenseaspublishedby+*theFreeSoftwareFoundation;eitherversion2oftheLicense,or+*(atyouroption)anylaterversion.+*/+#include<linux/fcntl.h>+#include<linux/slab.h>+#include<linux/module.h>+#include<linux/seq_file.h>+#include<linux/rculist.h>+#include<linux/rcupdate.h>+#include<linux/parser.h>+#include<linux/vmalloc.h>+#include<linux/kexec.h>+#include<linux/reboot.h>++#include"ima.h"++/*+*Restorethemeasurementlistfromthepreviouskernel.+*/+voidima_load_kexec_buffer(void)+{+void*kexec_buffer=NULL;+size_tkexec_buffer_size=0;+intrc;++rc=kexec_get_handover_buffer(&kexec_buffer,&kexec_buffer_size);+switch(rc){+case0:+rc=ima_restore_measurement_list(kexec_buffer_size,+kexec_buffer);+if(rc!=0)+pr_err("Failed to restore the measurement list: %d\n",+rc);++kexec_free_handover_buffer();+break;+case-ENOTSUPP:+pr_debug("Restoring the measurement list not supported\n");+break;+case-ENOENT:+pr_debug("No measurement list to restore\n");+break;+default:+pr_debug("Error restoring the measurement list: %d\n",rc);+}+}
@@ -205,3 +205,174 @@ int __init ima_init_template(void)returnresult;}++staticintima_restore_template_data(structima_template_desc*template_desc,+void*template_data,+inttemplate_data_size,+structima_template_entry**entry)+{+structbinary_field_data{+u32len;+u8data[0];+}__packed;++structbinary_field_data*field_data;+intoffset=0;+intret=0;+inti;++*entry=kzalloc(sizeof(**entry)++template_desc->num_fields*sizeof(structima_field_data),+GFP_NOFS);+if(!*entry)+return-ENOMEM;++(*entry)->template_desc=template_desc;+for(i=0;i<template_desc->num_fields;i++){+field_data=template_data+offset;++/* Each field of the template data is prefixed with a length. */+if(offset>(template_data_size-sizeof(field_data->len))){+pr_err("Restoring the template field failed\n");+ret=-EINVAL;+break;+}+offset+=sizeof(field_data->len);++if(offset>(template_data_size-field_data->len)){+pr_err("Restoring the template field data failed\n");+ret=-EINVAL;+break;+}+offset+=field_data->len;++(*entry)->template_data[i].len=field_data->len;+(*entry)->template_data_len+=sizeof(field_data->len);++(*entry)->template_data[i].data=+kzalloc(field_data->len+1,GFP_KERNEL);+if(!(*entry)->template_data[i].data){+ret=-ENOMEM;+break;+}+memcpy((*entry)->template_data[i].data,field_data->data,+field_data->len);+(*entry)->template_data_len+=field_data->len;+}++if(ret<0){+ima_free_template_entry(*entry);+*entry=NULL;+}++returnret;+}++#define MAX_TEMPLATE_NAME_LEN 30++/* Restore the serialized binary measurement list without extending PCRs. */+intima_restore_measurement_list(loff_tsize,void*buf)+{+structbinary_hdr_v1{+u32pcr;+u8digest[TPM_DIGEST_SIZE];+u32template_name_len;+chartemplate_name[0];+}__packed;+chartemplate_name[MAX_TEMPLATE_NAME_LEN];++structbinary_data_v1{+u32template_data_size;+chartemplate_data[0];+}__packed;++structima_kexec_hdr*khdr=buf;+structbinary_hdr_v1*hdr_v1;+structbinary_data_v1*data_v1;++void*bufp=buf+sizeof(*khdr);+void*bufendp=buf+khdr->buffer_size;+structima_template_entry*entry;+structima_template_desc*template_desc;+unsignedlongcount=0;+intret=0;++if(!buf||size<sizeof(*khdr))+return0;++if(khdr->version!=1){+pr_err("attempting to restore a incompatible measurement list");+return0;+}++/*+*imakexecbufferprefix:version,buffersize,count+*v1format:pcr,digest,template-name-len,template-name,+*template-data-size,template-data+*/+while((bufp<bufendp)&&(count++<khdr->count)){+if(count>ULONG_MAX-1){+pr_err("attempting to restore too many measurements");+ret=-EINVAL;+}++hdr_v1=bufp;+if((hdr_v1->template_name_len>MAX_TEMPLATE_NAME_LEN)||+((bufp+hdr_v1->template_name_len)>bufendp)){+pr_err("attempting to restore a template name \+thatistoolong\n");+ret=-EINVAL;+break;+}+bufp+=sizeof(*hdr_v1);++/* template name is not null terminated */+memcpy(template_name,bufp,hdr_v1->template_name_len);+template_name[hdr_v1->template_name_len]=0;++if(strcmp(template_name,"ima")==0){+pr_err("attempting to restore an unsupported \+template\"%s\" failed\n",template_name);+ret=-EINVAL;+break;+}+data_v1=bufp+=(u_int8_t)hdr_v1->template_name_len;++/* get template format */+template_desc=lookup_template_desc(template_name);+if(!template_desc){+pr_err("template \"%s\" not found\n",template_name);+ret=-EINVAL;+break;+}++if(bufp>(bufendp-sizeof(data_v1->template_data_size))){+pr_err("restoring the template data size failed\n");+ret=-EINVAL;+break;+}+bufp+=(u_int8_t)sizeof(data_v1->template_data_size);++if(bufp>(bufendp-data_v1->template_data_size)){+pr_err("restoring the template data failed\n");+ret=-EINVAL;+break;+}++ret=ima_restore_template_data(template_desc,+data_v1->template_data,+data_v1->template_data_size,+&entry);+if(ret<0)+break;++memcpy(entry->digest,hdr_v1->digest,TPM_DIGEST_SIZE);+entry->pcr=hdr_v1->pcr;+ret=ima_restore_measurement_entry(entry);+if(ret<0)+break;++bufp+=data_v1->template_data_size;+}+returnret;+}
@@ -27,6 +27,18 @@ config IMAtolearnmoreaboutIMA.Ifunsure,sayN.+configIMA_KEXEC+bool"Enable carrying the IMA measurement list across a soft boot"+depends onIMA&&TCG_TPM&&KEXEC_FILE+defaultn+help+TPMPCRsareonlyresetonahardreboot.Inordertovalidate+aTPM'squoteafterasoftboot,theIMAmeasurementlistofthe+runningkernelmustbesavedandrestoredonboot.++DependingontheIMApolicy,themeasurementlistcangrowto+beverylarge.+configIMA_MEASURE_PCR_IDXintdepends onIMA
@@ -29,6 +29,11 @@#define AUDIT_CAUSE_LEN_MAX 32LIST_HEAD(ima_measurements);/* list of all measurements */+#ifdef CONFIG_IMA_KEXEC+staticunsignedlongbinary_runtime_size;+#else+staticunsignedlongbinary_runtime_size=ULONG_MAX;+#endif/* key: inode (before secure-hashing a file) */structima_h_tableima_htable={
@@ -64,6 +69,24 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,returnret;}+/*+*Calculatethememoryrequiredforserializingasingle+*binary_runtime_measurementlistentry,whichcontainsa+*coupleofvariablelengthfields(e.gtemplatenameanddata).+*/+staticintget_binary_runtime_size(structima_template_entry*entry)+{+intsize=0;++size+=sizeof(u32);/* pcr */+size+=sizeof(entry->digest);+size+=sizeof(int);/* template name size field */+size+=strlen(entry->template_desc->name);+size+=sizeof(entry->template_data_len);+size+=entry->template_data_len;+returnsize;+}+/* ima_add_template_entry helper function:*-Addtemplateentrytothemeasurementlistandhashtable,for*allentriesexceptthosecarriedacrosskexec.
@@ -90,9 +113,26 @@ static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)key=ima_hash_key(entry->digest);hlist_add_head_rcu(&qe->hnext,&ima_htable.queue[key]);}++if(binary_runtime_size!=ULONG_MAX){+intsize;++size=get_binary_runtime_size(entry);+binary_runtime_size=(binary_runtime_size<ULONG_MAX-size)?+binary_runtime_size+size:ULONG_MAX;+}return0;}+/*+*Returntheamountofmemoryrequiredforserializingthe+*entirebinary_runtime_measurementlist.+*/+unsignedlongima_get_binary_runtime_size(void)+{+returnbinary_runtime_size;+};+staticintima_pcr_extend(constu8*hash,intpcr){intresult=0;
@@ -106,8 +146,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)returnresult;}-/* Add template entry to the measurement list and hash table,-*andextendthepcr.+/*+*Addtemplateentrytothemeasurementlistandhashtable,and+*extendthepcr.+*+*OnsystemswhichsupportcarryingtheIMAmeasurementlistacross+*kexec,maintainthetotalmemorysizerequiredforserializingthe+*binary_runtime_measurements.*/intima_add_template_entry(structima_template_entry*entry,intviolation,constchar*op,structinode*inode,
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot. This patch
serializes the IMA measurement list in the binary_runtime_measurements
format.
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_fs.c | 2 +-
security/integrity/ima/ima_kexec.c | 51 ++++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+), 1 deletion(-)
@@ -116,7 +116,7 @@ void ima_putc(struct seq_file *m, void *data, int datalen)*[eventdatalength]*eventdata[n]=templatespecificdata*/-staticintima_measurements_show(structseq_file*m,void*v)+intima_measurements_show(structseq_file*m,void*v){/* the list never shrinks, so we don't need a lock here */structima_queue_entry*qe=v;
The builtin and single custom templates are currently stored in an
array. In preparation for being able to restore a measurement list
containing multiple builtin/custom templates, this patch stores the
builtin and custom templates as a linked list. This will permit
defining more than one custom template per boot.
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 1 +
security/integrity/ima/ima_template.c | 37 +++++++++++++++++++++++++++--------
3 files changed, 32 insertions(+), 8 deletions(-)
@@ -15,16 +15,20 @@#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt+#include<linux/rculist.h>#include"ima.h"#include"ima_template_lib.h"-staticstructima_template_descdefined_templates[]={+staticstructima_template_descbuiltin_templates[]={{.name=IMA_TEMPLATE_IMA_NAME,.fmt=IMA_TEMPLATE_IMA_FMT},{.name="ima-ng",.fmt="d-ng|n-ng"},{.name="ima-sig",.fmt="d-ng|n-ng|sig"},{.name="",.fmt=""},/* placeholder for a custom format */};+staticLIST_HEAD(defined_templates);+spinlock_ttemplate_list;+staticstructima_template_fieldsupported_fields[]={{.field_id="d",.field_init=ima_eventdigest_init,.field_show=ima_show_template_digest},
The configured IMA measurement list template format can be replaced at
runtime on the boot command line, including a custom template format.
This patch adds support for restoring a measuremement list containing
multiple builtin/custom template formats.
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/ima_template.c | 58 +++++++++++++++++++++++++++++++++--
1 file changed, 55 insertions(+), 3 deletions(-)
@@ -227,6 +237,35 @@ int __init ima_init_template(void)returnresult;}+staticstructima_template_desc*restore_template_fmt(char*template_name)+{+structima_template_desc*template_desc=NULL;+intret;++ret=template_desc_init_fields(template_name,NULL,NULL);+if(ret<0){+pr_err("attempting to initialize the template \"%s\" failed\n",+template_name);+gotoout;+}++template_desc=kzalloc(sizeof(*template_desc),GFP_KERNEL);+if(!template_desc)+gotoout;++template_desc->name="";+template_desc->fmt=kstrdup(template_name,GFP_KERNEL);+if(!template_desc->fmt)+gotoout;++spin_lock(&template_list);+list_add_tail_rcu(&template_desc->list,&defined_templates);+spin_unlock(&template_list);+synchronize_rcu();+out:+returntemplate_desc;+}+staticintima_restore_template_data(structima_template_desc*template_desc,void*template_data,inttemplate_data_size,
@@ -359,10 +398,23 @@ int ima_restore_measurement_list(loff_t size, void *buf)}data_v1=bufp+=(u_int8_t)hdr_v1->template_name_len;-/* get template format */template_desc=lookup_template_desc(template_name);if(!template_desc){-pr_err("template \"%s\" not found\n",template_name);+template_desc=restore_template_fmt(template_name);+if(!template_desc)+break;+}++/*+*Onlytherunningsystem'stemplateformatisinitialized+*onboot.Asneeded,initializetheothertemplateformats.+*/+ret=template_desc_init_fields(template_desc->fmt,+&(template_desc->fields),+&(template_desc->num_fields));+if(ret<0){+pr_err("attempting to restore the template fmt \"%s\" \+failed\n", template_desc->fmt);ret=-EINVAL;break;}
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot. This patch
restores the measurement list.
Changelog:
- call ima_load_kexec_buffer() (Thiago)
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 10 ++
security/integrity/ima/ima_init.c | 2 +
security/integrity/ima/ima_kexec.c | 55 +++++++++++
security/integrity/ima/ima_queue.c | 10 ++
security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++
6 files changed, 249 insertions(+)
create mode 100644 security/integrity/ima/ima_kexec.c
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;
Unless there is no real need for this structure to be packed i suggest dropping
the attribute. When referenced through pointer 32bit ARM and MIPS (and likely
all other 32bit RISC CPUs) use rather inefficient byte loads and stores.
Worse, if, for example, ->count is going to be read/written concurrently from
multiple threads we get torn loads/stores thus losing atomicity of the access.
Petko
Hi Petko,
Thank you for review!
On Fri, 2016-08-05 at 11:44 +0300, Petko Manolov wrote:
On 16-08-04 08:24:29, Mimi Zohar wrote:
quoted
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot. This patch
restores the measurement list.
Changelog:
- call ima_load_kexec_buffer() (Thiago)
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 10 ++
security/integrity/ima/ima_init.c | 2 +
security/integrity/ima/ima_kexec.c | 55 +++++++++++
security/integrity/ima/ima_queue.c | 10 ++
security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++
6 files changed, 249 insertions(+)
create mode 100644 security/integrity/ima/ima_kexec.c
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;
Unless there is no real need for this structure to be packed i suggest dropping
the attribute. When referenced through pointer 32bit ARM and MIPS (and likely
all other 32bit RISC CPUs) use rather inefficient byte loads and stores.
Worse, if, for example, ->count is going to be read/written concurrently from
multiple threads we get torn loads/stores thus losing atomicity of the access.
This header is used to prefix the serialized binary measurement list
with some meta-data about the measurement list being restored.
Unfortunately kexec_get_handover_buffer() returns the segment size, not
the actual ima measurement list buffer size. The header info is set
using memcpy() once in ima_dump_measurement_list() and then the fields
are used in ima_restore_measurement_list() to verify the buffer.
The binary runtime measurement list is packed, so the other two
structures - binary_hdr_v1 and binary_data_v1 - must be packed. Does it
make sense for this header not to be packed as well? Would copying the
header fields to local variables before being used solve your concern?
Remember this code is used once on the kexec execute and again on
reboot.
Mimi
Hi Petko,
Thank you for review!
On Fri, 2016-08-05 at 11:44 +0300, Petko Manolov wrote:
quoted
On 16-08-04 08:24:29, Mimi Zohar wrote:
quoted
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot. This patch
restores the measurement list.
Changelog:
- call ima_load_kexec_buffer() (Thiago)
Signed-off-by: Mimi Zohar <redacted>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 10 ++
security/integrity/ima/ima_init.c | 2 +
security/integrity/ima/ima_kexec.c | 55 +++++++++++
security/integrity/ima/ima_queue.c | 10 ++
security/integrity/ima/ima_template.c | 171 ++++++++++++++++++++++++++++++++++
6 files changed, 249 insertions(+)
create mode 100644 security/integrity/ima/ima_kexec.c
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;
Unless there is no real need for this structure to be packed i suggest
dropping the attribute. When referenced through pointer 32bit ARM and MIPS
(and likely all other 32bit RISC CPUs) use rather inefficient byte loads and
stores.
Worse, if, for example, ->count is going to be read/written concurrently
from multiple threads we get torn loads/stores thus losing atomicity of the
access.
This header is used to prefix the serialized binary measurement list with some
meta-data about the measurement list being restored. Unfortunately
kexec_get_handover_buffer() returns the segment size, not the actual ima
measurement list buffer size. The header info is set using memcpy() once in
ima_dump_measurement_list() and then the fields are used in
ima_restore_measurement_list() to verify the buffer.
As long as there is no concurrent reads/writes this should be OK.
The binary runtime measurement list is packed, so the other two structures -
binary_hdr_v1 and binary_data_v1 - must be packed. Does it make sense for
this header not to be packed as well? Would copying the header fields to
local variables before being used solve your concern?
Copying to aligned variables would be necessary only if:
a) some sort of atomicity is needed, and/or
б) speed is of concern;
Remember this code is used once on the kexec execute and again on reboot.
If we don't need a) _and_ b) then you don't need to bother.
Petko
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot.
The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.
This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set* for actually carrying the
serialized measurement list across the kexec.
Mimi
Hi, Mimi
I am trying to convince myself of the security of the solution. I asked
Thiago as well, but may be I am be lagging behind in understanding.
We trust the kernel to hand over PCR values of the old kernel (which
cannot be validated) to the IMA subsystem in the new kernel for storage.
I guess the idea is for ima_add_boot_aggregate to do the right thing?
How do we validate what the old kernel is giving us? Why do we care for
the old measurement list? Is it still of significance in the new kernel?
Balbir Singh.
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;+
Am I understanding it correctly that this structure is passed between kernels?
If so it's an ABI and should use types with well defined sizes, as if it was
going out to userspace, shouldn't it?
cheers
On Tue, 2016-08-09 at 15:19 +1000, Balbir Singh wrote:
On 04/08/16 22:24, Mimi Zohar wrote:
quoted
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot.
The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.
This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set* for actually carrying the
serialized measurement list across the kexec.
Mimi
Hi, Mimi
I am trying to convince myself of the security of the solution. I asked
Thiago as well, but may be I am be lagging behind in understanding.
We trust the kernel to hand over PCR values of the old kernel (which
cannot be validated) to the IMA subsystem in the new kernel for storage.
I guess the idea is for ima_add_boot_aggregate to do the right thing?
How do we validate what the old kernel is giving us? Why do we care for
the old measurement list? Is it still of significance in the new kernel?
Hi Balbir,
To validate the hardware TPM PCR values requires walking the measurement
list simulating the TPM extend operation. The resulting values should
match the hardware TPM PCRs.
In the case of a soft reboot, the TPM PCRs are not reset to 0, so all
the measurements of the running system, including those from previous
soft reboots, need to be included in the measurement list. Without
these measurements, the simulated PCR values will not match the hardware
TPM PCR values. Thus the need for this patch set.
Measurements can not be added/removed/changed in the measurement list
without it being detectable.
Mimi
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all measurements */+/* Some details preceding the binary serialized measurement list */+structima_kexec_hdr{+unsignedshortversion;+unsignedlongbuffer_size;+unsignedlongcount;+}__packed;+
Am I understanding it correctly that this structure is passed between kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
If so it's an ABI and should use types with well defined sizes, as if it was
going out to userspace, shouldn't it?
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all
measurements
quoted
quoted
*/
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
From: David Laight <hidden> Date: 2016-08-09 13:37:43
From: Thiago Jung Bauermann
Sent: 09 August 2016 14:19
...
quoted
quoted
quoted
+/* Some details preceding the binary serialized measurement list *=
/
quoted
quoted
quoted
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
=20
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
64bit kernel to/from 32bit one? (if that makes sense on the hardware).
Whether we want to support that or not is another question...
In which case shouldn't they be annotated with the endianness??
Also why '__packed' - guarantees sub-optimal code generation.
Much better to include explicit padding to align everything.
David
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all
measurements
quoted
quoted
quoted
*/
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
The <securityfs/ima/binary_runtime_measurements is system architecture
dependent. It looks like the khdr->version check in
ima_restore_measurement_list() would fail if the architecture changes.
If/when we update the binary measurement list format to support multiple
TPM PCRs, we should address the endianness as well.
Mimi
On Tue, 2016-08-09 at 13:35 +0000, David Laight wrote:
Also why '__packed' - guarantees sub-optimal code generation.
Much better to include explicit padding to align everything.
This patch set does not define a new format, but piggy backs on top of
the existing <securityfs>/ima/binary_runtime_measurements list. The
prefixed buffer header includes a version, so that if in the future we
need to modify the format, we would be able to.
In terms of the prefixed header, how would you define the fields:
version, buffer size, number of measurements?
Mimi
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all
measurements
quoted
quoted
quoted
*/
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
The <securityfs/ima/binary_runtime_measurements is system architecture
dependent. It looks like the khdr->version check in
ima_restore_measurement_list() would fail if the architecture changes.
If/when we update the binary measurement list format to support multiple
TPM PCRs, we should address the endianness as well.
That should have been "TPM PCR banks". TPM 2.0 allows for multiple TPM
PCR banks.
Mimi
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all
measurements
quoted
quoted
quoted
*/
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
The answer to that question is almost certainly yes - on POWER the most
immediate example would be soft-rebooting from a BE host into an LE
Petitboot kernel, or vice versa.
@@ -102,6 +102,13 @@ struct ima_queue_entry {};externstructlist_headima_measurements;/* list of all
measurements
quoted
quoted
quoted
*/
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
Yes you must support that. BE -> LE and vice versa.
You should also consider the possibility that the next kernel is not
Linux.
cheers
Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
quoted
On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
quoted
Mimi Zohar [off-list ref] writes:
quoted
+/* Some details preceding the binary serialized measurement list
*/
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
Yes you must support that. BE -> LE and vice versa.
I didn't test BE - LE yet, but will do.
You should also consider the possibility that the next kernel is not
Linux.
If the next kernel is an ELF binary and it supports the kexec "calling
convention", it should work too. What could possibly go wrong? I can try
FreeBSD (I suppose it's an ELF kernel) and see what happens.
--
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2016-08-10 18:09:33
Thiago Jung Bauermann [off-list ref] writes:
Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
quoted
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
quoted
On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
quoted
Mimi Zohar [off-list ref] writes:
quoted
+/* Some details preceding the binary serialized measurement list
*/
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
Yes you must support that. BE -> LE and vice versa.
I didn't test BE - LE yet, but will do.
Thanks.
quoted
You should also consider the possibility that the next kernel is not
Linux.
If the next kernel is an ELF binary and it supports the kexec "calling
convention", it should work too. What could possibly go wrong? I can try
FreeBSD (I suppose it's an ELF kernel) and see what happens.
At least for old style kexec (not sys_kexec_load()) I don't think it
even needs to be an ELF binary.
I think there are folks working on FreeBSD (or $?BSD), so I think the
basic kexec part works.
There's nothing (yet) that wants to use this measurement list obviously,
but it should be designed such that it could be used by an unknown
future kernel that knows the ABI.
So given what you have above, you'd use something like:
struct ima_kexec_hdr {
u16 version;
u16 _reserved0;
u32 _reserved1;
u64 buffer_size;
u64 count;
};
cheers
On Wed, 2016-08-10 at 19:52 +1000, Michael Ellerman wrote:
quoted
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
quoted
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
quoted
On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
quoted
Mimi Zohar [off-list ref] writes:
quoted
+/* Some details preceding the binary serialized measurement list
*/
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
Yes you must support that. BE -> LE and vice versa.
I didn't test BE - LE yet, but will do.
Thanks.
Ok. There have been requests for making the binary_runtime_measurements
architecture independent. As this was not a network facing interface,
we left it in native format. With the kernel now consuming this data,
it makes sense for the binary_runtime_measurements to be in an
architecture independent format.
Unfortunately, as the <securityfs>/ima/binary_runtime_measurements is
not prefixed with any metadata, this change would need to be Kconfig
based, but kexec would always use the architecture independent format.
quoted
quoted
quoted
You should also consider the possibility that the next kernel is not
Linux.
Oh!
quoted
quoted
If the next kernel is an ELF binary and it supports the kexec "calling
convention", it should work too. What could possibly go wrong? I can try
FreeBSD (I suppose it's an ELF kernel) and see what happens.
At least for old style kexec (not sys_kexec_load()) I don't think it
even needs to be an ELF binary.
I think there are folks working on FreeBSD (or $?BSD), so I think the
basic kexec part works.
There's nothing (yet) that wants to use this measurement list obviously,
but it should be designed such that it could be used by an unknown
future kernel that knows the ABI.
So given what you have above, you'd use something like:
struct ima_kexec_hdr {
u16 version;
u16 _reserved0;
u32 _reserved1;
u64 buffer_size;
u64 count;
};
cheers
Thanks, I'll make this change.
I would suggest:
struct ima_kexec_hdr {
u64 buffer_size;
u64 count;
u16 version;
};
and let the compiler add the proper padding, depending on the architecture. On
32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) while
retaining the same functionality.
cheers,
Petko
On Wed, 2016-08-10 at 19:52 +1000, Michael Ellerman wrote:
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Mittwoch, 10 August 2016, 13:41:08 schrieb Michael Ellerman:
quoted
Thiago Jung Bauermann [off-list ref] writes:
quoted
Am Dienstag, 09 August 2016, 09:01:13 schrieb Mimi Zohar:
quoted
On Tue, 2016-08-09 at 20:59 +1000, Michael Ellerman wrote:
quoted
Mimi Zohar [off-list ref] writes:
quoted
+/* Some details preceding the binary serialized measurement list
*/
+struct ima_kexec_hdr {
+ unsigned short version;
+ unsigned long buffer_size;
+ unsigned long count;
+} __packed;
+
Am I understanding it correctly that this structure is passed between
kernels?
Yes, the header prefixes the measurement list, which is being passed on
the same computer to the next kernel. Could the architecture (eg.
LE/BE) change between soft re-boots?
Yes. I am able to boot a BE kernel from an LE kernel with my patches.
Whether we want to support that or not is another question...
Yes you must support that. BE -> LE and vice versa.
I didn't test BE - LE yet, but will do.
Thanks.
Ok. There have been requests for making the binary_runtime_measurements
architecture independent. As this was not a network facing interface,
we left it in native format. With the kernel now consuming this data,
it makes sense for the binary_runtime_measurements to be in an
architecture independent format.
Unfortunately, as the <securityfs>/ima/binary_runtime_measurements is
not prefixed with any metadata, this change would need to be Kconfig
based, but kexec would always use the architecture independent format.
quoted
quoted
You should also consider the possibility that the next kernel is not
Linux.
Oh!
quoted
If the next kernel is an ELF binary and it supports the kexec "calling
convention", it should work too. What could possibly go wrong? I can try
FreeBSD (I suppose it's an ELF kernel) and see what happens.
At least for old style kexec (not sys_kexec_load()) I don't think it
even needs to be an ELF binary.
I think there are folks working on FreeBSD (or $?BSD), so I think the
basic kexec part works.
There's nothing (yet) that wants to use this measurement list obviously,
but it should be designed such that it could be used by an unknown
future kernel that knows the ABI.
So given what you have above, you'd use something like:
struct ima_kexec_hdr {
u16 version;
u16 _reserved0;
u32 _reserved1;
u64 buffer_size;
u64 count;
};
cheers
From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=aculab.com@lists.ozlabs.org] On Behalf Of
quoted
quoted
quoted
So given what you have above, you'd use something like:
struct ima_kexec_hdr {
u16 version;
u16 _reserved0;
u32 _reserved1;
u64 buffer_size;
u64 count;
};
cheers
Thanks, I'll make this change.
I would suggest:
struct ima_kexec_hdr {
u64 buffer_size;
u64 count;
u16 version;
};
and let the compiler add the proper padding, depending on the architecture. On
32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) while
retaining the same functionality.
AAAArrrrgggg.....
That doesn't work for 32bit applications on 64bit hosts.
Which part won't work?
The extra bytes will make 0 difference to the allocation cost and lots to the
processing.
From: David Laight <hidden> Date: 2016-08-10 21:14:05
From: Linuxppc-dev [mailto:linuxppc-dev-bounces+david.laight=3Daculab.com@l=
ists.ozlabs.org] On Behalf Of
quoted
quoted
So given what you have above, you'd use something like:
struct ima_kexec_hdr {
u16 version;
u16 _reserved0;
u32 _reserved1;
u64 buffer_size;
u64 count;
};
cheers
Thanks, I'll make this change.
=20
I would suggest:
=20
struct ima_kexec_hdr {
u64 buffer_size;
u64 count;
u16 version;
};
=20
and let the compiler add the proper padding, depending on the architectur=
e. On
32bit machine we'll have 4 bytes smaller allocations (compared to 64bit) =
while
retaining the same functionality.
AAAArrrrgggg.....
That doesn't work for 32bit applications on 64bit hosts.
The extra bytes will make 0 difference to the allocation cost and
lots to the processing.
David
On Tue, 2016-08-09 at 15:19 +1000, Balbir Singh wrote:
quoted
On 04/08/16 22:24, Mimi Zohar wrote:
quoted
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot.
The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.
This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set* for actually carrying the
serialized measurement list across the kexec.
Mimi
Hi, Mimi
I am trying to convince myself of the security of the solution. I asked
Thiago as well, but may be I am be lagging behind in understanding.
We trust the kernel to hand over PCR values of the old kernel (which
cannot be validated) to the IMA subsystem in the new kernel for storage.
I guess the idea is for ima_add_boot_aggregate to do the right thing?
How do we validate what the old kernel is giving us? Why do we care for
the old measurement list? Is it still of significance in the new kernel?
Hi Balbir,
To validate the hardware TPM PCR values requires walking the measurement
list simulating the TPM extend operation. The resulting values should
match the hardware TPM PCRs.
In the case of a soft reboot, the TPM PCRs are not reset to 0, so all
the measurements of the running system, including those from previous
soft reboots, need to be included in the measurement list. Without
these measurements, the simulated PCR values will not match the hardware
TPM PCR values. Thus the need for this patch set.
Measurements can not be added/removed/changed in the measurement list
without it being detectable.
Thanks Mimi
I think that makes sense
So effectively we do
first kernel boot -> <measurements match PCR and measurements are saved>
second kernel boot -> <new PCR = first save measurements + new measurements>
and so on
Balbir Singh
On Thu, 2016-08-11 at 17:38 +1000, Balbir Singh wrote:
On 09/08/16 22:36, Mimi Zohar wrote:
quoted
On Tue, 2016-08-09 at 15:19 +1000, Balbir Singh wrote:
quoted
On 04/08/16 22:24, Mimi Zohar wrote:
quoted
The TPM PCRs are only reset on a hard reboot. In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot.
The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.
This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set* for actually carrying the
serialized measurement list across the kexec.
Mimi
Hi, Mimi
I am trying to convince myself of the security of the solution. I asked
Thiago as well, but may be I am be lagging behind in understanding.
We trust the kernel to hand over PCR values of the old kernel (which
cannot be validated) to the IMA subsystem in the new kernel for storage.
I guess the idea is for ima_add_boot_aggregate to do the right thing?
How do we validate what the old kernel is giving us? Why do we care for
the old measurement list? Is it still of significance in the new kernel?
Hi Balbir,
To validate the hardware TPM PCR values requires walking the measurement
list simulating the TPM extend operation. The resulting values should
match the hardware TPM PCRs.
In the case of a soft reboot, the TPM PCRs are not reset to 0, so all
the measurements of the running system, including those from previous
soft reboots, need to be included in the measurement list. Without
these measurements, the simulated PCR values will not match the hardware
TPM PCR values. Thus the need for this patch set.
Measurements can not be added/removed/changed in the measurement list
without it being detectable.
Thanks Mimi
I think that makes sense
So effectively we do
first kernel boot -> <measurements match PCR and measurements are saved>
second kernel boot -> <new PCR = first save measurements + new measurements>
and so on
No, the running system doesn't verify the measurement list against the
PCRs, before saving and carrying it across kexec. If the system has been
compromised, it can't be trusted to verify itself. Verifying the
measurement list needs to be done by a trusted third party. The system
just carries the measurement list(s) across kexec.
Mimi