RE: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

RE: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Winkler, Tomas]

>>> wrote:
>>> > Few storage technology such is EMMC, UFS, and NVMe support RPMB
>>> >hardware partition with common protocol and frame layout.
>>> > The RPMB partition cannot be accessed via standard block layer, but
>>> >by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
>>> >PROGRAM_KEY.
>>> >...
>>>
>>> If the same protocol is used by all these standards, why not export it directly
>>> (including the RESULT_READ command or not even knowing the command
>>> types)?
>> The protocol is the  same, but the wrapping of the packets is storage type specific so
>> I believe that the best abstraction is on those  4 operations level. I'm not sure if the code would
>> be simpler if it would be exposed on a lower level.
>
> I disagree. With the two storage types you support, the packets are
> identical. The only difference is the low level commands you use to
> send and receive them.
The packets are identical, but there are these little settings you need to be aware of
like set argument  31 in CMD23 for WRITE and PROGRAM_KEY and for others not so
the storage type transparency is lost, actually the UFS protocol is a bit cleaner in that sense.
I know it can be ironed somehow, but this just stresses the point. 

>> RESULT_READ  is  a command to be issued for getting preceeding write operation status, it's a kind of work around about the fact that you have to issue a read operation
>> in order to retrieve data in this case a  write operation result.  It can be successfully hidden and final result of the operation is delivered
>> to the user.
>>
>
> Yes, it is possible to hide the result read command, but that does not
> mean you should. The rpmb protocol is designed to let two endpoints
> communicate in a way that lets them detect tampering. While the packet
> you inject does not contain any protected data, you can still view it
> as a form of tampering. 

I’m not seeing it like injecting packets, those are merely commands embedded in the packets that need to be performed in order to complete the operation
and there is not security value in them, the only end to end protection is truly given by the MAC verification and this is preserved. 

>If a future rpmb protocol version adds
> features, you could loose the ability to inject packets.

The code will have to change anyhow.  

>
>>> While I would prefer an rpmb specific interface over the existing raw
>>> mmc command interface, all I need is an rpmb operation that lets me send
>>> and receive buffers without interruption.
>>
>> I currently don't see an obstacle on doing the same with proposed interface, what is the issue are seeing.
>>
>
> The main issue is that you are injecting commands, so code that
> follows the mmc spec will not work.

Yes, it should be storage type independent, not mmc spec dependant. 
The interface reduces the number of data passed between user space and kernel, the device power management is simpler. 

>
>>> You can find our exiting user-space code here at
>>> https://android.googlesource.com/platform/system/core/+/master/trusty/s
>>> torage/proxy/rpmb.c.
>>> If you use an interface more similar to this, I think your emmc and ufs
>>> specific code would be simpler. Also, if you don't need the in-kernel
>>> interface, the kernel would not need to know the details of the rpmb
>>> protocol at all.
>>
>> My major interest is the in-kernel protocol the user space API was more intended for debug, but I found it would be even more useful.
>> The store type  access is very RPMB specific  for both MMC and UFS and needs to do special operations for RPMB so I don't see how this awareness can be removed or I'm not reading your proposal correctly.
>
> The interface we use specifies reliable-write, write and read
> operations on an rpmb partition. I don't think you need to know more
> than this in either mmc or ufs. I have not seen the ufs spec, but
> based on your code it looks like reliable-write and write can map to
> the same command there.
Yes, the interface can be also abstract on let’s call it raw rpmb packet read/write level, but I didn’t see the value in at the time as RPMB operations and the steps are well defined.
Maybe there is a place for to support for fing grained access or at leasting adding RESULT_READ command as well,  for the usecases like yours.

>> Accessing RPMB is a multiple stepsoperation, the steps can be driven from the user space as done in EMMC ioctl but hidning would reduce the number of system calls and amount of data passed,
>> in some sense the same as in the  new mmc  MMC_IOC_MULTI_CMD is trying ot achive.
>>
> The main purpose of using the MMC_IOC_MULTI_CMD is not to reduce the
> number of syscalls. It is to prevent other mmc operations from getting
> interleaved with the rpmb packets. Some emmc chips will only respond
> error packet if any other partitions are read from between the write
> and read operation on the rpmb partition.

