Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-29 Thread Thiago Jung Bauermann
Am Donnerstag, 29 September 2016, 16:43:08 schrieb Eric W. Biederman:
> Thiago Jung Bauermann  writes:
> > Hello Eric,
> > 
> > Am Dienstag, 20 September 2016, 11:07:29 schrieb Eric W. Biederman:
> >> A semi-generic concept called a hand-over buffer seems to be a
> >> construction of infrustructure for no actual reason that will just
> >> result in confusion.  There are lots of things that are handed over,
> >> the
> >> flattend device tree, ramdisks, bootparams on x86, etc, etc.  ima is
> >> not
> >> special in this execpt for being perhaps the first addition that we are
> >> going to want the option of including on most architectures.
> > 
> > Ok, I understand. I decided to implement a generic concept because I
> > thought that proposing a feature that is more useful than what I need
> > it for would increase its chance of being accepted. It's interesting to
> > see that it had the opposite effect.
> 
> Yes.  In this case it was not clear that anyone else could use it, and
> being less generic you can tweak the needs of the code to ima without
> anyone having to worry about it.
> 
> So thank you very much for making the code more specific to the
> circumstances.

Thank you very much for your feedback and your reviews!

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center



Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-29 Thread Eric W. Biederman
Thiago Jung Bauermann  writes:

> Hello Eric,
>
> Am Dienstag, 20 September 2016, 11:07:29 schrieb Eric W. Biederman:
>> A semi-generic concept called a hand-over buffer seems to be a
>> construction of infrustructure for no actual reason that will just
>> result in confusion.  There are lots of things that are handed over, the
>> flattend device tree, ramdisks, bootparams on x86, etc, etc.  ima is not
>> special in this execpt for being perhaps the first addition that we are
>> going to want the option of including on most architectures.
>
> Ok, I understand. I decided to implement a generic concept because I thought 
> that proposing a feature that is more useful than what I need it for would  
> increase its chance of being accepted. It's interesting to see that it had 
> the opposite effect.

Yes.  In this case it was not clear that anyone else could use it, and
being less generic you can tweak the needs of the code to ima without
anyone having to worry about it.

So thank you very much for making the code more specific to the circumstances.

> I reworked and simplified the code and folded the hand-over buffer patches 
> into Mimi's patch series to carry the measurement list across kexec. The 
> kexec buffer code is in the following patches now:
>
> [PATCH v5 01/10] powerpc: ima: Get the kexec buffer passed by the previous 
> kernel
> [PATCH v5 05/10] powerpc: ima: Send the kexec buffer to the next
> kernel

That plus  [PATCH v5 06/10] ima: on soft reboot, save the measurement list

> Each patch has a changelog listing what I changed to make it specific to 
> IMA.

I am a little sad to see you needed to modify kexec_file.c to get where
you were going, but that isn't a huge issue either way.

Eric




Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-26 Thread Thiago Jung Bauermann
Hello Eric,

Am Dienstag, 20 September 2016, 11:07:29 schrieb Eric W. Biederman:
> Thiago Jung Bauermann  writes:
> > Am Samstag, 17 September 2016, 00:17:37 schrieb Eric W. Biederman:
> >> Thiago Jung Bauermann  writes:
> > Is this what you had in mind?
> 
> Sort of.
> 
> I was just thinking that instead of having the boot path verify your ima
> list matches what is in the tpm and halting the boot there, we could
> test that on reboot.  Which would give a clean failure without the nasty
> poking into a prepared image.  The downside is that we have already run
> the shutdown scripts so it wouldn't be much cleaner, than triggering a
> machine reboot from elsewhere.
> 
> But I don't think we should spend too much time on that.  It was a
> passing thought.  We should focus on getting a non-poked ima buffer
> cleanly into kexec and we can worry about the rest later.

I was thinking of this as something orthogonal to the ima buffer feature.
But you're right, it's better not to discuss this now. I'll post a separate 
patch for this later.

