On Mon, May 09, 2016 at 12:58:48PM +0200, Jan Schermer wrote: > > On 09 May 2016, at 12:50, martin.wi...@ts.fujitsu.com wrote: > >> I sort_of_assumed that PCR-18 would only be present if the policy > >> verification passed, and would be different different (or all 0s) when the > >> verification failed. > >> This is a bit dangerous if anyone uses it. > > > > You need to use "halt" policy. > > > >> I think something simple like hashing "1" into it when it fails > >> verification would make it useful > > > > If your system is compromised, how do you ensure that this actually > > happens? If you use "halt" policy, the system won't boot with a tampered > > kernel/initrd unless TXT is off. If TXT is off, PCR 18 will be invalid. > > > > So my recommendation is: "halt" policy, pcr_map=da, and protect > > sensitive data by sealing it against PCR 18. Unless I am overlooking > > something, that should be reasonably safe. > Yes, I get it and for me this is fine. > > But if anyone wants to use it with "nonfatal" policy (which could be a valid > scenario) then this would make it useful - the system will still boot but any > secrets won't unseal because of invalid PCR. One can then possibly fix > whatever went wrong (like not generating a new policy on kernel upgrade by > mistake) without having to powercycle the server, revert the policy etc. > In other words, sysadmins usually prefer a booting system they can fix to one > that just halts :-) > > Jan > > Martin
Hi All, First off, I wanted to thank you guys, this thread helped me a ton finally understanding and getting it fully working on my laptop. It'd been on the backburner for ages. I agree with the preference for a system that actually boots. I made a proof of concept that caps PCR18 on a verification failure. I dont really see why we would need both nonfatal(for testing) and continue(not that useful) so I repurposed continue. Making a brand new policy type too would also be easy from the looks of it. Is continue used for something else that I am not aware of that a patch like this would break? What do you think of an approach like this? Can you give it a whirl? I have only tested it on my laptop so more testing is needed. It only extends the value once so calculating the good and failed values is easy. -- Jason
diff -ur tboot-1.9.4.orig/tboot/common/policy.c tboot-1.9.4/tboot/common/policy.c
--- tboot-1.9.4.orig/tboot/common/policy.c 2016-05-19 01:20:26.000000000 +0800
+++ tboot-1.9.4/tboot/common/policy.c 2016-06-19 02:51:33.640032904 +0800
@@ -64,6 +64,7 @@
extern void shutdown(void);
extern void s3_launch(void);
+void fail_cap_pcrs(void);
/* MLE/kernel shared data page (in boot.S) */
extern tboot_shared_t _tboot_shared;
@@ -77,6 +78,7 @@
TB_POLACT_CONTINUE,
TB_POLACT_UNMEASURED_LAUNCH,
TB_POLACT_HALT,
+ TB_POLACT_FAIL_CONTINUE,
} tb_policy_action_t;
/* policy map types */
@@ -109,10 +111,10 @@
{ TB_POLTYPE_CONT_VERIFY_FAIL, TB_POLACT_HALT,
{
- {TB_ERR_MODULE_VERIFICATION_FAILED, TB_POLACT_CONTINUE},
- {TB_ERR_NV_VERIFICATION_FAILED, TB_POLACT_CONTINUE},
- {TB_ERR_POLICY_NOT_PRESENT, TB_POLACT_CONTINUE},
- {TB_ERR_POLICY_INVALID, TB_POLACT_CONTINUE},
+ {TB_ERR_MODULE_VERIFICATION_FAILED, TB_POLACT_FAIL_CONTINUE},
+ {TB_ERR_NV_VERIFICATION_FAILED, TB_POLACT_FAIL_CONTINUE},
+ {TB_ERR_POLICY_NOT_PRESENT, TB_POLACT_FAIL_CONTINUE},
+ {TB_ERR_POLICY_INVALID, TB_POLACT_FAIL_CONTINUE},
{TB_ERR_NONE, TB_POLACT_CONTINUE},
}
},
@@ -193,6 +195,7 @@
/* current policy */
static const tb_policy_t* g_policy = &_def_policy;
+static bool fail_continue = false;
/*
* read_policy_from_tpm
@@ -570,6 +573,9 @@
switch ( action ) {
case TB_POLACT_CONTINUE:
return;
+ case TB_POLACT_FAIL_CONTINUE:
+ fail_cap_pcrs();
+ return;
case TB_POLACT_UNMEASURED_LAUNCH:
/* restore mtrr state saved before */
restore_mtrrs(NULL);
@@ -592,6 +598,26 @@
#define VL_ENTRIES(i) g_pre_k_s3_state.vl_entries[i]
#define NUM_VL_ENTRIES g_pre_k_s3_state.num_vl_entries
+void fail_cap_pcrs(void)
+{
+ uint8_t buf[] = { 0x01, 0x23, 0x45, 0x67 };
+
+ if (!fail_continue) {
+ /* something in the verification failed, but we are continuing
+ so cap PCR 18. only extend it once */
+ printk(TBOOT_INFO"Verification failed, capping PCR 18\n");
+
+ VL_ENTRIES(NUM_VL_ENTRIES).hl.count = 1;
+ VL_ENTRIES(NUM_VL_ENTRIES).hl.entries[0].alg = g_tpm->cur_alg;
+ if ( !hash_buffer(buf, sizeof(buf), &VL_ENTRIES(NUM_VL_ENTRIES).hl.entries[0].hash, g_tpm->cur_alg) )
+ apply_policy(TB_ERR_FATAL);
+
+ VL_ENTRIES(NUM_VL_ENTRIES++).pcr = 18;
+
+ fail_continue = true;
+ }
+}
+
/*
* verify modules against Verified Launch policy and save hash
* if pol_entry is NULL, assume it is for module 0, which gets extended
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic patterns at an interface-level. Reveals which users, apps, and protocols are consuming the most bandwidth. Provides multi-vendor support for NetFlow, J-Flow, sFlow and other flows. Make informed decisions using capacity planning reports. http://sdm.link/zohomanageengine
_______________________________________________ tboot-devel mailing list tboot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tboot-devel
