Re: [PATCH AUTOSEL 5.7 03/30] ima: extend boot_aggregate with kernel measurements

[Ken Goldman]

On 12/12/2020 9:22 PM, Mimi Zohar wrote:

Ok.   Going forward, it sounds like we need to define a new
"boot_aggregate" record.  One that contains a version number and PCR
mask.


Just BTW, there is a TCG standard for a TPM 2.0 PCR mask that works
well.

There is also a standard for an event log version number.  It is
the first event of a TPM 2.0 event log.  It is strange.

One useful field, though, is a mapping between the algorithm ID (e.g.,
sha256 is 0x000b) and the digest size (e.g., 32 bytes).  This permits
a parser to parse a log even when it encounters an unknown digest
algorithm.

Re: [PATCH 0/2] public key: IMA signer logging: Log public key of IMA Signature signer in IMA log

[Ken Goldman]

On 5/20/2019 7:15 PM, Lakshmi wrote:

On 5/17/19 7:41 AM, Ken Goldman wrote:

Hi Ken,

Apologize for the delay in responding.

Since a platform typically uses only a few signing keys, 4 bytes makes 
the chance of a collision quite small.  The collision would have to be 
within the same log, not global.


In that worst case, the verifier would have to try two keys.  It's a
slight performance penalty, but does anything break?


Problem Statement:
- If the attestation service has to re-validate the signature reported 
in the IMA log, the service has to maintain the hash\signature of all 
the binaries deployed on all the client nodes. This approach will not 
scale for large cloud deployments.


1 - How is your solution - including a public key with each event - 
related to this issue?


2 - I don't understand how a large cloud affects scale.  Wouldn't the 
verifier would typically be checking known machines - those of their 
enterprise - not every machine on the cloud?


Can't we assume a typical attestation use case has a fairly locked down 
OS with a limited number of applications.



- Possibility of collision of "Key Ids" is non-zero
- In the service if the "Key Id" alone is used to verify using a map of
"Key Id" to "Signing Key(s)", the service cannot determine if the 
trusted signing key was indeed used by the client for signature 
validation (Due to "Key Id" collision issue or malicious signature).


Like I said, it should be rare.  In the worst case, can't the service 
tell by trying both keys?




Proposed Solution:
- The service receives known\trusted signing key(s) from a trusted 
source (that is different from the client machines)
- The clients measure the keys in key rings such as IMA, Platform, 
BuiltIn Trusted, etc. as early as possible in the boot sequence.
- Leave all IMA measurements the same - i.e., we don't log public keys 
in the IMA log for each file, but just use what is currently available 
in IMA.


I thought your solution was to change the IMA measurements, adding the 
public key to each entry with a new template?  Did I misunderstand, or 
do you have a new proposal?




Impact:
- The service can verify that the keyrings have only known\trusted keys.


If the service already has trusted keys from a trusted source, why do 
they have to come from the client at all?



- The service can cross check the "key id" with the key rings measured.
- The look up of keys using the key id would be simpler and faster on 
the service side.

- It can also handle collision of Key Ids.


How does this solve the collision issue?  If there are two keys with the 
same key ID, isn't there still a collision?




Note that the following is a key assumption:

- Only keys signed by a key in the "BuiltIn Trusted Keyring" can be 
added to IMA\Platform keyrings.


I understand how the client keyring is used in IMA to check file
signatures, but how is that related to the attestation service?




Thanks,
  -lakshmi

Re: [PATCH 0/2] public key: IMA signer logging: Log public key of IMA Signature signer in IMA log

[Ken Goldman]

On 5/16/2019 9:29 PM, Lakshmi wrote:

On 5/16/19 7:34 AM, Ken Goldman wrote:


But outside the client machine this key id is not sufficient to
uniquely determine which key the signature corresponds to.


Why is this not sufficient?

In my implementation, I create a lookup table at the attestation 
service that maps the 4-byte IMA log key identifier to the signing 
public key.


Are you concerned about collisions?  Something else?


Yes - the concern is collision.

The "Subject Key Identifier" (SKI) for no two certificate can be the 
same. But since we are using only the last 4 bytes of the SKI it can 
collide. That's mainly the reason I want to log the entire public key.




Since a platform typically uses only a few signing keys, 4 bytes makes 
the chance of a collision quite small.  The collision would have to be 
within the same log, not global.


In that worst case, the verifier would have to try two keys.  It's a
slight performance penalty, but does anything break?

A new template with a larger SKI, perhaps 8 bytes, might be safer.  It 
doesn't expand the log size nearly as much as having a full public key.


Are you suggesting that the client supply the verification public key 
and that the verifier trust it?  Wouldn't that make the log self signed?


How would the verifier determine that the key from the IMA log is 
valid / trusted / not revoked from the log itself?


IMA log is backed by the TPM. So if the public key is added to the IMA 
log the attestation service can validate the key information.

I am not sure if that answers your question.


The TPM just ensures that the log has not been altered.  It does nothing 
for signature verification, right?


The verifier can check that the supplied signature matches the supplied 
public key.  However, how could it verify that the public key is trusted 
to sign the code?  Doesn't that have to be out of band?


E.g., an attacker could create a log with their own signatures and 
public keys.  The signature would verify, but it's the attacker's key.


It's essentially a self-signed file.


A minor question here.

Are you proposing that the IMA log contain a single ima-sigkey entry 
per public key followed by ima-sig entries?


Or are you proposing that ima-sig be replaced by ima-sigkey, and that 
each event would contain both the signature and the public key?


If the latter, this could add 25M to a server's 100K log.  Would that 
increase in size concern anyone?  Could it be a concern on the other 
end, an IoT device with limited memory?


Mimi had raised the same concern. I will update my implementation to 
include the certification information in the IMA log only once per key - 
when that key is added to the IMA or Platform keyring.


If you include the public key only once, don't you have the same 
collision problem?  Two log entries could (in theory) and the same SKI. 
How would the verifier know which public key to use.


However, I think the fundamental question is whether the verifier can 
accept public keys supplied by the untrusted client.  I believe that the 
verifier has to receive the public keys out of band, from a trusted 
source - not the client.

Re: [PATCH 0/2] public key: IMA signer logging: Log public key of IMA Signature signer in IMA log

[Ken Goldman]

On 5/14/2019 1:14 PM, Lakshmi wrote:

The motive behind this patch series is to measure the public key
of the IMA signature signer in the IMA log.


I have some questions about the rationale for the design ...


> The IMA signature of the file, logged using ima-sig template, contains
the key identifier of the key that was used to generate the signature.
But outside the client machine this key id is not sufficient to
uniquely determine which key the signature corresponds to.


Why is this not sufficient?

In my implementation, I create a lookup table at the attestation service 
that maps the 4-byte IMA log key identifier to the signing public key.


Are you concerned about collisions?  Something else?


Providing the public key of the signer in the IMA log would
allow, for example, an attestation service to securely verify
if the key used to generate the IMA signature is a valid and
trusted one, and that the key has not been revoked or expired.


Are you suggesting that the client supply the verification public key 
and that the verifier trust it?  Wouldn't that make the log self signed?


How would the verifier determine that the key from the IMA log is valid 
/ trusted / not revoked from the log itself?



An attestation service would just need to maintain a list of
valid public keys and using the data from the IMA log can attest
the system files loaded on the client machine.


If the verifier has the list of valid keys, why must they also come with 
the IMA log?




To achieve the above the patch series does the following:
   - Adds a new method in asymmetric_key_subtype to query
     the public key of the given key
   - Adds a new IMA template namely "ima-sigkey" to store\read
     the public key of the IMA signature signer. This template
     extends the existing template "ima-sig"


A minor question here.

Are you proposing that the IMA log contain a single ima-sigkey entry per 
public key followed by ima-sig entries?


Or are you proposing that ima-sig be replaced by ima-sigkey, and that 
each event would contain both the signature and the public key?


If the latter, this could add 25M to a server's 100K log.  Would that 
increase in size concern anyone?  Could it be a concern on the other 
end, an IoT device with limited memory?

Re: tpm_tis TPM2.0 not detected on cold boot

[Ken Goldman]

On 12/30/2018 8:22 AM, Michael Niewöhner wrote:

difference is that on a cold boot, the TPM takes longer to initialize.

Well, as I said. Waiting for 10, 20 or even 60 seconds in the boot manager does
not solve the problem. So the problem is NOT that the TPM takes longer to
initialize. Even adding a delay of 20 seconds before TPM init does not solve
that while that should be more than enough time.



This may be TPM hardware dependent.  As I understand it ...