I didn’t encountered as our interface doesn’t suffer form that issue but yes this just another reason to use the new rpmb interface
>
>>> I have not tested your code, but it looks like we would have to modify the
>>> storage proxy to interpret the data it currently passes through and remove all
>>> RESULT_READ packets.
>> Your proxy driver just sends ioctls so this wouldn't be much difference and the rpmb code on the trusty w need rewrite just rpmb_send () function,
>> actually removing one set of buffer size. I will try to make that modification if it helps?
>>
>
> No I don't want you to modify the code that runs in the secure OS.
> This would require additional code in boot loaders and proxy servers
> using the existing MMC_IOC_MULTI_CMD command as they too would have to
> interpret the packets to inject RESULT_READ packets.

I’ve looked at the proxy and and secure OS, the rewirte is not so hard, though.
I really cannot figure to boo loader and secure OS interactions, you have this notion of TP and TDEA ports but those are not used from the secure OS. 
Is there a software you can point me to? 

Second if it won’t be possible to use the current implementation if the storage type change UFS or NVMe anyhow and on the othernad
I’m not suggesting to kill MMC ioctl, so this won’t be breackages of the existing software. 

In bottom line I will try to add raw read/write access to RPMB to support fine grained access and see if you can work with that.


I really appreciate your feedback.
Thanks
Tomas 

Re: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Arve Hjønnevåg]

On Tue, Jun 14, 2016 at 2:05 PM, Winkler, Tomas <tomas.winkler@intel.com> wrote:
>>>> wrote:
>>>> > Few storage technology such is EMMC, UFS, and NVMe support RPMB
>>>> >hardware partition with common protocol and frame layout.
>>>> > The RPMB partition cannot be accessed via standard block layer, but
>>>> >by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
>>>> >PROGRAM_KEY.
>>>> >...
>>>>
>>>> If the same protocol is used by all these standards, why not export it directly
>>>> (including the RESULT_READ command or not even knowing the command
>>>> types)?
>>> The protocol is the  same, but the wrapping of the packets is storage type specific so
>>> I believe that the best abstraction is on those  4 operations level. I'm not sure if the code would
>>> be simpler if it would be exposed on a lower level.
>>
>> I disagree. With the two storage types you support, the packets are
>> identical. The only difference is the low level commands you use to
>> send and receive them.
> The packets are identical, but there are these little settings you need to be aware of
> like set argument  31 in CMD23 for WRITE and PROGRAM_KEY and for others not so
> the storage type transparency is lost, actually the UFS protocol is a bit cleaner in that sense.
> I know it can be ironed somehow, but this just stresses the point.
>

Bit 31 selects reliable write. Your driver does not need to know the
command number to set this correctly if the client specifies if a
reliable write is needed.

>>> RESULT_READ  is  a command to be issued for getting preceeding write operation status, it's a kind of work around about the fact that you have to issue a read operation
>>> in order to retrieve data in this case a  write operation result.  It can be successfully hidden and final result of the operation is delivered
>>> to the user.
>>>
>>
>> Yes, it is possible to hide the result read command, but that does not
>> mean you should. The rpmb protocol is designed to let two endpoints
>> communicate in a way that lets them detect tampering. While the packet
>> you inject does not contain any protected data, you can still view it
>> as a form of tampering.
>
> I’m not seeing it like injecting packets, those are merely commands embedded in the packets that need to be performed in order to complete the operation
> and there is not security value in them, the only end to end protection is truly given by the MAC verification and this is preserved.

You are sending an extra packet to send that command.

>
>>If a future rpmb protocol version adds
>> features, you could loose the ability to inject packets.
>
> The code will have to change anyhow.
>

Why would your code need to change? If your code does not inspect and
inject packets, the data in those packets can change independently
from your code.