> >> So from 10,000 feet I think that is correct.
> >> 
> >> I am not quite certain why a new mechanism is being invented.  We have
> >> other information that is already passed (much of it architecture
> >> specific) like the flattened device tree.  If you remove the need to
> >> update the information can you just append this information to the
> >> flattened device tree without a new special mechanism to pass the data?
> >> 
> >> I am just reluctant to invent a new mechanism when there is an existing
> >> mechanism that looks like it should work without problems.
> > 
> > Michael Ellerman suggested putting the buffer contents inside the device
> > tree itself, but the s390 people are also planning to implement this
> > feature. That architecture doesn't use device trees, so a solution that
> > depends on DTs won't help them.
> > 
> > With this mechanism each architecture will still need its own way of
> > communicating to the next kernel where the buffer is, but I think it's
> > easier to pass a base address and length than to pass a whole buffer.
> 
> A base address and length pair is fine.  There are several other pieces
> of data that we pass that way.
> 
> > I suppose we could piggyback the ima measurements buffer at the end of
> > one of the other segments such as the kernel or, in the case of
> > powerpc, the dtb but it looks hackish to me. I think it's cleaner to
> > put it in its own segment.
> 
> The boot protocol unfortunately is different on different architectures,
> and for each one we will have to implement and document the change.
> Because when you get into boot protocol issues you can't assume the
> kernel you are booting is the same version as the kernel that is booting
> it.
> 
> Where I run into a problem is you added a semi-generic concept a
> hand-over buffer.  Not a ima data buffer but a hand-over buffer.
> 
> The data falling in it's own dedicated area of memory and being added
> with kexec_add_buffer is completely fine.  I can see a dedicated pointer
> in struct kimage if necessary.
> 
> A semi-generic concept called a hand-over buffer seems to be a
> construction of infrustructure for no actual reason that will just
> result in confusion.  There are lots of things that are handed over, the
> flattend device tree, ramdisks, bootparams on x86, etc, etc.  ima is not
> special in this execpt for being perhaps the first addition that we are
> going to want the option of including on most architectures.

Ok, I understand. I decided to implement a generic concept because I thought 
that proposing a feature that is more useful than what I need it for would  
increase its chance of being accepted. It's interesting to see that it had 
the opposite effect.

I reworked and simplified the code and folded the hand-over buffer patches 
into Mimi's patch series to carry the measurement list across kexec. The 
kexec buffer code is in the following patches now:

[PATCH v5 01/10] powerpc: ima: Get the kexec buffer passed by the previous 
kernel
[PATCH v5 05/10] powerpc: ima: Send the kexec buffer to the next kernel

Each patch has a changelog listing what I changed to make it specific to 
IMA.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center



Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-20 Thread Eric W. Biederman

Thiago Jung Bauermann  writes:

> Am Samstag, 17 September 2016, 00:17:37 schrieb Eric W. Biederman:
>> Thiago Jung Bauermann  writes:
>> > Hello Eric,
>> > 
>> > Am Freitag, 16 September 2016, 14:47:13 schrieb Eric W. Biederman:
>> >> I can see tracking to see if the list has changed at some
>> >> point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.
>> > 
>> > Yes, that is an interesting feature that I can add using the checksum-
>> > verifying part of my code. I can submit a patch for that if there's
>> > interest, adding a reboot notifier that verifies the checksum and causes
>> > a regular reboot instead of a kexec reboot if the checksum fails.
>> 
>> I was thinking an early failure instead of getting all of the way down
>> into a kernel an discovering the tpm/ima subsystem would not
>> initialized.  But where that falls in the reboot pathway I don't expect
>> there is much value in it.
>
> I'm not sure I understand. What I described doesn't involve the tpm or ima. 
> I'm suggesting that if I take the parts of patch 4/5 in the kexec hand-over 
> buffer series that verify the image checksum, I can submit a separate patch 
> that checks the integrity of the kexec image early in kernel_kexec() and 
> reverts to a regular reboot if the check fails. This would be orthogonal to 
> ima carrying its measurement list across kexec.
>
> I think there is value in that, because if the kexec image is corrupted the 
> machine will just get stuck in the purgatory and (unless it's a platform 
> where the purgatory can print to the console) without even an error message 
> explaining what is going on. Whereas if we notice the corruption before 
> jumping into the purgatory we can switch to a regular reboot and the machine 
> will boot successfully.
>
> To have an early failure, when would the checksum verification be done?
> What I can think of is to have kexec_file_load accept a new flag 
> KEXEC_FILE_VERIFY_IMAGE, which userspace could use to request an integrity 
> check when it's about to start the reboot procedure. Then it can decide to 
> either reload the kernel or use a regular reboot if the image is corrupted.
>
> Is this what you had in mind?

Sort of.

I was just thinking that instead of having the boot path verify your ima
list matches what is in the tpm and halting the boot there, we could
test that on reboot.  Which would give a clean failure without the nasty
poking into a prepared image.  The downside is that we have already run
the shutdown scripts so it wouldn't be much cleaner, than triggering a
machine reboot from elsewhere.

But I don't think we should spend too much time on that.  It was a
passing thought.  We should focus on getting a non-poked ima buffer
cleanly into kexec and we can worry about the rest later.