A TPM is permitted to run it's self tests in the background after an
Init (cold boot) , but it's not required to do so.

A TPM that does not - that waits until one of the self test commands is 
issued - will appear to take longer to initialize.


In fact, it's not taking longer.  It's just waiting for some software
to issue a self test command, and will wait forever.

Re: tpm_tis TPM2.0 not detected on cold boot

[Ken Goldman]

On 12/29/2018 10:33 PM, Mimi Zohar wrote:

But the problem you've described is on a cold boot, not a soft reboot.
  Both the soft reboot and kexec are working properly.  It seems the
difference is that on a cold boot, the TPM takes longer to initialize.


I would expect this.

The TPM doesn't even see a 'soft reboot' right?

OTOH, a 'cold boot' hits the TPM Init pin, which causes the take several
actions.  Probably the one with the most impact is resetting the state
of self test.  Thus, the TPM will require a function to be self tested
before it can be used, adding delays.

Re: [tpmdd-devel] New ML for TPM and IMA

[Ken Goldman]

Newbie question:  Do I have to subscribe, or are email addresses being 
migrated?


On 9/15/2017 1:18 PM, Jarkko Sakkinen wrote:

Hi

Many people were kicked out from the SourceForge mailing list in the
July because SF required a resubscription, including non-human users,
such as patchwork.

We decided to create a new mailing list linux-integ...@vger.kernel.org
to cover both TPM and IMA since they tend to have cross dependencies.
Otherwise, the maintainer hierarchy etc. will stay the same.

It is all documented here:

http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity

 From now on use this list for patches and discussion instead of the
legacy list.

Re: [tpmdd-devel] New ML for TPM and IMA

[Ken Goldman]

Newbie question:  Do I have to subscribe, or are email addresses being 
migrated?


On 9/15/2017 1:18 PM, Jarkko Sakkinen wrote:

Hi

Many people were kicked out from the SourceForge mailing list in the
July because SF required a resubscription, including non-human users,
such as patchwork.

We decided to create a new mailing list linux-integ...@vger.kernel.org
to cover both TPM and IMA since they tend to have cross dependencies.
Otherwise, the maintainer hierarchy etc. will stay the same.

It is all documented here:

http://kernsec.org/wiki/index.php/Linux_Kernel_Integrity

 From now on use this list for patches and discussion instead of the
legacy list.

Re: [tpmdd-devel] [PATCH v2 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Ken Goldman]

On 9/6/2017 12:12 PM, Jason Gunthorpe wrote:


The problem with this approach is that the TPM could totally block
the CPU for very long periods of time.

It seems very risky to enable..



How would you characterize "very long"?

The TPM vendors confirm that they empty the FIFO at internal speeds that
are comparable to the bus speed.  Thus, any stall will be sub-usec.  Is
that an issue?

In addition, new TPMs have ever larger FIFO's, making stalls less likely
going forward.

Re: [tpmdd-devel] [PATCH v2 1/4] tpm: ignore burstcount to improve tpm_tis send() performance.

[Ken Goldman]

On 9/6/2017 12:12 PM, Jason Gunthorpe wrote:


The problem with this approach is that the TPM could totally block
the CPU for very long periods of time.

It seems very risky to enable..



How would you characterize "very long"?

The TPM vendors confirm that they empty the FIFO at internal speeds that
are comparable to the bus speed.  Thus, any stall will be sub-usec.  Is
that an issue?

In addition, new TPMs have ever larger FIFO's, making stalls less likely
going forward.

Re: [tpmdd-devel] [PATCH v2] tpm_tis: fix stall after iowrite*()s

[Ken Goldman]

On 8/15/2017 4:13 PM, Haris Okanovic wrote:

ioread8() operations to TPM MMIO addresses can stall the cpu when
immediately following a sequence of iowrite*()'s to the same region.

For example, cyclitest measures ~400us latency spikes when a non-RT
usermode application communicates with an SPI-based TPM chip (Intel Atom
E3940 system, PREEMPT_RT_FULL kernel). The spikes are caused by a
stalling ioread8() operation following a sequence of 30+ iowrite8()s to
the same address. I believe this happens because the write sequence is
buffered (in cpu or somewhere along the bus), and gets flushed on the
first LOAD instruction (ioread*()) that follows.

The enclosed change appears to fix this issue: read the TPM chip's
access register (status code) after every iowrite*() operation to
amortize the cost of flushing data to chip across multiple instructions.


I worry a bit about "appears to fix".  It seems odd that the TPM device 
driver would be the first code to uncover this.  Can anyone confirm that 
the chipset does indeed have this bug?


I'd also like an indication of the performance penalty.  We're doing a 
lot of work to improve the performance and I worry that "do a read after 
every write" will have a performance impact.

Re: [tpmdd-devel] [PATCH v2] tpm_tis: fix stall after iowrite*()s

[Ken Goldman]

On 8/15/2017 4:13 PM, Haris Okanovic wrote:

ioread8() operations to TPM MMIO addresses can stall the cpu when
immediately following a sequence of iowrite*()'s to the same region.

For example, cyclitest measures ~400us latency spikes when a non-RT
usermode application communicates with an SPI-based TPM chip (Intel Atom
E3940 system, PREEMPT_RT_FULL kernel). The spikes are caused by a
stalling ioread8() operation following a sequence of 30+ iowrite8()s to
the same address. I believe this happens because the write sequence is
buffered (in cpu or somewhere along the bus), and gets flushed on the
first LOAD instruction (ioread*()) that follows.

The enclosed change appears to fix this issue: read the TPM chip's
access register (status code) after every iowrite*() operation to
amortize the cost of flushing data to chip across multiple instructions.


I worry a bit about "appears to fix".  It seems odd that the TPM device 
driver would be the first code to uncover this.  Can anyone confirm that 
the chipset does indeed have this bug?


I'd also like an indication of the performance penalty.  We're doing a 
lot of work to improve the performance and I worry that "do a read after 
every write" will have a performance impact.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/14/2017 6:10 AM, Jarkko Sakkinen wrote:

Nuvoton, ST Micro, and Infineon confirmed that the TPM empties a byte from
the FIFO in under 1 usec.  So, even with a static burst count, the entire
FIFO would empty in under 10 usec.


Does this break anything lets say in a decade time frame? If it does, I
will not even consider this. Can you give a definitive answer for that?


My attempt at predicting the future ...

1 - Clock speed should get faster, not slower, so the 1 usec wait state 
should get shorter.


2 - TPM buffers should get larger.  I'm already reading of 256 byte 
buffers.  So there should be fewer wait states.


3 - There is a trend toward using the CRB, making the FIFO less applicable.

4 - There is a trend away from LPC connected serial port devices, 
printers, and PS/2 mouse and keyboard, and toward USB connected devices. 
 I assume this will continue, making the already insignificant wait 
states irrelevant.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/14/2017 6:10 AM, Jarkko Sakkinen wrote:

Nuvoton, ST Micro, and Infineon confirmed that the TPM empties a byte from
the FIFO in under 1 usec.  So, even with a static burst count, the entire
FIFO would empty in under 10 usec.


Does this break anything lets say in a decade time frame? If it does, I
will not even consider this. Can you give a definitive answer for that?


My attempt at predicting the future ...

1 - Clock speed should get faster, not slower, so the 1 usec wait state 
should get shorter.


2 - TPM buffers should get larger.  I'm already reading of 256 byte 
buffers.  So there should be fewer wait states.


3 - There is a trend toward using the CRB, making the FIFO less applicable.

4 - There is a trend away from LPC connected serial port devices, 
printers, and PS/2 mouse and keyboard, and toward USB connected devices. 
 I assume this will continue, making the already insignificant wait 
states irrelevant.

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/13/2017 7:53 PM, msuchanek wrote:

About 500 out of 700 mainboards sold today has a PS/2 port which is
probably due to prevalence of legacy devices and usbhid limitations.

Similarily many boards have serial and parallel hardware ports.

In all diagrams detailed enough to show these ports I have seen them
attached to the LPC bus.


Do these boards have a TPM?  Remember that the TPM requires special LPC 
bus cycles.


Even if so, the TPM LPC bus wait states are less than a usec.  My 
thought is that  it's unlikely that any device (serial port, mouse, 
keyboard, printer) will be adversely affected.

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/13/2017 7:53 PM, msuchanek wrote:

About 500 out of 700 mainboards sold today has a PS/2 port which is
probably due to prevalence of legacy devices and usbhid limitations.

Similarily many boards have serial and parallel hardware ports.