>>
>>>> While I would prefer an rpmb specific interface over the existing raw
>>>> mmc command interface, all I need is an rpmb operation that lets me send
>>>> and receive buffers without interruption.
>>>
>>> I currently don't see an obstacle on doing the same with proposed interface, what is the issue are seeing.
>>>
>>
>> The main issue is that you are injecting commands, so code that
>> follows the mmc spec will not work.
>
> Yes, it should be storage type independent, not mmc spec dependant.
> The interface reduces the number of data passed between user space and kernel, the device power management is simpler.
>

>From what I can tell we have multiple storage types that use mostly
compatible specs, but you chose to "simplify" it. I strongly suggest
passing the existing protocol through as-is, and only perform packet
inspection for incompatible storage types (if or when they exist).

>>
>>>> You can find our exiting user-space code here at
>>>> https://android.googlesource.com/platform/system/core/+/master/trusty/s
>>>> torage/proxy/rpmb.c.
>>>> If you use an interface more similar to this, I think your emmc and ufs
>>>> specific code would be simpler. Also, if you don't need the in-kernel
>>>> interface, the kernel would not need to know the details of the rpmb
>>>> protocol at all.
>>>
>>> My major interest is the in-kernel protocol the user space API was more intended for debug, but I found it would be even more useful.
>>> The store type  access is very RPMB specific  for both MMC and UFS and needs to do special operations for RPMB so I don't see how this awareness can be removed or I'm not reading your proposal correctly.
>>
>> The interface we use specifies reliable-write, write and read
>> operations on an rpmb partition. I don't think you need to know more
>> than this in either mmc or ufs. I have not seen the ufs spec, but
>> based on your code it looks like reliable-write and write can map to
>> the same command there.
> Yes, the interface can be also abstract on let’s call it raw rpmb packet read/write level, but I didn’t see the value in at the time as RPMB operations and the steps are well defined.
> Maybe there is a place for to support for fing grained access or at leasting adding RESULT_READ command as well,  for the usecases like yours.
>
>>> Accessing RPMB is a multiple stepsoperation, the steps can be driven from the user space as done in EMMC ioctl but hidning would reduce the number of system calls and amount of data passed,
>>> in some sense the same as in the  new mmc  MMC_IOC_MULTI_CMD is trying ot achive.
>>>
>> The main purpose of using the MMC_IOC_MULTI_CMD is not to reduce the
>> number of syscalls. It is to prevent other mmc operations from getting
>> interleaved with the rpmb packets. Some emmc chips will only respond
>> error packet if any other partitions are read from between the write
>> and read operation on the rpmb partition.
>
> I didn’t encountered as our interface doesn’t suffer form that issue but yes this just another reason to use the new rpmb interface

The MMC_IOC_MULTI_CMD solves that problem and is already merged.

>>
>>>> I have not tested your code, but it looks like we would have to modify the
>>>> storage proxy to interpret the data it currently passes through and remove all
>>>> RESULT_READ packets.
>>> Your proxy driver just sends ioctls so this wouldn't be much difference and the rpmb code on the trusty w need rewrite just rpmb_send () function,
>>> actually removing one set of buffer size. I will try to make that modification if it helps?
>>>
>>
>> No I don't want you to modify the code that runs in the secure OS.
>> This would require additional code in boot loaders and proxy servers
>> using the existing MMC_IOC_MULTI_CMD command as they too would have to
>> interpret the packets to inject RESULT_READ packets.
>
> I’ve looked at the proxy and and secure OS, the rewirte is not so hard, though.
> I really cannot figure to boo loader and secure OS interactions, you have this notion of TP and TDEA ports but those are not used from the secure OS.
> Is there a software you can point me to?

I don't have any bootloader code to share, if that is what you are
asking for. The goal is to have access to the rpmb data before loading
Linux. For this to work, the bootloader has to implement the rpmb
proxy operations (which should be as simple to implement as possible).
I don't want to maintain two protocols where the bootloader and
old-linux clients use the full rpmb protocol and new linux clients use
your rpmb protocol.

>
> Second if it won’t be possible to use the current implementation if the storage type change UFS or NVMe anyhow and on the othernad
> I’m not suggesting to kill MMC ioctl, so this won’t be breackages of the existing software.