>> >> At least the common bootloader cases that I know of using kexec are
>> >> very
>> >> minimal distributions that live in a ramdisk and as such it should be
>> >> very straight forward to measure what is needed at or before
>> >> sys_kexec_load.  But that was completely dismissed as unrealistic so I
>> >> don't have a clue what actual problem you are trying to solve.
>> > 
>> > We are interested in solving the problem in a general way because it
>> > will be useful to us in the future for the case of an arbitrary number
>> > of kexecs (and thus not only a bootloader but also multiple full-blown
>> > distros may be involved in the chain).
>> > 
>> > But you are right that for the use case for which we currently need this
>> > feature it's feasible to measure everything upfront. We can cross the
>> > other bridge when we get there.
>> 
>> Then let's start there.  Passing the measurment list is something that
>> should not be controversial.
>
> Great!
>
>> >> If there is anyway we can start small and not with this big scary
>> >> infrastructure change I would very much prefer it.
>> > 
>> > Sounds good. If we pre-measure everything then the following patches
>> > from my buffer hand-over series are enough:
>> > 
>> > [PATCH v5 2/5] kexec_file: Add buffer hand-over support for the next
>> > kernel [PATCH v5 3/5] powerpc: kexec_file: Add buffer hand-over support
>> > for the next kernel
>> > 
>> > Would you consider including those two?
>> > 
>> > And like I mentioned in the cover letter, patch 1/5 is an interesting
>> > improvement that is worth considering.
>> 
>> So from 10,000 feet I think that is correct.
>> 
>> I am not quite certain why a new mechanism is being invented.  We have
>> other information that is already passed (much of it architecture
>> specific) like the flattened device tree.  If you remove the need to
>> update the information can you just append this information to the
>> flattened device tree without a new special mechanism to pass the data?
>> 
>> I am just reluctant to invent a new mechanism when there is an existing
>> mechanism that looks like it should work without problems.
>
> Michael 

Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-18 Thread Thiago Jung Bauermann
Am Samstag, 17 September 2016, 00:17:37 schrieb Eric W. Biederman:
> Thiago Jung Bauermann  writes:
> > Hello Eric,
> > 
> > Am Freitag, 16 September 2016, 14:47:13 schrieb Eric W. Biederman:
> >> I can see tracking to see if the list has changed at some
> >> point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.
> > 
> > Yes, that is an interesting feature that I can add using the checksum-
> > verifying part of my code. I can submit a patch for that if there's
> > interest, adding a reboot notifier that verifies the checksum and causes
> > a regular reboot instead of a kexec reboot if the checksum fails.
> 
> I was thinking an early failure instead of getting all of the way down
> into a kernel an discovering the tpm/ima subsystem would not
> initialized.  But where that falls in the reboot pathway I don't expect
> there is much value in it.

I'm not sure I understand. What I described doesn't involve the tpm or ima. 
I'm suggesting that if I take the parts of patch 4/5 in the kexec hand-over 
buffer series that verify the image checksum, I can submit a separate patch 
that checks the integrity of the kexec image early in kernel_kexec() and 
reverts to a regular reboot if the check fails. This would be orthogonal to 
ima carrying its measurement list across kexec.

I think there is value in that, because if the kexec image is corrupted the 
machine will just get stuck in the purgatory and (unless it's a platform 
where the purgatory can print to the console) without even an error message 
explaining what is going on. Whereas if we notice the corruption before 
jumping into the purgatory we can switch to a regular reboot and the machine 
will boot successfully.

To have an early failure, when would the checksum verification be done?
What I can think of is to have kexec_file_load accept a new flag 
KEXEC_FILE_VERIFY_IMAGE, which userspace could use to request an integrity 
check when it's about to start the reboot procedure. Then it can decide to 
either reload the kernel or use a regular reboot if the image is corrupted.

Is this what you had in mind?

> >> At least the common bootloader cases that I know of using kexec are
> >> very
> >> minimal distributions that live in a ramdisk and as such it should be
> >> very straight forward to measure what is needed at or before
> >> sys_kexec_load.  But that was completely dismissed as unrealistic so I
> >> don't have a clue what actual problem you are trying to solve.
> > 
> > We are interested in solving the problem in a general way because it
> > will be useful to us in the future for the case of an arbitrary number
> > of kexecs (and thus not only a bootloader but also multiple full-blown
> > distros may be involved in the chain).
> > 
> > But you are right that for the use case for which we currently need this
> > feature it's feasible to measure everything upfront. We can cross the
> > other bridge when we get there.
> 
> Then let's start there.  Passing the measurment list is something that
> should not be controversial.

Great!

