linux-kernel - RE: [RFC PATCH v1 2/2] ufs: core: Add advanced RPMB support in ufs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DM6PR04MB657570B147A1D3B6FE5F8797FC3F9@DM6PR04MB6575.namprd04.prod.outlook.com>
Date:   Tue, 8 Nov 2022 21:00:31 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     Bean Huo <beanhuo@...pp.de>,
        "alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
        "jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "stanley.chu@...iatek.com" <stanley.chu@...iatek.com>,
        "beanhuo@...ron.com" <beanhuo@...ron.com>,
        "bvanassche@....org" <bvanassche@....org>,
        "tomas.winkler@...el.com" <tomas.winkler@...el.com>,
        "daejun7.park@...sung.com" <daejun7.park@...sung.com>,
        "quic_cang@...cinc.com" <quic_cang@...cinc.com>,
        "quic_nguyenb@...cinc.com" <quic_nguyenb@...cinc.com>,
        "quic_xiaosenh@...cinc.com" <quic_xiaosenh@...cinc.com>,
        "quic_richardp@...cinc.com" <quic_richardp@...cinc.com>,
        "quic_asutoshd@...cinc.com" <quic_asutoshd@...cinc.com>,
        "hare@...e.de" <hare@...e.de>
CC:     "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH v1 2/2] ufs: core: Add advanced RPMB support in
 ufs_bsg

> Avri,
> thanks for your comments and review.
> 
> 
> On Tue, 2022-11-08 at 19:09 +0000, Avri Altman wrote:
> > > Add advanced RPMB support in ufs_bsg. For these reasons, we try to
> > > implement Advanced RPMB in ufs_bsg:
> > > 1. According to the UFS specification, only one RPMB operation can
> > > be performed at any time. We can ensure this by using reserved slot
> > > and its dev_cmd sync operation protection mechanism.
> >
> > Regardless of its technical convenience, this approach unfortunately
> > breaks the spec.
> >
> > The spec say (please note the line numbers):
> >
> > ".....
> >
> > 5197 12.4.5.1 Advanced RPMB Message
> >
> > 5198 An Advanced RPMB Message is composed of an Advanced RPMB
> Meta
> > Information and a MAC/KEY in
> >
> > 5199 the EHS field in *COMMAND UPIU* and *RESPONSE UPIU*. Advanced
> > RPMB Data is delivered through
> >
> > ....."
> 
> > Moreover, in the examples that are provided, it is still expected to
> > be carried via SECURITY PROTOCOL IN and SECURITY PROTOCOL OUT,
> >
> > See e.g. Figure 12.15 — Authenticated Data Write Flow (in Advanced
> > RPMB Mode).
> >
> >
> not quite get what you meant here.
I meant that we should still build upius that contains security protocol commands.
 
> >
> 
> >
> > Therefore, wrapping the rpmb packets in a query-request upiu and
> > query-response upiu is not allowed.
> >
> >
> 
> no, I didn't wrap RPMB packet in query-request/response, it is inupiu_req
> and upiu_rsp, it is upiu command.
I was thinking you are using a query request upiu because you are using device management commands flow,
And specifically, I didn't see where you are setting ucd_req_ptr->sc.cdb,
Which should hold the security protocol cdb.

> Based on Bart's suggestion, we shouldn't
> change the current ufs_bsg structure. I think his concern is that if we change
> ufs_bsg structure, the user space tool also needs to change as well.
Adding command upiu support will not break any ufs-bsg tools, e.g. ufs-utils.
It would just add this capability.

Thanks,
Avri

> 
> >
> > Still, I agree that the approach you suggested, namely to rely on the
> > ufs-bsg driver, is the cleanest way to handle the advance rpmb access.
> >
> > However, IMHO, you need to do it is by adding command UPIU to the
> > ufs-bsg driver.
> >
> >
> 
> Yes, agree with you on this point. But we still need to use reserved slots for
> RPMB or command UPIU, we don't want to affect IO requests on the normal
> path.
> 
> One problem is that we didn't split the dev_manage command and the
> RPMB command in their completion handlers. I would like to change
> dev_man to passthrough or something else, and then split dev_man and
> RPMB, otherwise, they would be mixed in one dev_man completion handler.
> No technical issues here, just want to make it more readable and
> maintainable.
> 
> 
> 
> Kind regards,
> Bean
> 
> >
> > Thanks,
> >
> > Avri