I don't think you need to be compatible with existing Linux user-space
code, but it would be possible by emulating the MMC_IOC_MULTI_CMD
command. You should try to be compatible with existing secure os
interfaces though, as there is no reason for this to be different for
ufs and emmc.

>
> In bottom line I will try to add raw read/write access to RPMB to support fine grained access and see if you can work with that.
>

Why not change your code to be compatible with code written against
the existing specs instead? I don't see a need for an interface where
the client has to prepare all the packets defined by the spec except
for one special packet that the kernel will inject.

>
> I really appreciate your feedback.
> Thanks
> Tomas
>
>
>
>



-- 
Arve Hjønnevåg

Re: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Arve Hjønnevåg]

On Thu, Jun 2, 2016 at 6:17 AM, Winkler, Tomas <tomas.winkler@intel.com> wrote:
>>
>> On Wed, Jun 1, 2016 at 2:41 PM, Tomas Winkler <tomas.winkler@intel.com>
>> wrote:
>> > Few storage technology such is EMMC, UFS, and NVMe support RPMB
>> >hardware partition with common protocol and frame layout.
>> > The RPMB partition cannot be accessed via standard block layer, but
>> >by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
>> >PROGRAM_KEY.
>> >...
>>
>> If the same protocol is used by all these standards, why not export it directly
>> (including the RESULT_READ command or not even knowing the command
>> types)?
> The protocol is the  same, but the wrapping of the packets is storage type specific so
> I believe that the best abstraction is on those  4 operations level. I'm not sure if the code would
> be simpler if it would be exposed on a lower level.

I disagree. With the two storage types you support, the packets are
identical. The only difference is the low level commands you use to
send and receive them.

> RESULT_READ  is  a command to be issued for getting preceeding write operation status, it's a kind of work around about the fact that you have to issue a read operation
> in order to retrieve data in this case a  write operation result.  It can be successfully hidden and final result of the operation is delivered
> to the user.
>

Yes, it is possible to hide the result read command, but that does not
mean you should. The rpmb protocol is designed to let two endpoints
communicate in a way that lets them detect tampering. While the packet
you inject does not contain any protected data, you can still view it
as a form of tampering. If a future rpmb protocol version adds
features, you could loose the ability to inject packets.

>> While I would prefer an rpmb specific interface over the existing raw
>> mmc command interface, all I need is an rpmb operation that lets me send
>> and receive buffers without interruption.
>
> I currently don't see an obstacle on doing the same with proposed interface, what is the issue are seeing.
>

The main issue is that you are injecting commands, so code that
follows the mmc spec will not work.

>> You can find our exiting user-space code here at
>> https://android.googlesource.com/platform/system/core/+/master/trusty/s
>> torage/proxy/rpmb.c.
>> If you use an interface more similar to this, I think your emmc and ufs
>> specific code would be simpler. Also, if you don't need the in-kernel
>> interface, the kernel would not need to know the details of the rpmb
>> protocol at all.
>
> My major interest is the in-kernel protocol the user space API was more intended for debug, but I found it would be even more useful.
> The store type  access is very RPMB specific  for both MMC and UFS and needs to do special operations for RPMB so I don't see how this awareness can be removed or I'm not reading your proposal correctly.

The interface we use specifies reliable-write, write and read
operations on an rpmb partition. I don't think you need to know more
than this in either mmc or ufs. I have not seen the ufs spec, but
based on your code it looks like reliable-write and write can map to
the same command there.

> Accessing RPMB is a multiple stepsoperation, the steps can be driven from the user space as done in EMMC ioctl but hidning would reduce the number of system calls and amount of data passed,
> in some sense the same as in the  new mmc  MMC_IOC_MULTI_CMD is trying ot achive.
>
The main purpose of using the MMC_IOC_MULTI_CMD is not to reduce the
number of syscalls. It is to prevent other mmc operations from getting
interleaved with the rpmb packets. Some emmc chips will only respond
error packet if any other partitions are read from between the write
and read operation on the rpmb partition.

>> I have not tested your code, but it looks like we would have to modify the
>> storage proxy to interpret the data it currently passes through and remove all
>> RESULT_READ packets.
> Your proxy driver just sends ioctls so this wouldn't be much difference and the rpmb code on the trusty w need rewrite just rpmb_send () function,
> actually removing one set of buffer size. I will try to make that modification if it helps?
>