> >> If there is anyway we can start small and not with this big scary
> >> infrastructure change I would very much prefer it.
> > 
> > Sounds good. If we pre-measure everything then the following patches
> > from my buffer hand-over series are enough:
> > 
> > [PATCH v5 2/5] kexec_file: Add buffer hand-over support for the next
> > kernel [PATCH v5 3/5] powerpc: kexec_file: Add buffer hand-over support
> > for the next kernel
> > 
> > Would you consider including those two?
> > 
> > And like I mentioned in the cover letter, patch 1/5 is an interesting
> > improvement that is worth considering.
> 
> So from 10,000 feet I think that is correct.
> 
> I am not quite certain why a new mechanism is being invented.  We have
> other information that is already passed (much of it architecture
> specific) like the flattened device tree.  If you remove the need to
> update the information can you just append this information to the
> flattened device tree without a new special mechanism to pass the data?
> 
> I am just reluctant to invent a new mechanism when there is an existing
> mechanism that looks like it should work without problems.

Michael Ellerman suggested putting the buffer contents inside the device 
tree itself, but the s390 people are also planning to implement this 
feature. That architecture doesn't use device trees, so a solution that 
depends on DTs won't help them.

With this mechanism each architecture will still need its own way of 
communicating to the next kernel where the buffer is, but I think it's 
easier to pass a base address and length than to pass a whole buffer.

I suppose we could piggyback the ima measurements buffer at the end of one 
of the other segments such as the kernel or, in the case of powerpc, the dtb 
but it looks hackish to me. I think it's cleaner to put it in its own 
segment.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center



Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-16 Thread Eric W. Biederman
Thiago Jung Bauermann  writes:

> Hello Eric,
>
> Am Freitag, 16 September 2016, 14:47:13 schrieb Eric W. Biederman:
>> Mimi Zohar  writes:
>> > Hi Andrew,
>> > 
>> > On Wed, 2016-08-31 at 18:38 -0400, Mimi Zohar wrote:
>> >> On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
>> >> > On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar 
>  wrote:
>> >> > > 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, possibly of a different architecture.
>> >> > > 
>> >> > > 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.
>> >> > > 
>> >> > > Up to now, the binary_runtime_measurements was defined as
>> >> > > architecture
>> >> > > native format.  The assumption being that userspace could and would
>> >> > > handle any architecture conversions.  With the ability of carrying
>> >> > > the
>> >> > > measurement list across kexec, possibly from one architecture to a
>> >> > > different one, the per boot architecture information is lost and
>> >> > > with it
>> >> > > the ability of recalculating the template digest hash.  To resolve
>> >> > > this
>> >> > > problem, without breaking the existing ABI, this patch set
>> >> > > introduces
>> >> > > the boot command line option "ima_canonical_fmt", which is
>> >> > > arbitrarily
>> >> > > defined as little endian.
>> >> > > 
>> >> > > The need for this boot command line option will be limited to the
>> >> > > existing version 1 format of the binary_runtime_measurements.
>> >> > > Subsequent formats will be defined as canonical format (eg. TPM 2.0
>> >> > > support for larger digests).
>> >> > > 
>> >> > > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
>> >> > > hand-over for the next kernel" patch set.
>> >> > > 
>> >> > > These patches can also be found in the next-kexec-restore branch
>> >> > > of:
>> >> > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
>> >> > > .git
>> >> > 
>> >> > I'll merge these into -mm to get some linux-next exposure.  I don't
>> >> > know what your upstream merge plans will be?
>> >> 
>> >> Sounds good.  I'm hoping to get some review/comments on this patch set
>> >> as well.  At the moment, I'm chasing down a kernel test robot report
>> >> from this afternoon.
>> > 
>> > My concern about changing the canonical format as originally defined in
>> > patch 9/9 from big endian to little endian never materialized.  Andreas
>> > Steffan, the patch author, is happy either way.
>> > 
>> > We proposed two methods of addressing Eric Biederman's concerns of not
>> > including the IMA measurement list segment in the kexec hash as
>> > described in  https://lkml.org/lkml/2016/9/9/355.
>> > 
>> > - defer calculating and verifying the serialized IMA measurement list
>> > buffer hash to IMA
>> > - calculate the kexec hash on load, verify it on the kexec execute,
>> > before re-calculating and updating it.
>> 
>> I need to ask: How this is anticipated to interact with kexec on panic?
>> Because honestly I can't see this ever working in that case.  The
>> assumption is that the original kernel has gone crazy.  So from a
>> practical standpoint any trusted path should have been invalided.
>
> We are not interested in carrying the measurement list in the case of kexec 
> on panic. I see that the code is adding a hand-over buffer to the crash 
> image as well, but that is a bug.
>
> The fix is to do nothing in ima_add_kexec_buffer if image->type != 
> KEXEC_TYPE_DEFAULT.
>  
>> This entire idea of updating the kexec image makes me extremely
>> extremely nervious.  It feels like sticking a screw driver through the
>> spokes of your bicicle tires while ridding down the road.
>> 
>> I can see tracking to see if the list has changed at some
>> point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.
>
> Yes, that is an interesting feature that I can add using the checksum-
> verifying part of my code. I can submit a patch for that if there's 
> interest, adding a reboot notifier that verifies the checksum and causes a 
> regular reboot instead of a kexec reboot if the checksum fails.