In all diagrams detailed enough to show these ports I have seen them
attached to the LPC bus.


Do these boards have a TPM?  Remember that the TPM requires special LPC 
bus cycles.


Even if so, the TPM LPC bus wait states are less than a usec.  My 
thought is that  it's unlikely that any device (serial port, mouse, 
keyboard, printer) will be adversely affected.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/9/2017 4:43 PM, Peter Huewe wrote:


Yes that's bad, especially with current msleep(5) is actually
msleep(20). However, please also keep in mind SPI tpms show a much
higher burst count value,  (255) our I2C TPM SLB9645 even shows
something in the range of 1k. :)


One of our platforms has a TPM 1.2 with an 8 byte FIFO and a static 
burst count.  This is where we're debugging.



Would another solution be to reduce the burst count poll and sleep
to, e.g., 100 usec or even 10 usec? This would probably help
greatly, but till not incur the wait states that triggered the
NACK.


If you use sleep it's not guaranteed that you wakeup after exactly xx
specified amount of time. Just that you sleep at least xx amount of
time. Otherwise you would have to do delay/busywaiting.


Understood.  However, if the DD maintainers will not accept ignoring 
burstCount, would a short sleep (e.g., 10 usec) be acceptable.


If so, we can benchmark it.


Imho the best option is to figure out whether any vendor can
determine the "FIFO flush time" i.e. how much time does it take to
empty the fifo and fillup the burstcount and use this on the lower
bound of an usleep range. I don't think tpms are in the range of < 10
us...

@Ken: Maybe can you check in DDWG?


I asked this week.

Nuvoton, ST Micro, and Infineon confirmed that the TPM empties a byte 
from the FIFO in under 1 usec.  So, even with a static burst count, the 
entire FIFO would empty in under 10 usec.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/9/2017 4:43 PM, Peter Huewe wrote:


Yes that's bad, especially with current msleep(5) is actually
msleep(20). However, please also keep in mind SPI tpms show a much
higher burst count value,  (255) our I2C TPM SLB9645 even shows
something in the range of 1k. :)


One of our platforms has a TPM 1.2 with an 8 byte FIFO and a static 
burst count.  This is where we're debugging.



Would another solution be to reduce the burst count poll and sleep
to, e.g., 100 usec or even 10 usec? This would probably help
greatly, but till not incur the wait states that triggered the
NACK.


If you use sleep it's not guaranteed that you wakeup after exactly xx
specified amount of time. Just that you sleep at least xx amount of
time. Otherwise you would have to do delay/busywaiting.


Understood.  However, if the DD maintainers will not accept ignoring 
burstCount, would a short sleep (e.g., 10 usec) be acceptable.


If so, we can benchmark it.


Imho the best option is to figure out whether any vendor can
determine the "FIFO flush time" i.e. how much time does it take to
empty the fifo and fillup the burstcount and use this on the lower
bound of an usleep range. I don't think tpms are in the range of < 10
us...

@Ken: Maybe can you check in DDWG?


I asked this week.

Nuvoton, ST Micro, and Infineon confirmed that the TPM empties a byte 
from the FIFO in under 1 usec.  So, even with a static burst count, the 
entire FIFO would empty in under 10 usec.