No I don't want you to modify the code that runs in the secure OS.
This would require additional code in boot loaders and proxy servers
using the existing MMC_IOC_MULTI_CMD command as they too would have to
interpret the packets to inject RESULT_READ packets.

> Thanks for interest and your review.
> Tomas
>
>



-- 
Arve Hjønnevåg

RE: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Winkler, Tomas]

> 
> On Wed, Jun 1, 2016 at 2:41 PM, Tomas Winkler <tomas.winkler@intel.com>
> wrote:
> > Few storage technology such is EMMC, UFS, and NVMe support RPMB
> >hardware partition with common protocol and frame layout.
> > The RPMB partition cannot be accessed via standard block layer, but
> >by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
> >PROGRAM_KEY.
> >...
> 
> If the same protocol is used by all these standards, why not export it directly
> (including the RESULT_READ command or not even knowing the command
> types)?
The protocol is the  same, but the wrapping of the packets is storage type specific so 
I believe that the best abstraction is on those  4 operations level. I'm not sure if the code would
be simpler if it would be exposed on a lower level.  
RESULT_READ  is  a command to be issued for getting preceeding write operation status, it's a kind of work around about the fact that you have to issue a read operation
in order to retrieve data in this case a  write operation result.  It can be successfully hidden and final result of the operation is delivered
to the user.

> While I would prefer an rpmb specific interface over the existing raw
> mmc command interface, all I need is an rpmb operation that lets me send
> and receive buffers without interruption.

I currently don't see an obstacle on doing the same with proposed interface, what is the issue are seeing.

> You can find our exiting user-space code here at
> https://android.googlesource.com/platform/system/core/+/master/trusty/s
> torage/proxy/rpmb.c.
> If you use an interface more similar to this, I think your emmc and ufs
> specific code would be simpler. Also, if you don't need the in-kernel
> interface, the kernel would not need to know the details of the rpmb
> protocol at all.

My major interest is the in-kernel protocol the user space API was more intended for debug, but I found it would be even more useful. 
The store type  access is very RPMB specific  for both MMC and UFS and needs to do special operations for RPMB so I don't see how this awareness can be removed or I'm not reading your proposal correctly.
Accessing RPMB is a multiple stepsoperation, the steps can be driven from the user space as done in EMMC ioctl but hidning would reduce the number of system calls and amount of data passed, 
in some sense the same as in the  new mmc  MMC_IOC_MULTI_CMD is trying ot achive.

> I have not tested your code, but it looks like we would have to modify the
> storage proxy to interpret the data it currently passes through and remove all
> RESULT_READ packets.
Your proxy driver just sends ioctls so this wouldn't be much difference and the rpmb code on the trusty w need rewrite just rpmb_send () function,
actually removing one set of buffer size. I will try to make that modification if it helps?

Thanks for interest and your review. 
Tomas 
 

Re: [PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Arve Hjønnevåg]

On Wed, Jun 1, 2016 at 2:41 PM, Tomas Winkler <tomas.winkler@intel.com> wrote:
> Few storage technology such is EMMC, UFS, and NVMe support RPMB
> hardware partition with common protocol and frame layout.
> The RPMB partition cannot be accessed via standard block layer, but
> by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
> PROGRAM_KEY.
>...

If the same protocol is used by all these standards, why not export it
directly (including the RESULT_READ command or not even knowing the
command types)? While I would prefer an rpmb specific interface over
the existing raw mmc command interface, all I need is an rpmb
operation that lets me send and receive buffers without interruption.
You can find our exiting user-space code here at
https://android.googlesource.com/platform/system/core/+/master/trusty/storage/proxy/rpmb.c.
If you use an interface more similar to this, I think your emmc and
ufs specific code would be simpler. Also, if you don't need the
in-kernel interface, the kernel would not need to know the details of
the rpmb protocol at all.

I have not tested your code, but it looks like we would have to modify
the storage proxy to interpret the data it currently passes through
and remove all RESULT_READ packets.