I was thinking an early failure instead of getting all of the way down
into a kernel an discovering the tpm/ima subsystem would not
initialized.  But where that falls in the reboot pathway I don't expect
there is much value in it.

>> At least the common bootloader cases that I know of using kexec are very
>> minimal distributions that live in a ramdisk and as such it should be
>> very straight forward to measure what is needed at or before
>> 

Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-16 Thread Thiago Jung Bauermann
Hello Eric,

Am Freitag, 16 September 2016, 14:47:13 schrieb Eric W. Biederman:
> Mimi Zohar  writes:
> > Hi Andrew,
> > 
> > On Wed, 2016-08-31 at 18:38 -0400, Mimi Zohar wrote:
> >> On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
> >> > On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar 
 wrote:
> >> > > 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, possibly of a different architecture.
> >> > > 
> >> > > 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.
> >> > > 
> >> > > Up to now, the binary_runtime_measurements was defined as
> >> > > architecture
> >> > > native format.  The assumption being that userspace could and would
> >> > > handle any architecture conversions.  With the ability of carrying
> >> > > the
> >> > > measurement list across kexec, possibly from one architecture to a
> >> > > different one, the per boot architecture information is lost and
> >> > > with it
> >> > > the ability of recalculating the template digest hash.  To resolve
> >> > > this
> >> > > problem, without breaking the existing ABI, this patch set
> >> > > introduces
> >> > > the boot command line option "ima_canonical_fmt", which is
> >> > > arbitrarily
> >> > > defined as little endian.
> >> > > 
> >> > > The need for this boot command line option will be limited to the
> >> > > existing version 1 format of the binary_runtime_measurements.
> >> > > Subsequent formats will be defined as canonical format (eg. TPM 2.0
> >> > > support for larger digests).
> >> > > 
> >> > > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
> >> > > hand-over for the next kernel" patch set.
> >> > > 
> >> > > These patches can also be found in the next-kexec-restore branch
> >> > > of:
> >> > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
> >> > > .git
> >> > 
> >> > I'll merge these into -mm to get some linux-next exposure.  I don't
> >> > know what your upstream merge plans will be?
> >> 
> >> Sounds good.  I'm hoping to get some review/comments on this patch set
> >> as well.  At the moment, I'm chasing down a kernel test robot report
> >> from this afternoon.
> > 
> > My concern about changing the canonical format as originally defined in
> > patch 9/9 from big endian to little endian never materialized.  Andreas
> > Steffan, the patch author, is happy either way.
> > 
> > We proposed two methods of addressing Eric Biederman's concerns of not
> > including the IMA measurement list segment in the kexec hash as
> > described in  https://lkml.org/lkml/2016/9/9/355.
> > 
> > - defer calculating and verifying the serialized IMA measurement list
> > buffer hash to IMA
> > - calculate the kexec hash on load, verify it on the kexec execute,
> > before re-calculating and updating it.
> 
> I need to ask: How this is anticipated to interact with kexec on panic?
> Because honestly I can't see this ever working in that case.  The
> assumption is that the original kernel has gone crazy.  So from a
> practical standpoint any trusted path should have been invalided.

We are not interested in carrying the measurement list in the case of kexec 
on panic. I see that the code is adding a hand-over buffer to the crash 
image as well, but that is a bug.

The fix is to do nothing in ima_add_kexec_buffer if image->type != 
KEXEC_TYPE_DEFAULT.
 
> This entire idea of updating the kexec image makes me extremely
> extremely nervious.  It feels like sticking a screw driver through the
> spokes of your bicicle tires while ridding down the road.
> 
> I can see tracking to see if the list has changed at some
> point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.

Yes, that is an interesting feature that I can add using the checksum-
verifying part of my code. I can submit a patch for that if there's 
interest, adding a reboot notifier that verifies the checksum and causes a 
regular reboot instead of a kexec reboot if the checksum fails.

> At least the common bootloader cases that I know of using kexec are very
> minimal distributions that live in a ramdisk and as such it should be
> very straight forward to measure what is needed at or before
> sys_kexec_load.  But that was completely dismissed as unrealistic so I
> don't have a clue what actual problem you are trying to solve.

We are interested in solving the problem in a general way because it will be 
useful to us in the future for the case of an arbitrary number of kexecs 
(and thus not only a bootloader but also multiple full-blown distros may be 
involved in the chain).