Re: [Linux-ima-devel] [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

Following up on this thread based on this week's TCG call ...

1 - burstCount can safely be ignored on writes.  This is explicit in
most places in the TCG spec.  In places where it is not explicit, it was 
simply an editorial omission.  We are going through the spec and adding 
"without incurring wait states."


TCG is willing to publish an errata if that makes developers more 
comfortable.


2 - These are multi-mhz buses.  The TPM vendors conformed that wait 
states, even if incurred, will be sub-usec.  I.e., less that a microsecond.


Essentially, the DD is loading the FIFO, and the TPM is unloading the 
FIFO at processor speeds.


Thus, even if one were worried about an odd system new enough to have a 
TPM, but old enough to have an LPC attached printer, keyboard, mouse or 
floppy, the delay in printing or typing will be insignificant.


3 - I asked several platform vendors with long TCG experience, and they 
said that they know of no motherboards that share the LPC bus with a TPM 
plus another device.

Re: [Linux-ima-devel] [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

Following up on this thread based on this week's TCG call ...

1 - burstCount can safely be ignored on writes.  This is explicit in
most places in the TCG spec.  In places where it is not explicit, it was 
simply an editorial omission.  We are going through the spec and adding 
"without incurring wait states."


TCG is willing to publish an errata if that makes developers more 
comfortable.


2 - These are multi-mhz buses.  The TPM vendors conformed that wait 
states, even if incurred, will be sub-usec.  I.e., less that a microsecond.


Essentially, the DD is loading the FIFO, and the TPM is unloading the 
FIFO at processor speeds.


Thus, even if one were worried about an odd system new enough to have a 
TPM, but old enough to have an LPC attached printer, keyboard, mouse or 
floppy, the delay in printing or typing will be insignificant.


3 - I asked several platform vendors with long TCG experience, and they 
said that they know of no motherboards that share the LPC bus with a TPM 
plus another device.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/9/2017 5:00 PM, Peter Huewe wrote:


Since we are the linux kernel, we do have to care for legacy
devices. And a system with LPC, PS2Mouse on SuperIO and a TPM are not
that uncommon.

And heck, we even have support for 1.1b TPM devices


Understood.  However, remember that SuperIO is a 1980's device that
predates the TPM.  Since the TPM requires special LPC bus cycles, it's
even less likely that an old chipset has an attached TPM.


Interesting - let me check with Georg tomorrow. Unfortunately I do
not have access to my tcg mails from home (since I'm not working :), 
but did you _explicitly_ talk about LPC and the system? I'm sure the

TPM does not care about the waitstates...


Yes, Infineon was one of the 3 TPM vendors who confirmed this.

Re: Aw: Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/9/2017 5:00 PM, Peter Huewe wrote:


Since we are the linux kernel, we do have to care for legacy
devices. And a system with LPC, PS2Mouse on SuperIO and a TPM are not
that uncommon.

And heck, we even have support for 1.1b TPM devices


Understood.  However, remember that SuperIO is a 1980's device that
predates the TPM.  Since the TPM requires special LPC bus cycles, it's
even less likely that an old chipset has an attached TPM.


Interesting - let me check with Georg tomorrow. Unfortunately I do
not have access to my tcg mails from home (since I'm not working :), 
but did you _explicitly_ talk about LPC and the system? I'm sure the

TPM does not care about the waitstates...


Yes, Infineon was one of the 3 TPM vendors who confirmed this.

Re: [Linux-ima-devel] [PATCH 11/12] ima: don't report measurements if digests are included in the loaded lists

[Ken Goldman]

On 7/25/2017 11:44 AM, Roberto Sassu wrote:

Don't report measurements if the file digest has been included in
an uploaded digest list.

The advantage of this solution is that the boot time overhead, when
a TPM is available, is very small because a PCR is extended only
for unknown files. The disadvantage is that verifiers do not know
anymore which and when files are accessed (they must assume that
the worst case happened, i.e. all files have been accessed).


Am I reading this correctly that you want to measure certain files, but 
not ones that have been included in a "digest list", which sounds like a 
white list of sorts.


If so, I have two concerns:

1 - How would the client get this digest list?  Shouldn't it be up to 
the relying party to decide what is trusted and not trusted, not the client?


What of the case with two different relying parties that have a 
different list of trusted applications?  E.g., one trusts any version of 
program X, while the other trusts only version 3.1 and up?


2 - What about files on the digest list that were not run?  The relying 
party may want to know if a program wasn't run?  E.g., antivirus or a 
firewall.


If the rule is "don't measure if it's on the digest list", how does the 
relying party know if it was run?

Re: [Linux-ima-devel] [PATCH 11/12] ima: don't report measurements if digests are included in the loaded lists

[Ken Goldman]

On 7/25/2017 11:44 AM, Roberto Sassu wrote:

Don't report measurements if the file digest has been included in
an uploaded digest list.

The advantage of this solution is that the boot time overhead, when
a TPM is available, is very small because a PCR is extended only
for unknown files. The disadvantage is that verifiers do not know
anymore which and when files are accessed (they must assume that
the worst case happened, i.e. all files have been accessed).


Am I reading this correctly that you want to measure certain files, but 
not ones that have been included in a "digest list", which sounds like a 
white list of sorts.


If so, I have two concerns:

1 - How would the client get this digest list?  Shouldn't it be up to 
the relying party to decide what is trusted and not trusted, not the client?


What of the case with two different relying parties that have a 
different list of trusted applications?  E.g., one trusts any version of 
program X, while the other trusts only version 3.1 and up?


2 - What about files on the digest list that were not run?  The relying 
party may want to know if a program wasn't run?  E.g., antivirus or a 
firewall.


If the rule is "don't measure if it's on the digest list", how does the 
relying party know if it was run?

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/8/2017 3:11 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 07, 2017 at 01:52:34PM +0200, Peter Huewe wrote:

>> Are you sure this is a good idea?
>> On lpc systems this more or less stalls the bus, including 
keyboard/mouse (if connected via superio lpc).

>>
>> On which systems have you tested this?
>> Spi/Lpc? Architecture?
>>
>> This might not be noticable for small transfers, but think about 
much larger transfers

>>
>> Imho: NACK from my side.
>>
>> Thanks,
>> Peter
>
> Thanks Peter, a great insight. TPM could share the bus with other
> devices. Even if this optimizes the performance for TPM it might cause
> performance issues elsewhere.

Does anyone know of platforms where this occurs?

I suspect (but not sure) that the days of SuperIO connecting floppy 
drives, printer ports, and PS/2 mouse ports on the LPC bus are over, and 
such legacy systems will not have a TPM. Would SuperIO even support the 
special TPM LPC bus cycles?


Even then, the wait states of a mhz speed LPC are likely to be usec,
not noticeable for even a mouse.

Is this a reasonable assumption?

If so, to we affect TPM performance to the point where it's unusable to 
help a case that is unlikely to appear in current platforms?


>
> One more viewpoint: TCG must added the burst count for a reason (might
> be very well related what Peter said). Is ignoring it something that TCG
> recommends? Not following standard exactly in the driver code sometimes
> makes sense on *small details* but I would not say that this a small
> detail...

I checked with the TCG's device driver work group (DDWG).  Both the spec 
editor and 3 TPM vendors - Infineon, Nuvoton, and ST Micro - agreed that 
ignoring burst count may incur wait states but nothing more.  Operations 
will still be successful.

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/8/2017 3:11 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 07, 2017 at 01:52:34PM +0200, Peter Huewe wrote:

>> Are you sure this is a good idea?
>> On lpc systems this more or less stalls the bus, including 
keyboard/mouse (if connected via superio lpc).

>>
>> On which systems have you tested this?
>> Spi/Lpc? Architecture?
>>
>> This might not be noticable for small transfers, but think about 
much larger transfers

>>
>> Imho: NACK from my side.
>>
>> Thanks,
>> Peter
>
> Thanks Peter, a great insight. TPM could share the bus with other
> devices. Even if this optimizes the performance for TPM it might cause
> performance issues elsewhere.

Does anyone know of platforms where this occurs?

I suspect (but not sure) that the days of SuperIO connecting floppy 
drives, printer ports, and PS/2 mouse ports on the LPC bus are over, and 
such legacy systems will not have a TPM. Would SuperIO even support the 
special TPM LPC bus cycles?


Even then, the wait states of a mhz speed LPC are likely to be usec,
not noticeable for even a mouse.

Is this a reasonable assumption?

If so, to we affect TPM performance to the point where it's unusable to 
help a case that is unlikely to appear in current platforms?


>
> One more viewpoint: TCG must added the burst count for a reason (might
> be very well related what Peter said). Is ignoring it something that TCG
> recommends? Not following standard exactly in the driver code sometimes
> makes sense on *small details* but I would not say that this a small
> detail...

I checked with the TCG's device driver work group (DDWG).  Both the spec 
editor and 3 TPM vendors - Infineon, Nuvoton, and ST Micro - agreed that 
ignoring burst count may incur wait states but nothing more.  Operations 
will still be successful.

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/8/2017 3:11 PM, Jarkko Sakkinen wrote:

On Mon, Aug 07, 2017 at 01:52:34PM +0200, Peter Huewe wrote:

Imho: NACK from my side.

After these viewpoints definitive NACK from my side too...



I responded to the thread comments separately.  However, assuming NACK 
is the final response, I have a question.


The problem is the 5 msec sleep between polls of burst count.  In the 
case of one TPM with an 8 byte FIFO, a 32 byte transfer incurs 4 of 
these sleeps.


Would another solution be to reduce the burst count poll and sleep to, 
e.g., 100 usec or even 10 usec?  This would probably help greatly, but 
still not incur the wait states that triggered the NACK.


My worry is that the scheduler would not be able to context switch that 
fast, and so we wouldn't actually see usec speed polling.


Can a kernel expert offer an opinion?

Re: [tpmdd-devel] [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount

[Ken Goldman]

On 8/8/2017 3:11 PM, Jarkko Sakkinen wrote:

On Mon, Aug 07, 2017 at 01:52:34PM +0200, Peter Huewe wrote:

Imho: NACK from my side.

After these viewpoints definitive NACK from my side too...



I responded to the thread comments separately.  However, assuming NACK 
is the final response, I have a question.


The problem is the 5 msec sleep between polls of burst count.  In the 
case of one TPM with an 8 byte FIFO, a 32 byte transfer incurs 4 of 
these sleeps.


Would another solution be to reduce the burst count poll and sleep to, 
e.g., 100 usec or even 10 usec?  This would probably help greatly, but 
still not incur the wait states that triggered the NACK.


My worry is that the scheduler would not be able to context switch that 
fast, and so we wouldn't actually see usec speed polling.


Can a kernel expert offer an opinion?

Re: [tpmdd-devel] [Linux-ima-devel] [PATCH v3 0/6] Updated API for TPM 2.0 PCR extend

[Ken Goldman]

On 6/28/2017 1:28 PM, Jarkko Sakkinen wrote:
> On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
>> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
>>> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>>
>>
>>> To move this forward and be more constructive here's how I see it
>>> should be done (along the lines, draft):
>>>
>>> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
>>>   const u8 *hash);

This appears to be a single algorithm extend.

TPM 2.0 permits all algorithms to be extended in one operation. 
Splitting it is likely to nearly double the extend time.


Would performance be better using the TPM pattern, a count plus 
algorithm / digest pairs?  It's TPML_DIGEST_VALUES, the input to 
TPM2_PCR_Extend.


>>>
>>> The paramater 'alg' is crypto ID as specified by crypto subsystem.
>>
>> Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
>> agile measurement list will contain the TPM crypto hash algorithm ids
>> (TPM crypto-ID).
>
> Doesn't this lock you to TPM?
>

If you seriously want to do this, I guess it is fine by me but I'm
just wondering why the measurement list couldn't use something with
more loose binding to TPM.

Are you asking, "Why use the TPM algorithm ID?"  If so:

1 - The IMA measurement log is already closely linked to a TPM.

2 - Why not use the TPM algorithm IDs?  They are standardized (ISO) and 
maintained.  It's unlikely that a TPM will ever be manufactured that 
uses a digest algorithm that is not in the TCG registry.


3 - The device driver needs the TPM algorithm ID already to do the 
extend, so it seems natural to use that value everywhere.


>>> TPM driver must have a precompiled table of mappings for crypto IDs
>>> and TPM algorithm IDs.
>>
>> We could map the TPM crypto-IDs to the crypto subsystem IDs and then
>> map them back, but is that necessary?

That's the question.  Why have two values and add the mapping?

>>> There's absolutely no need to pass digest size like you do BTW as 
it >>> is defined by the standard.

>>
>> For algorithms known to the crypto subsystem, that is fine, but for
>> the unknown TPM crypto algorithms, we would need to somehow query the
>> TPM for the digest sizes to create the mapping.
>>
>> Mimi
>
> There's a TPM command to query TPM algorithms.

This is true - one getcap to determine the number of algorithms, then a 
pcr read, then parse the response structures and match the algorithms to 
sizes.


Alternatively, could you create a table mapping the algorithm to the 
size?  There are currently 8 approved algorithms, meaning the table is 
32 bytes, probably less code than the queries.


As for an algorithm appearing in the TPM that's not in the table, it 
takes a year or more for a new algorithm to appear.  Is that enough time 
to patch the device driver?


FYI, the 8 algorithms are:

sha1, sha256, sha384, sha512, sm3-256, sha3-256, sha3-384, sha3-512.

I am only aware of sha1, sha256, and sm3-256 being used in production 
hardware TPMs.

Re: [tpmdd-devel] [Linux-ima-devel] [PATCH v3 0/6] Updated API for TPM 2.0 PCR extend

[Ken Goldman]

On 6/28/2017 1:28 PM, Jarkko Sakkinen wrote:
> On Mon, Jun 26, 2017 at 08:33:59AM -0400, Mimi Zohar wrote:
>> On Sat, 2017-06-24 at 11:03 +0200, Jarkko Sakkinen wrote:
>>> On Wed, Jun 21, 2017 at 04:29:35PM +0200, Roberto Sassu wrote:
>>
>>
>>> To move this forward and be more constructive here's how I see it
>>> should be done (along the lines, draft):
>>>
>>> int tpm_pcr_extend(u32 chip_num, int pcr_idx, unsigned int alg,
>>>   const u8 *hash);

This appears to be a single algorithm extend.

TPM 2.0 permits all algorithms to be extended in one operation. 
Splitting it is likely to nearly double the extend time.


Would performance be better using the TPM pattern, a count plus 
algorithm / digest pairs?  It's TPML_DIGEST_VALUES, the input to 
TPM2_PCR_Extend.


>>>
>>> The paramater 'alg' is crypto ID as specified by crypto subsystem.
>>
>> Based on Kenneth Goldman's input, the new IMA TPM-2.0 crypto hash
>> agile measurement list will contain the TPM crypto hash algorithm ids
>> (TPM crypto-ID).
>
> Doesn't this lock you to TPM?
>

If you seriously want to do this, I guess it is fine by me but I'm
just wondering why the measurement list couldn't use something with
more loose binding to TPM.

Are you asking, "Why use the TPM algorithm ID?"  If so:

1 - The IMA measurement log is already closely linked to a TPM.

2 - Why not use the TPM algorithm IDs?  They are standardized (ISO) and 
maintained.  It's unlikely that a TPM will ever be manufactured that 
uses a digest algorithm that is not in the TCG registry.


3 - The device driver needs the TPM algorithm ID already to do the 
extend, so it seems natural to use that value everywhere.


>>> TPM driver must have a precompiled table of mappings for crypto IDs
>>> and TPM algorithm IDs.
>>
>> We could map the TPM crypto-IDs to the crypto subsystem IDs and then
>> map them back, but is that necessary?

That's the question.  Why have two values and add the mapping?

>>> There's absolutely no need to pass digest size like you do BTW as 
it >>> is defined by the standard.

>>
>> For algorithms known to the crypto subsystem, that is fine, but for
>> the unknown TPM crypto algorithms, we would need to somehow query the
>> TPM for the digest sizes to create the mapping.
>>
>> Mimi
>
> There's a TPM command to query TPM algorithms.

This is true - one getcap to determine the number of algorithms, then a 
pcr read, then parse the response structures and match the algorithms to 
sizes.


Alternatively, could you create a table mapping the algorithm to the 
size?  There are currently 8 approved algorithms, meaning the table is 
32 bytes, probably less code than the queries.


As for an algorithm appearing in the TPM that's not in the table, it 
takes a year or more for a new algorithm to appear.  Is that enough time 
to patch the device driver?


FYI, the 8 algorithms are:

sha1, sha256, sha384, sha512, sm3-256, sha3-256, sha3-384, sha3-512.

I am only aware of sha1, sha256, and sm3-256 being used in production 
hardware TPMs.

Re: [tpmdd-devel] [PATCH] tpm: consolidate the TPM startup code

[Ken Goldman]

On 6/20/2017 3:31 PM, Jason Gunthorpe wrote:


+#define TPM_ORD_STARTUP 153 +#define TPM_ST_CLEAR 1


We should really have a tpm1.h and tpm2.h that has all these various 
constants and things instead of open coding them randomly all over..


While you're doing that, perhaps put the ordinal numbers in hex.  The
TPM 1.2 spec uses both hex and decimal, but TPM 2.0 uses hex.

Re: [tpmdd-devel] [PATCH] tpm: consolidate the TPM startup code

[Ken Goldman]

On 6/20/2017 3:31 PM, Jason Gunthorpe wrote:


+#define TPM_ORD_STARTUP 153 +#define TPM_ST_CLEAR 1


We should really have a tpm1.h and tpm2.h that has all these various 
constants and things instead of open coding them randomly all over..


While you're doing that, perhaps put the ordinal numbers in hex.  The
TPM 1.2 spec uses both hex and decimal, but TPM 2.0 uses hex.

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/18/2017 5:38 AM, Roberto Sassu wrote:


There cannot be buffer overflow, because the length of each digest
field is known.


Crypto Agile: pcr[4] total_digest_len[4]
   digest1_len[4] digest1[digest1_len] ...

The way I read this, the digest length is supplied by the caller, which 
is slightly different from "known".  For example, if I supply a digest 
length of 0x, a too trusting (buggy) parser could overflow the 
buffer.


total_digest_len is similarly untrusted.  The attacker can send invalid 
values.

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/18/2017 5:38 AM, Roberto Sassu wrote:


There cannot be buffer overflow, because the length of each digest
field is known.


Crypto Agile: pcr[4] total_digest_len[4]
   digest1_len[4] digest1[digest1_len] ...

The way I read this, the digest length is supplied by the caller, which 
is slightly different from "known".  For example, if I supply a digest 
length of 0x, a too trusting (buggy) parser could overflow the 
buffer.


total_digest_len is similarly untrusted.  The attacker can send invalid 
values.

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/18/2017 5:38 AM, Roberto Sassu wrote:

On 5/17/2017 6:28 PM, Ken Goldman wrote:

On 5/17/2017 3:25 AM, Roberto Sassu wrote:


The format of digestN is: :\0, the same used
for the file digest.


Since the format is changing from the SHA-1 log format anyway ...

How do people feel about the colon and null terminated string format for
algorithm identifiers?

The TCG standard enumerations are uint16_t, and there is a registry of
hash algorithms.

As a consuming parser, it feels nice to know it's always 2 bytes and not
have to worry about a missing colon or a missing nul terminator risking
a buffer overflow.


There cannot be buffer overflow, because the length of each digest
field is known.

Roberto



I was not referring to the digest, but the digest algorithm.

I wanted opinions on the colon and null terminated string format for 
algorithm identifiers.


The TCG standard log uses the TCG standard enumerations.  They're always 
exactly 2 bytes.  Parsing is trivial.


If IMA uses strings, the attacker can send, e.g., sha1: and not null 
terminate it.  A careful parser can go a byte at a time until it reaches 
a maximum length - if you specify a maximum length.  But it is an attack 
surface.  Is there a corresponding advantage?

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/18/2017 5:38 AM, Roberto Sassu wrote:

On 5/17/2017 6:28 PM, Ken Goldman wrote:

On 5/17/2017 3:25 AM, Roberto Sassu wrote:


The format of digestN is: :\0, the same used
for the file digest.


Since the format is changing from the SHA-1 log format anyway ...

How do people feel about the colon and null terminated string format for
algorithm identifiers?

The TCG standard enumerations are uint16_t, and there is a registry of
hash algorithms.

As a consuming parser, it feels nice to know it's always 2 bytes and not
have to worry about a missing colon or a missing nul terminator risking
a buffer overflow.


There cannot be buffer overflow, because the length of each digest
field is known.

Roberto



I was not referring to the digest, but the digest algorithm.

I wanted opinions on the colon and null terminated string format for 
algorithm identifiers.


The TCG standard log uses the TCG standard enumerations.  They're always 
exactly 2 bytes.  Parsing is trivial.


If IMA uses strings, the attacker can send, e.g., sha1: and not null 
terminate it.  A careful parser can go a byte at a time until it reaches 
a maximum length - if you specify a maximum length.  But it is an attack 
surface.  Is there a corresponding advantage?

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/17/2017 3:25 AM, Roberto Sassu wrote:


The format of digestN is: :\0, the same used
for the file digest.


Since the format is changing from the SHA-1 log format anyway ...

How do people feel about the colon and null terminated string format for 
algorithm identifiers?


The TCG standard enumerations are uint16_t, and there is a registry of 
hash algorithms.


As a consuming parser, it feels nice to know it's always 2 bytes and not 
have to worry about a missing colon or a missing nul terminator risking 
a buffer overflow.

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/17/2017 3:25 AM, Roberto Sassu wrote:


The format of digestN is: :\0, the same used
for the file digest.


Since the format is changing from the SHA-1 log format anyway ...

How do people feel about the colon and null terminated string format for 
algorithm identifiers?


The TCG standard enumerations are uint16_t, and there is a registry of 
hash algorithms.


As a consuming parser, it feels nice to know it's always 2 bytes and not 
have to worry about a missing colon or a missing nul terminator risking 
a buffer overflow.

Re: [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.

[Ken Goldman]

On 5/10/2017 7:54 PM, Stefan Berger wrote:

Implement the request_locality function. To set the locality on the
backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
a command to the backend to set the locality for the next commands.


When this says "for the next commands", does that mean that the locality
is fixed for all commands until a new request_locality is sent to change it?

Re: [tpmdd-devel] [PATCH v4 2/2] tpm: vtpm_proxy: Implement request_locality function.

[Ken Goldman]

On 5/10/2017 7:54 PM, Stefan Berger wrote:

Implement the request_locality function. To set the locality on the
backend we define vendor-specific TPM 1.2 and TPM 2 ordinals and send
a command to the backend to set the locality for the next commands.


When this says "for the next commands", does that mean that the locality
is fixed for all commands until a new request_locality is sent to change it?

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/16/2017 8:53 AM, Roberto Sassu wrote:

A new IMA measurement list format, called Crypto Agile, will be introduced
shortly to take full advantage of the algorithm flexibility of TPM 2.0.
With the new format, it will be possible to provide for each list entry
multiple digests, each calculated with an algorithm supported by the TPM.
Those digests will be used by remote entities to verify the integrity of
the measurements list.

The current (SHA1) and the new (Crypto Agile) format definitions are:

SHA1: pcr[4] digest[20]
   template_name_len[4] template_name[template_name_len]
   template_data_len[4] template_data[template_data_len]

Crypto Agile: pcr[4] total_digest_len[4]
   digest1_len[4] digest1[digest1_len] ...
   digestN_len[4] digestN[digestN_len]
   template_name_len[4] template_name[template_name_len]
   template_data_len[4] template_data[template_data_len]


1 - In this proposed format, how does the parser or consumer of the log
know what algorithm is used for digestN.
For example, the TCG standard format uses TPML_DIGEST_VALUES
uint32_t count - the number of digests TPMT_HA
TPMT_HA digests[]

where a TPMT_HA is
algorithm identifier
digest byte array

2 - Not a criticism, just a question for understanding ...  Would it be 
true that the total_digest_length == the sum of all the digestN_len 
values plus 4 bytes for each length.


Does it determine how many digests there are by when the total length is 
consumed?

Re: [Linux-ima-devel] [PATCH 0/7] IMA: new parser for ima_restore_measurement_list()

[Ken Goldman]

On 5/16/2017 8:53 AM, Roberto Sassu wrote:

A new IMA measurement list format, called Crypto Agile, will be introduced
shortly to take full advantage of the algorithm flexibility of TPM 2.0.
With the new format, it will be possible to provide for each list entry
multiple digests, each calculated with an algorithm supported by the TPM.
Those digests will be used by remote entities to verify the integrity of
the measurements list.

The current (SHA1) and the new (Crypto Agile) format definitions are:

SHA1: pcr[4] digest[20]
   template_name_len[4] template_name[template_name_len]
   template_data_len[4] template_data[template_data_len]

Crypto Agile: pcr[4] total_digest_len[4]
   digest1_len[4] digest1[digest1_len] ...
   digestN_len[4] digestN[digestN_len]
   template_name_len[4] template_name[template_name_len]
   template_data_len[4] template_data[template_data_len]


1 - In this proposed format, how does the parser or consumer of the log
know what algorithm is used for digestN.
For example, the TCG standard format uses TPML_DIGEST_VALUES
uint32_t count - the number of digests TPMT_HA
TPMT_HA digests[]

where a TPMT_HA is
algorithm identifier
digest byte array

2 - Not a criticism, just a question for understanding ...  Would it be 
true that the total_digest_length == the sum of all the digestN_len 
values plus 4 bytes for each length.


Does it determine how many digests there are by when the total length is 
consumed?

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:


Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex

While it does this, there's no need for it to have a RM interface
because what it does between the acquisition and drop of the mutex
can't be seen by or have any effect on userspace (whether it uses the
RM or not).  So currently, the question doesn't arise, which is the
situation you see.


1 - This appears to depend on the RM not releasing the mutex until all 
objects are swapped out.  Correct?  Same for sessions?


2 - A startauthsession can cause a regap error.  Does the above depend 
on the RM doing early regapping so the RM won't see that error?


3 - There's also the problem where the TPM saved session slots 
(typically 64) are full.  My intuition is that the best solution is for 
the RM to reserve 3 slots for the kernel.

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:


Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex

While it does this, there's no need for it to have a RM interface
because what it does between the acquisition and drop of the mutex
can't be seen by or have any effect on userspace (whether it uses the
RM or not).  So currently, the question doesn't arise, which is the
situation you see.


1 - This appears to depend on the RM not releasing the mutex until all 
objects are swapped out.  Correct?  Same for sessions?


2 - A startauthsession can cause a regap error.  Does the above depend 
on the RM doing early regapping so the RM won't see that error?


3 - There's also the problem where the TPM saved session slots 
(typically 64) are full.  My intuition is that the best solution is for 
the RM to reserve 3 slots for the kernel.

Re: [tpmdd-devel] [PATCH v3 2/7] tpm: validate TPM 2.0 commands

[Ken Goldman]

On 3/20/2017 5:54 AM, alexander.stef...@infineon.com wrote:


There are a few special cases that need some thought though. For
example, it is possible to use an upgrade to switch the TPM family
from 1.2 to 2.0 (or vice versa). In this case it seems useful to let
the kernel reinitialize the TPM driver, so it uses the correct
timeouts for communication, activates the correct features (resource
manager or not?), etc., without needing to reboot the system.


In practice, would a TPM upgrade from TPM 1.2 to TPM 2.0 even occur 
without a reboot?  Is it an important use case?


1 - It would leave the SHA-256 PCRs in the reset state.

2 - It's possible that this upgrade would also require a BIOS upgrade.

Re: [tpmdd-devel] [PATCH v3 2/7] tpm: validate TPM 2.0 commands

[Ken Goldman]

On 3/20/2017 5:54 AM, alexander.stef...@infineon.com wrote:


There are a few special cases that need some thought though. For
example, it is possible to use an upgrade to switch the TPM family
from 1.2 to 2.0 (or vice versa). In this case it seems useful to let
the kernel reinitialize the TPM driver, so it uses the correct
timeouts for communication, activates the correct features (resource
manager or not?), etc., without needing to reboot the system.


In practice, would a TPM upgrade from TPM 1.2 to TPM 2.0 even occur 
without a reboot?  Is it an important use case?


1 - It would leave the SHA-256 PCRs in the reset state.

2 - It's possible that this upgrade would also require a BIOS upgrade.

Re: [tpmdd-devel] [PATCH] tpm: Add sysfs interface to show TPM hardware version

[Ken Goldman]

On 3/13/2017 3:10 AM, Peter Huewe wrote:


And yes you are right there is currently no way, except for trial and
error, for the userspace to determine this. So an interface to get
this information makes sense to me.


In practice, I suspect that a single user space application won't
support both TPMs.  It will send the first command, get an error
response code that says it's the wrong TPM, and exit.

Note that, although there is no overlap in the command API, the packet
format is compatible enough that a meaningful response can be returned.

Re: [tpmdd-devel] [PATCH] tpm: Add sysfs interface to show TPM hardware version

[Ken Goldman]

On 3/13/2017 3:10 AM, Peter Huewe wrote:


And yes you are right there is currently no way, except for trial and
error, for the userspace to determine this. So an interface to get
this information makes sense to me.


In practice, I suspect that a single user space application won't
support both TPMs.  It will send the first command, get an error
response code that says it's the wrong TPM, and exit.

Note that, although there is no overlap in the command API, the packet
format is compatible enough that a meaningful response can be returned.

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms

[Ken Goldman]

On 2/26/2017 1:30 PM, Dr. Greg Wettstein wrote:


For example, Ken's tools which come in his TSS2 library, don't work
properly with the 'spaces' device due to the virtualization lifetime.
As an example, the getcapability call will 'lie' about the number of
transient handles which are available through the device.  Attempts to
string multiple transaction sequences together will fail as well.


Two comments:

1 = The intent of the command line tools was for rapid prototyping 
scripts against a SW TPM, and then as sample code for writing the 
application.


2 - If you really want to script against a hardware TPM, it can be done. 
 Simply place a proxy between the TSS and the TPM device driver.  The 
proxy passes commands from the TCP socket to the TPM device driver.  It 
keeps the connection open so the resource manager doesn't flush between 
transactions.


The proxy can be obtained from here.  It's from TPM 1.2 days, but it 
works for TPM 2.0 as well.


https://sourceforge.net/projects/ibmswtpm/files/?source=navbar

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms

[Ken Goldman]

On 2/26/2017 1:30 PM, Dr. Greg Wettstein wrote:


For example, Ken's tools which come in his TSS2 library, don't work
properly with the 'spaces' device due to the virtualization lifetime.
As an example, the getcapability call will 'lie' about the number of
transient handles which are available through the device.  Attempts to
string multiple transaction sequences together will fail as well.


Two comments:

1 = The intent of the command line tools was for rapid prototyping 
scripts against a SW TPM, and then as sample code for writing the 
application.


2 - If you really want to script against a hardware TPM, it can be done. 
 Simply place a proxy between the TSS and the TPM device driver.  The 
proxy passes commands from the TCP socket to the TPM device driver.  It 
keeps the connection open so the resource manager doesn't flush between 
transactions.


The proxy can be obtained from here.  It's from TPM 1.2 days, but it 
works for TPM 2.0 as well.


https://sourceforge.net/projects/ibmswtpm/files/?source=navbar

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/21/2017 1:24 PM, Nayna wrote:


[snip]


1. Take locks.
2. Load transient objects from the backing storage by using ContextLoad
and map virtual handles to physical handles.
3. Perform the transaction.
4. Save transient objects to backing storage by using ContextSave and
map resulting physical handle to virtual handle if there is such.

This commit does not implement virtualization support for hmac and
policy sessions.



If I have understood discussions correctly, I assume, that kernel TPM
operations will also be routed via RM. And I think that is not happening
now with these patches.


There is one corner case that requires kernel operations to go through
the RM, and that's the session context regapping.  If the kernel did not 
go through the RM, it could not handle TPM_RC_CONTEXT_GAP.


I believe that kernel operations such as PCR extend, that don't require
sessions, do not have to go through the RM.

Kernel operations that use objects perhaps can bypass the RM as long as 
there is a lock and the kernel also context saves objects and returns 
the TPM to the empty state before it releases the lock.


Finally, I suspect that the RM should reserve 3 sessions for kernel
operations, so the kernel can't block because user space applications 
have filled all the session slots.

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/21/2017 1:24 PM, Nayna wrote:


[snip]


1. Take locks.
2. Load transient objects from the backing storage by using ContextLoad
and map virtual handles to physical handles.
3. Perform the transaction.
4. Save transient objects to backing storage by using ContextSave and
map resulting physical handle to virtual handle if there is such.

This commit does not implement virtualization support for hmac and
policy sessions.



If I have understood discussions correctly, I assume, that kernel TPM
operations will also be routed via RM. And I think that is not happening
now with these patches.


There is one corner case that requires kernel operations to go through
the RM, and that's the session context regapping.  If the kernel did not 
go through the RM, it could not handle TPM_RC_CONTEXT_GAP.


I believe that kernel operations such as PCR extend, that don't require
sessions, do not have to go through the RM.

Kernel operations that use objects perhaps can bypass the RM as long as 
there is a lock and the kernel also context saves objects and returns 
the TPM to the empty state before it releases the lock.


Finally, I suspect that the RM should reserve 3 sessions for kernel
operations, so the kernel can't block because user space applications 
have filled all the session slots.

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:


Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex


The advantage to context save/ context load over load / flush
is that load requires the parent(s).  The parent chain may be long,
a parent may require authorization, or authorization may be impossible 
because PCRs are no longer in the correct state.


In TPM 1.2, there was a performance difference because load was an 
asymmetric key operation, but it's symmetric in TPM 2.0.



When the kernel needs to use resources that persisted beyond it
dropping the chip->tpm_mutex (say using policy or audit sessions),
then it would need to become a customer of the RM.


BTW, use of an EK private key requires a policy session.

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:


Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex


The advantage to context save/ context load over load / flush
is that load requires the parent(s).  The parent chain may be long,
a parent may require authorization, or authorization may be impossible 
because PCRs are no longer in the correct state.


In TPM 1.2, there was a performance difference because load was an 
asymmetric key operation, but it's symmetric in TPM 2.0.



When the kernel needs to use resources that persisted beyond it
dropping the chip->tpm_mutex (say using policy or audit sessions),
then it would need to become a customer of the RM.


BTW, use of an EK private key requires a policy session.

Re: [tpmdd-devel] [RFC] tpm2-space: add handling for global session exhaustion

[Ken Goldman]

On 2/10/2017 11:46 AM, James Bottomley wrote:

On Fri, 2017-02-10 at 04:03 -0600, Dr. Greg Wettstein wrote:

On Feb 9, 11:24am, James Bottomley wrote:



quote: 810 milliseconds
verify signature: 635 milliseconds

...

Part of the way of reducing the latency is not to use the TPM for
things that don't require secrecy: container signature verification is
one such because the container is signed with a private key to which
...


Agreed.  There are a few times one would verify a signature inside the 
TPM, but they're far from mainstream:


1 - Early in the boot cycle, when there's no crypto library.

2 - When the crypto library doesn't support the required algorithm.

3 - When a ticket is needed to prove to the TPM later that it verified
the signature.

Re: [tpmdd-devel] [RFC] tpm2-space: add handling for global session exhaustion

[Ken Goldman]

On 2/10/2017 11:46 AM, James Bottomley wrote:

On Fri, 2017-02-10 at 04:03 -0600, Dr. Greg Wettstein wrote:

On Feb 9, 11:24am, James Bottomley wrote:



quote: 810 milliseconds
verify signature: 635 milliseconds

...

Part of the way of reducing the latency is not to use the TPM for
things that don't require secrecy: container signature verification is
one such because the container is signed with a private key to which
...


Agreed.  There are a few times one would verify a signature inside the 
TPM, but they're far from mainstream:


1 - Early in the boot cycle, when there's no crypto library.

2 - When the crypto library doesn't support the required algorithm.

3 - When a ticket is needed to prove to the TPM later that it verified
the signature.

Re: [RFC] tpm2-space: add handling for global session exhaustion

[Ken Goldman]

On 1/27/2017 5:04 PM, James Bottomley wrote:


Beware the nasty corner case:

- Application asks for a session and gets 0200

- Time elapses and 0200 gets forcibly flushed

- Later, app comes back, asks for a second session and again gets
0200.

- App gets very confused.

May it be better to close the connection completely, which the
application can detect, than flush a session and give this corner
case?


if I look at the code I've written, I don't know what the session
number is, I just save sessionHandle in a variable for later use (lets
say to v1).  If I got the same session number returned at a later time
and placed it in v2, all I'd notice is that an authorization using v1
would fail.  I'm not averse to killing the entire connection but,
assuming you have fallback, it might be kinder simply to ensure that
the operations with the reclaimed session fail (which is what the code
currently does).


My worry is that this session failure cannot be detected by the 
application.  An HMAC failure could cause the app to tell a user that 
they entered the wrong password.  Misleading.  On the TPM, it could 
trigger the dictionary attack lockout.  For a PIN index, it could 
consume a failure count.  Killing a policy session that has e.g., a 
policy signed term could cause the application to go back to some 
external entity for another authorization signature.


Let's go up to the stack.  What's the attack?

If we're worried about many simultaneous applications (wouldn't that be 
wonderful), why not just let startauthsession fail?  The application can 
just retry periodically.  Just allocate them in triples so there's no 
deadlock.


If we're worried about a DoS attack, killing a session just helps the 
attacker.  The attacker can create a few connections and spin on 
startauthsession, locking everyone out anyway.


~~

Also, let's remember that this is a rare application.  Sessions are only 
needed for remote access (requiring encryption, HMAC or salt), or policy 
sessions.


~~

Should the code also reserve a session for the kernel?  Mark it not 
kill'able?

Re: [RFC] tpm2-space: add handling for global session exhaustion

[Ken Goldman]

On 1/27/2017 5:04 PM, James Bottomley wrote:


Beware the nasty corner case:

- Application asks for a session and gets 0200

- Time elapses and 0200 gets forcibly flushed

- Later, app comes back, asks for a second session and again gets
0200.

- App gets very confused.

May it be better to close the connection completely, which the
application can detect, than flush a session and give this corner
case?


if I look at the code I've written, I don't know what the session
number is, I just save sessionHandle in a variable for later use (lets
say to v1).  If I got the same session number returned at a later time
and placed it in v2, all I'd notice is that an authorization using v1
would fail.  I'm not averse to killing the entire connection but,
assuming you have fallback, it might be kinder simply to ensure that
the operations with the reclaimed session fail (which is what the code
currently does).


My worry is that this session failure cannot be detected by the 
application.  An HMAC failure could cause the app to tell a user that 
they entered the wrong password.  Misleading.  On the TPM, it could 
trigger the dictionary attack lockout.  For a PIN index, it could 
consume a failure count.  Killing a policy session that has e.g., a 
policy signed term could cause the application to go back to some 
external entity for another authorization signature.


Let's go up to the stack.  What's the attack?

If we're worried about many simultaneous applications (wouldn't that be 
wonderful), why not just let startauthsession fail?  The application can 
just retry periodically.  Just allocate them in triples so there's no 
deadlock.


If we're worried about a DoS attack, killing a session just helps the 
attacker.  The attacker can create a few connections and spin on 
startauthsession, locking everyone out anyway.


~~

Also, let's remember that this is a rare application.  Sessions are only 
needed for remote access (requiring encryption, HMAC or salt), or policy 
sessions.


~~

Should the code also reserve a session for the kernel?  Mark it not 
kill'able?

Re: [PATCH v2 1/2] tpm2: add session handle context saving and restoring to the space code

[Ken Goldman]

On 1/27/2017 7:32 PM, James Bottomley wrote:


Sessions are also isolated during each instance of a tpm space.  This
means that spaces shouldn't be able to see each other's sessions and
is enforced by ensuring that a space user may only refer to sessions
handles that are present in their own chip->session_tbl.  Finally when
a space is closed, all the sessions belonging to it should be flushed
so the handles may be re-used by other spaces.


This should be true for transient objects as well.

Re: [PATCH v2 1/2] tpm2: add session handle context saving and restoring to the space code

[Ken Goldman]

On 1/27/2017 7:32 PM, James Bottomley wrote:


Sessions are also isolated during each instance of a tpm space.  This
means that spaces shouldn't be able to see each other's sessions and
is enforced by ensuring that a space user may only refer to sessions
handles that are present in their own chip->session_tbl.  Finally when
a space is closed, all the sessions belonging to it should be flushed
so the handles may be re-used by other spaces.


This should be true for transient objects as well.

[tpmdd-devel] [PATCH v6 2/2] tpm: enhance TPM 2.0 PCR extend to,support multiple banks

[Ken Goldman]

The current TPM 2.0 device driver extends only the SHA1 PCR bank
but the TCG Specification[1] recommends extending all active PCR
banks, to prevent malicious users from setting unused PCR banks with
fake measurements and quoting them.

The existing in-kernel interface(tpm_pcr_extend()) expects only a
SHA1 digest.  To extend all active PCR banks with differing
digest sizes, the SHA1 digest is padded with trailing 0's as needed.

This patch reuses the defined digest sizes from the crypto subsystem,
adding a dependency on CRYPTO_HASH_INFO module.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"


Tested-by: Kenneth Goldman 

I obtained an IMA event log from a Power platform, along with the PCR 10 
value from both the SHA-1 and SHA-256 banks of its Nuvoton TPM 2.0.  I 
independently validated that the event log matches the TPM PCR values.

[tpmdd-devel] [PATCH v6 2/2] tpm: enhance TPM 2.0 PCR extend to,support multiple banks

[Ken Goldman]

The current TPM 2.0 device driver extends only the SHA1 PCR bank
but the TCG Specification[1] recommends extending all active PCR
banks, to prevent malicious users from setting unused PCR banks with
fake measurements and quoting them.

The existing in-kernel interface(tpm_pcr_extend()) expects only a
SHA1 digest.  To extend all active PCR banks with differing
digest sizes, the SHA1 digest is padded with trailing 0's as needed.

This patch reuses the defined digest sizes from the crypto subsystem,
adding a dependency on CRYPTO_HASH_INFO module.

[1] TPM 2.0 Specification referred here is "TCG PC Client Specific
Platform Firmware Profile for TPM 2.0"


Tested-by: Kenneth Goldman 

I obtained an IMA event log from a Power platform, along with the PCR 10 
value from both the SHA-1 and SHA-256 banks of its Nuvoton TPM 2.0.  I 
independently validated that the event log matches the TPM PCR values.

Re: [tpmdd-devel] [PATCH v9 2/2] tpm: add securityfs support,for TPM 2.0 firmware event log

[Ken Goldman]

You do not need to send a new patch set version as long as this
one gets peer tested. And it needs to be tested without hacks
like plumbing TCPA with TPM 2.0 in QEMU. OF code paths needs to
be peer tested to be more specific.

For me the code itself looks good but I simply cannot take it in
in the current situation.

/Jarkko


Tested-by: Kenneth Goldman 

I validated a firmware event log taken from a Power 8 against PCR 0-7 
values for the SHA-1 and SHA-256 banks from a Nuvoton TPM 2.0 chip on 
that same platform.

Re: [tpmdd-devel] [PATCH v9 2/2] tpm: add securityfs support,for TPM 2.0 firmware event log

[Ken Goldman]

You do not need to send a new patch set version as long as this
one gets peer tested. And it needs to be tested without hacks
like plumbing TCPA with TPM 2.0 in QEMU. OF code paths needs to
be peer tested to be more specific.

For me the code itself looks good but I simply cannot take it in
in the current situation.

/Jarkko


Tested-by: Kenneth Goldman 

I validated a firmware event log taken from a Power 8 against PCR 0-7 
values for the SHA-1 and SHA-256 banks from a Nuvoton TPM 2.0 chip on 
that same platform.

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/9/2017 6:16 PM, Jarkko Sakkinen wrote:


Here's my cuts for the kernel:

- Kernel virtualizes handle areas. It's mechanical.
- Kernel does not virtualize bodies. It's not mechanical.
- At least the first version of the RM will not do other than session
  isolation for sessions.


Is it correct that "bodies" are the parameter area of the commands and 
responses?


if so, eventually something should virtualize getcapability.  It may be 
safer in user space, but it can mask RM issues.

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/9/2017 6:16 PM, Jarkko Sakkinen wrote:


Here's my cuts for the kernel:

- Kernel virtualizes handle areas. It's mechanical.
- Kernel does not virtualize bodies. It's not mechanical.
- At least the first version of the RM will not do other than session
  isolation for sessions.


Is it correct that "bodies" are the parameter area of the commands and 
responses?


if so, eventually something should virtualize getcapability.  It may be 
safer in user space, but it can mask RM issues.

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/5/2017 2:20 PM, Jason Gunthorpe wrote:


I'd rather give up features (eg policy sessions, if necessary) for the
unpriv fd than give up security of the unpriv fd.


Please don't give up policy.  Nearly every use case of that we think of 
for TPM 2.0 uses policy sessions.


E.g.,

In 1.2, PCR authorization was built in to the object.  In 2.0, it's a 
policy.


In 1.2, key types were restricted to certain commands.  In 2.0, it's a 
policy.


Then there are all the new use cases - time restricted keys, use count 
restricted keys, keys with a PIN, etc., all use policy.


Even use of the EK primary key requires a policy, and that's needed for 
salt (getting the first password in securely) and attestation (proof 
that the TPM is authentic).

Re: [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/5/2017 2:20 PM, Jason Gunthorpe wrote:


I'd rather give up features (eg policy sessions, if necessary) for the
unpriv fd than give up security of the unpriv fd.


Please don't give up policy.  Nearly every use case of that we think of 
for TPM 2.0 uses policy sessions.


E.g.,

In 1.2, PCR authorization was built in to the object.  In 2.0, it's a 
policy.


In 1.2, key types were restricted to certain commands.  In 2.0, it's a 
policy.


Then there are all the new use cases - time restricted keys, use count 
restricted keys, keys with a PIN, etc., all use policy.


Even use of the EK primary key requires a policy, and that's needed for 
salt (getting the first password in securely) and attestation (proof 
that the TPM is authentic).

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/3/2017 4:47 PM, Jason Gunthorpe wrote:


I think we should also consider TPM 1.2 support in all of this, it is
still a very popular piece of hardware and it is equally able to
support a RM.


I suspect that TPM 2.0 and TPM 1.2 are so different that there may be 
little or no code in common.


My immediate need is for a 2.0 resource manager, since it's a gap in the 
technology, while 1.2 does have tcsd.

Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

[Ken Goldman]

On 1/3/2017 4:47 PM, Jason Gunthorpe wrote:


I think we should also consider TPM 1.2 support in all of this, it is
still a very popular piece of hardware and it is equally able to
support a RM.


I suspect that TPM 2.0 and TPM 1.2 are so different that there may be 
little or no code in common.


My immediate need is for a 2.0 resource manager, since it's a gap in the 
technology, while 1.2 does have tcsd.