-- 
Arve Hjønnevåg

[PATCH v4 0/8] Replay Protected Memory Block (RPMB) subsystem

[Tomas Winkler]

Few storage technology such is EMMC, UFS, and NVMe support RPMB
hardware partition with common protocol and frame layout.
The RPMB partition cannot be accessed via standard block layer, but
by a set of specific commands: WRITE, READ, GET_WRITE_COUNTER, and
PROGRAM_KEY.
Such a partition provides authenticated and replay protected access,
hence suitable as a secure storage.

A storage device registers its RPMB hardware (emmc) partition or
RPMB W-LUN (ufs) with the RPMB layer providing an implementation for
send_rpmb_req() handler.
There is as well simulation platform device. This is handy as an RPMB
key can be programmed only once at storage device lifetime.

The RPMB layer aims to provide in-kernel API for Trusted Execution
Environment (TEE) devices that are capable to securely compute block
frame signature. In case a TEE device wish to store a replay protected
data, it creates an RPMB frame with requested data and computes HMAC of
the frame, then it requests the storage device via RPMB layer to store
the data.
A TEE driver can claim rpmb interface, for example,
via  class_interface_register ().

A parallel user space API is provided via /dev/rpmbX character
device with a single IOCTL command similar to the one provided by
mmc/ioctl. h
This API may help applications such as 
https://android.googlesource.com/trusty/app/storage/
To be obliviose to the underlaying HW storage technology.

There is a sample tool under tools/rpmb/ directory that exercises
this interface.

Tomas Winkler (8):
  rpmb: add Replay Protected Memory Block (RPMB) subsystem
  char: rpmb: add sysfs-class ABI documentation
  char: rpmb: add device attributes
  char: rpmb: provide user space interface
  char: rpmb: add RPMB simulation device
  tools rpmb: add RPBM access tool
  mmc: block: register RPMB partition with the RPMB subsystem
  scsi: ufs: connect to RPMB subsystem

 Documentation/ABI/testing/sysfs-class-rpmb |  44 ++
 Documentation/ioctl/ioctl-number.txt       |   1 +
 MAINTAINERS                                |  10 +
 drivers/char/Kconfig                       |   2 +
 drivers/char/Makefile                      |   1 +
 drivers/char/rpmb/Kconfig                  |  25 +
 drivers/char/rpmb/Makefile                 |   6 +
 drivers/char/rpmb/cdev.c                   | 207 +++++++
 drivers/char/rpmb/core.c                   | 408 ++++++++++++++
 drivers/char/rpmb/rpmb-cdev.h              |  31 ++
 drivers/char/rpmb/rpmb_sim.c               | 589 ++++++++++++++++++++
 drivers/mmc/card/Kconfig                   |   1 +
 drivers/mmc/card/block.c                   | 292 ++++++++++
 drivers/scsi/ufs/Kconfig                   |   1 +
 drivers/scsi/ufs/ufshcd.c                  | 219 ++++++++
 drivers/scsi/ufs/ufshcd.h                  |   2 +
 include/linux/rpmb.h                       | 138 +++++
 include/uapi/linux/rpmb.h                  | 120 ++++
 tools/Makefile                             |  14 +-
 tools/rpmb/.gitignore                      |   2 +
 tools/rpmb/Makefile                        |  32 ++
 tools/rpmb/rpmb.c                          | 862 +++++++++++++++++++++++++++++
 22 files changed, 3002 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-rpmb
 create mode 100644 drivers/char/rpmb/Kconfig
 create mode 100644 drivers/char/rpmb/Makefile
 create mode 100644 drivers/char/rpmb/cdev.c
 create mode 100644 drivers/char/rpmb/core.c
 create mode 100644 drivers/char/rpmb/rpmb-cdev.h
 create mode 100644 drivers/char/rpmb/rpmb_sim.c
 create mode 100644 include/linux/rpmb.h
 create mode 100644 include/uapi/linux/rpmb.h
 create mode 100644 tools/rpmb/.gitignore
 create mode 100644 tools/rpmb/Makefile
 create mode 100644 tools/rpmb/rpmb.c

-- 
2.5.5