But you are right 

Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-16 Thread Eric W. Biederman
ebied...@xmission.com (Eric W. Biederman) writes:

> Mimi Zohar  writes:
>
>> Hi Andrew,
>>
>> On Wed, 2016-08-31 at 18:38 -0400, Mimi Zohar wrote:
>>> On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
>>> > On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar  
>>> > wrote:
>>> > 
>>> > > 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, possibly of a different architecture.
>>> > > 
>>> > > 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.
>>> > > 
>>> > > Up to now, the binary_runtime_measurements was defined as architecture
>>> > > native format.  The assumption being that userspace could and would
>>> > > handle any architecture conversions.  With the ability of carrying the
>>> > > measurement list across kexec, possibly from one architecture to a
>>> > > different one, the per boot architecture information is lost and with it
>>> > > the ability of recalculating the template digest hash.  To resolve this
>>> > > problem, without breaking the existing ABI, this patch set introduces
>>> > > the boot command line option "ima_canonical_fmt", which is arbitrarily
>>> > > defined as little endian.
>>> > > 
>>> > > The need for this boot command line option will be limited to the
>>> > > existing version 1 format of the binary_runtime_measurements.
>>> > > Subsequent formats will be defined as canonical format (eg. TPM 2.0
>>> > > support for larger digests).
>>> > > 
>>> > > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
>>> > > hand-over for the next kernel" patch set. 
>>> > > 
>>> > > These patches can also be found in the next-kexec-restore branch of:
>>> > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
>>> > 
>>> > I'll merge these into -mm to get some linux-next exposure.  I don't
>>> > know what your upstream merge plans will be?
>>> 
>>> Sounds good.  I'm hoping to get some review/comments on this patch set
>>> as well.  At the moment, I'm chasing down a kernel test robot report
>>> from this afternoon.
>>
>> My concern about changing the canonical format as originally defined in
>> patch 9/9 from big endian to little endian never materialized.  Andreas
>> Steffan, the patch author, is happy either way.
>>
>> We proposed two methods of addressing Eric Biederman's concerns of not
>> including the IMA measurement list segment in the kexec hash as
>> described in  https://lkml.org/lkml/2016/9/9/355.
>>
>> - defer calculating and verifying the serialized IMA measurement list
>> buffer hash to IMA
>> - calculate the kexec hash on load, verify it on the kexec execute,
>> before re-calculating and updating it.
>
> I need to ask: How this is anticipated to interact with kexec on panic?
> Because honestly I can't see this ever working in that case.  The
> assumption is that the original kernel has gone crazy.  So from a
> practical standpoint any trusted path should have been invalided.
>
> This entire idea of updating the kexec image makes me extremely
> extremely nervious.  It feels like sticking a screw driver through the
> spokes of your bicicle tires while ridding down the road.
>
> I can see tracking to see if the list has changed at some
> point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.
>
> At least the common bootloader cases that I know of using kexec are very
> minimal distributions that live in a ramdisk and as such it should be
> very straight forward to measure what is needed at or before
> sys_kexec_load.  But that was completely dismissed as unrealistic so I
> don't have a clue what actual problem you are trying to solve.
>
> If there is anyway we can start small and not with this big scary
> infrastructure change I would very much prefer it.

I have thought about this a little more and the entire reason for
updating things on the fly really really disturbs me.  To prove you are
trusted the new kernel is going to have to present that whole trusted
list of files to someone.  Which means in my naive understanding of the
situation that any change in that list of files is going to have to be
tracked and audited.

So the idea that we have to be super flexible in the kernel (when we are
inflexible in userspace) does not make a bit of sense to me.

So no.  I am not in favor of adding a mechanism to kexec that gives me
the screaming heebie jeebies and that appears to be complete at odds
with what that mechanism is trying to do.

AKA if you are going to trust any old thing, or any old change on the
reboot path than it doesn't make sense to track them.  If you are
tracking them it doesn't make sense to have a mechanism where 

Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-16 Thread Eric W. Biederman
Mimi Zohar  writes:

> Hi Andrew,
>
> On Wed, 2016-08-31 at 18:38 -0400, Mimi Zohar wrote:
>> On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
>> > On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar  
>> > wrote:
>> > 
>> > > 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, possibly of a different architecture.
>> > > 
>> > > 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.
>> > > 
>> > > Up to now, the binary_runtime_measurements was defined as architecture
>> > > native format.  The assumption being that userspace could and would
>> > > handle any architecture conversions.  With the ability of carrying the
>> > > measurement list across kexec, possibly from one architecture to a
>> > > different one, the per boot architecture information is lost and with it
>> > > the ability of recalculating the template digest hash.  To resolve this
>> > > problem, without breaking the existing ABI, this patch set introduces
>> > > the boot command line option "ima_canonical_fmt", which is arbitrarily
>> > > defined as little endian.
>> > > 
>> > > The need for this boot command line option will be limited to the
>> > > existing version 1 format of the binary_runtime_measurements.
>> > > Subsequent formats will be defined as canonical format (eg. TPM 2.0
>> > > support for larger digests).
>> > > 
>> > > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
>> > > hand-over for the next kernel" patch set. 
>> > > 
>> > > These patches can also be found in the next-kexec-restore branch of:
>> > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
>> > 
>> > I'll merge these into -mm to get some linux-next exposure.  I don't
>> > know what your upstream merge plans will be?
>> 
>> Sounds good.  I'm hoping to get some review/comments on this patch set
>> as well.  At the moment, I'm chasing down a kernel test robot report
>> from this afternoon.
>
> My concern about changing the canonical format as originally defined in
> patch 9/9 from big endian to little endian never materialized.  Andreas
> Steffan, the patch author, is happy either way.
>
> We proposed two methods of addressing Eric Biederman's concerns of not
> including the IMA measurement list segment in the kexec hash as
> described in  https://lkml.org/lkml/2016/9/9/355.
>
> - defer calculating and verifying the serialized IMA measurement list
> buffer hash to IMA
> - calculate the kexec hash on load, verify it on the kexec execute,
> before re-calculating and updating it.

I need to ask: How this is anticipated to interact with kexec on panic?
Because honestly I can't see this ever working in that case.  The
assumption is that the original kernel has gone crazy.  So from a
practical standpoint any trusted path should have been invalided.

This entire idea of updating the kexec image makes me extremely
extremely nervious.  It feels like sticking a screw driver through the
spokes of your bicicle tires while ridding down the road.

I can see tracking to see if the list has changed at some
point and causing a reboot(LINUX_REBOOT_CMD_KEXEC) to fail.

At least the common bootloader cases that I know of using kexec are very
minimal distributions that live in a ramdisk and as such it should be
very straight forward to measure what is needed at or before
sys_kexec_load.  But that was completely dismissed as unrealistic so I
don't have a clue what actual problem you are trying to solve.

If there is anyway we can start small and not with this big scary
infrastructure change I would very much prefer it.

Eric


Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-09-15 Thread Mimi Zohar
Hi Andrew,

On Wed, 2016-08-31 at 18:38 -0400, Mimi Zohar wrote:
> On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
> > On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar  
> > wrote:
> > 
> > > 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, possibly of a different architecture.
> > > 
> > > 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.
> > > 
> > > Up to now, the binary_runtime_measurements was defined as architecture
> > > native format.  The assumption being that userspace could and would
> > > handle any architecture conversions.  With the ability of carrying the
> > > measurement list across kexec, possibly from one architecture to a
> > > different one, the per boot architecture information is lost and with it
> > > the ability of recalculating the template digest hash.  To resolve this
> > > problem, without breaking the existing ABI, this patch set introduces
> > > the boot command line option "ima_canonical_fmt", which is arbitrarily
> > > defined as little endian.
> > > 
> > > The need for this boot command line option will be limited to the
> > > existing version 1 format of the binary_runtime_measurements.
> > > Subsequent formats will be defined as canonical format (eg. TPM 2.0
> > > support for larger digests).
> > > 
> > > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
> > > hand-over for the next kernel" patch set. 
> > > 
> > > These patches can also be found in the next-kexec-restore branch of:
> > > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> > 
> > I'll merge these into -mm to get some linux-next exposure.  I don't
> > know what your upstream merge plans will be?
> 
> Sounds good.  I'm hoping to get some review/comments on this patch set
> as well.  At the moment, I'm chasing down a kernel test robot report
> from this afternoon.

My concern about changing the canonical format as originally defined in
patch 9/9 from big endian to little endian never materialized.  Andreas
Steffan, the patch author, is happy either way.

We proposed two methods of addressing Eric Biederman's concerns of not
including the IMA measurement list segment in the kexec hash as
described in  https://lkml.org/lkml/2016/9/9/355.

- defer calculating and verifying the serialized IMA measurement list
buffer hash to IMA
- calculate the kexec hash on load, verify it on the kexec execute,
before re-calculating and updating it.

We implemented both options, which can be found in the
next-kexec-restore.with-ima-checksum and next-kexec-restore branches of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

Thiago's patches posted yesterday and mine posted this morning are based
on kexec doing the hashing (the second option).

Lastly, we've addressed the automated kernel build and runtime bug
reports.

Since the IMA patches are dependent on the kexec ones, the two patch
sets need to be upstreamed together.  How should we proceed?   Should
they be upstreamed through your tree?

Mimi



Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-08-31 Thread Mimi Zohar
On Wed, 2016-08-31 at 13:50 -0700, Andrew Morton wrote:
> On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar  
> wrote:
> 
> > 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, possibly of a different architecture.
> > 
> > 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.
> > 
> > Up to now, the binary_runtime_measurements was defined as architecture
> > native format.  The assumption being that userspace could and would
> > handle any architecture conversions.  With the ability of carrying the
> > measurement list across kexec, possibly from one architecture to a
> > different one, the per boot architecture information is lost and with it
> > the ability of recalculating the template digest hash.  To resolve this
> > problem, without breaking the existing ABI, this patch set introduces
> > the boot command line option "ima_canonical_fmt", which is arbitrarily
> > defined as little endian.
> > 
> > The need for this boot command line option will be limited to the
> > existing version 1 format of the binary_runtime_measurements.
> > Subsequent formats will be defined as canonical format (eg. TPM 2.0
> > support for larger digests).
> > 
> > This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
> > hand-over for the next kernel" patch set. 
> > 
> > These patches can also be found in the next-kexec-restore branch of:
> > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> 
> I'll merge these into -mm to get some linux-next exposure.  I don't
> know what your upstream merge plans will be?

Sounds good.  I'm hoping to get some review/comments on this patch set
as well.  At the moment, I'm chasing down a kernel test robot report
from this afternoon.

Mimi



Re: [PATHC v2 0/9] ima: carry the measurement list across kexec

2016-08-31 Thread Andrew Morton
On Tue, 30 Aug 2016 18:40:02 -0400 Mimi Zohar  wrote:

> 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, possibly of a different architecture.
> 
> 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.
> 
> Up to now, the binary_runtime_measurements was defined as architecture
> native format.  The assumption being that userspace could and would
> handle any architecture conversions.  With the ability of carrying the
> measurement list across kexec, possibly from one architecture to a
> different one, the per boot architecture information is lost and with it
> the ability of recalculating the template digest hash.  To resolve this
> problem, without breaking the existing ABI, this patch set introduces
> the boot command line option "ima_canonical_fmt", which is arbitrarily
> defined as little endian.
> 
> The need for this boot command line option will be limited to the
> existing version 1 format of the binary_runtime_measurements.
> Subsequent formats will be defined as canonical format (eg. TPM 2.0
> support for larger digests).
> 
> This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
> hand-over for the next kernel" patch set. 
> 
> These patches can also be found in the next-kexec-restore branch of:
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

I'll merge these into -mm to get some linux-next exposure.  I don't
know what your upstream merge planes will be?



[PATHC v2 0/9] ima: carry the measurement list across kexec

2016-08-30 Thread Mimi Zohar
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, possibly of a different architecture.

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.

Up to now, the binary_runtime_measurements was defined as architecture
native format.  The assumption being that userspace could and would
handle any architecture conversions.  With the ability of carrying the
measurement list across kexec, possibly from one architecture to a
different one, the per boot architecture information is lost and with it
the ability of recalculating the template digest hash.  To resolve this
problem, without breaking the existing ABI, this patch set introduces
the boot command line option "ima_canonical_fmt", which is arbitrarily
defined as little endian.

The need for this boot command line option will be limited to the
existing version 1 format of the binary_runtime_measurements.
Subsequent formats will be defined as canonical format (eg. TPM 2.0
support for larger digests).

This patch set pre-req's Thiago Bauermann's "kexec_file: Add buffer
hand-over for the next kernel" patch set. 

These patches can also be found in the next-kexec-restore branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

Mimi

Andreas Steffen (1):
  ima: platform-independent hash value

Mimi Zohar (7):
  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
  ima: define a canonical binary_runtime_measurements list format

Thiago Jung Bauermann (1):
  ima: on soft reboot, save the measurement list

 Documentation/kernel-parameters.txt   |   4 +
 include/linux/ima.h   |  12 ++
 kernel/kexec_file.c   |   4 +
 security/integrity/ima/Kconfig|  12 ++
 security/integrity/ima/Makefile   |   1 +
 security/integrity/ima/ima.h  |  28 +++
 security/integrity/ima/ima_crypto.c   |   6 +-
 security/integrity/ima/ima_fs.c   |  30 +++-
 security/integrity/ima/ima_init.c |   2 +
 security/integrity/ima/ima_kexec.c| 201 +
 security/integrity/ima/ima_main.c |   1 +
 security/integrity/ima/ima_queue.c|  72 +++-
 security/integrity/ima/ima_template.c | 289 --
 security/integrity/ima/ima_template_lib.c |   6 +-
 14 files changed, 637 insertions(+), 31 deletions(-)
 create mode 100644 security/integrity/ima/ima_kexec.c

-- 
2.1.0