{"action":"created","issue":{"url":"https://api.github.com/repos/OP-TEE/optee_os/issues/296","labels_url":"https://api.github.com/repos/OP-TEE/optee_os/issues/296/labels{/name}","comments_url":"https://api.github.com/repos/OP-TEE/optee_os/issues/296/comments","events_url":"https://api.github.com/repos/OP-TEE/optee_os/issues/296/events","html_url":"https://github.com/OP-TEE/optee_os/issues/296","id":75410861,"number":296,"title":"Connecting RPMB to the storage APIs","user":{"login":"youssefesmat","id":11986363,"avatar_url":"https://avatars.githubusercontent.com/u/11986363?v=3","gravatar_id":"","url":"https://api.github.com/users/youssefesmat","html_url":"https://github.com/youssefesmat","followers_url":"https://api.github.com/users/youssefesmat/followers","following_url":"https://api.github.com/users/youssefesmat/following{/other_user}","gists_url":"https://api.github.com/users/youssefesmat/gists{/gist_id}","starred_url":"https://api.github.com/users/youssefesmat/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/youssefesmat/subscriptions","organizations_url":"https://api.github.com/users/youssefesmat/orgs","repos_url":"https://api.github.com/users/youssefesmat/repos","events_url":"https://api.github.com/users/youssefesmat/events{/privacy}","received_events_url":"https://api.github.com/users/youssefesmat/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-05-12T00:57:09Z","updated_at":"2015-05-22T02:13:04Z","closed_at":null,"body":"Hello,\r\n\r\nI have been working with Paul (paswan-ms) on RPMB support in OP-TEE. The following is a proposal for RPMB support that builds upon the encrypted FS changes (pull request 231).\r\n\r\n----------\r\n\r\n<h3><b>Connecting the APIs</b></h3>\r\n\r\nThe proposal here is to add another tee_file_operations table that can be updated by the platform.\r\nToday, the tee_svc_storage_XX APIs (tee_svc_storage.c) call the lower level APIs using the tee_file_ops function table. This table was changed with the security changes to call the tee_enc_fs_XX APIs. Internally the tee_enc_fs_XX APIs call the lower level tee_common_fs_XX APIs.\r\n\r\nThis change will add another table that the tee_enc_fs_XX APIs can use to call the lower layer physical storage APIs called tee_phy_file_ops. Thus, instead of tee_enc_fs_XX APIs calling tee_common_fs directly they will call tee_phy_file_ops.XX.\r\n\r\nThe table will be prototyped in tee_fs.h as follows:\r\n\r\n    extern struct tee_file_operations tee_phy_file_ops;\r\n\r\nThe table will be defined in tee_ fs_common.c in the following way:\r\n\r\n    #if ! defined(CFG_PHY_FS)\r\n    struct tee_file_operations tee_phy_file_ops = {\r\n    \t.open = tee_fs_common_open,\r\n    \t.close = tee_fs_common_close,\r\n    \t.read = tee_ fs_common_read,\r\n    \t.write = tee_ fs_common_write,\r\n    \t.lseek = tee_ fs_common_lseek,\r\n    \t.ftruncate = tee_ fs_common_ftruncate,\r\n    \t.rename = tee_fs_common_rename,\r\n    \t.unlink = tee_fs_common_unlink,\r\n    \t.mkdir = tee_fs_common_mkdir,\r\n    \t.opendir = tee_fs_common_opendir,\r\n    \t.closedir = tee_fs_common_closedir,\r\n    \t.readdir = tee_fs_common_readdir,\r\n    \t.rmdir = tee_fs_common_rmdir,\r\n    \t.access = tee_fs_common_access,\r\n    \t.stat = tee_fs_common_stat //needs to be added\r\n    };\r\n    #endif\r\n \r\nAnd tee_file_ops in tee_enc_fs.c will be changed to:\r\n\r\n    struct tee_file_operations tee_file_ops = {\r\n    \t.open = tee_enc_fs_open,\r\n    \t.close = tee_enc_fs_close,\r\n    \t.read = tee_enc_fs_read,\r\n    \t.write = tee_enc_fs_write,\r\n    \t.lseek = tee_enc_fs_lseek,\r\n    \t.ftruncate = tee_enc_fs_ftruncate,\r\n    \t.rename = tee_phy_file_ops.rename,\r\n    \t.unlink = tee_phy_file_ops.unlink,\r\n    \t.mkdir = tee_phy_file_ops.mkdir,\r\n    \t.opendir = tee_phy_file_ops.opendir,\r\n    \t.closedir = tee_phy_file_ops.closedir,\r\n    \t.readdir = tee_phy_file_ops.readdir,\r\n    \t.rmdir = tee_phy_file_ops.rmdir,\r\n    \t.access = tee_phy_file_ops.access,\r\n    \t.stat = tee_phy_file_ops.stat\r\n    };\r\nThe platform can then define CFG_PHY_FS and define the table in platform specific files. In our case the platform will provide another definition for the table pointing to the RPMB APIs. Such as the following:\r\n\r\n    struct tee_file_operations tee_phy_file_ops = {\r\n    \t.open = tee_rpmb_fs_open,\r\n    \t.close = tee_rpmb_fs_close,\r\n    \t.read = tee_ rpmb_fs_read,\r\n    \t.write = tee_ rpmb_fs_write,\r\n    \t.lseek = tee_ rpmb_fs_lseek,\r\n    \t.ftruncate = tee_ rpmb_fs_ftruncate,\r\n    \t.rename = tee_ rpmb_fs_rename,\r\n    \t.unlink = tee_ rpmb_fs_unlink,\r\n    \t.mkdir = tee_ rpmb_fs_mkdir,\r\n    \t.opendir = tee_ rpmb_fs_opendir,\r\n    \t.closedir = tee_ rpmb_fs_closedir,\r\n    \t.readdir = tee_ rpmb_fs_readdir,\r\n    \t.rmdir = tee_ rpmb_fs_rmdir,\r\n    \t.access = tee_ rpmb_fs_access,\r\n    \t.stat = tee_rpmb_fs_stat\r\n    };\r\n\r\n<h3><b>RPMB APIs</b></h3>\r\n\r\nThe current implementation of RPMB implements a FAT like table that stores the file name, location and size in the RPMB block. The FAT table sits in the beginning of the RRMB block.\r\n\r\n<h4><b>Directory Support</b></h4>\r\n\r\nDirectory support is missing from the current RPMB implementation that needs to be added to support calls from the higher level encryption APIs. In OPTEE today there is only a one-level deep directory hierarchy defined as TA_ID/OBJECT_ID. There is a directory for every TA that contains the objects that that TA owns.\r\n\r\n<h4><b>Short Term Solution</b></h4>\r\nThe quickest short term solution to add “directory” support to the current implementation of RPMB is to store the full path as the name of the file in the FAT table. The FAT table supports a maximum file name of 48 characters. The object ID maximum length is 64 character, plus the size of the UUID for the TA is 16 plus one for the directory divisor character would require us to change the maximum file size to 81 characters or to (sizeof(TEE_UUID) + TEE_OBJECT_ID_MAX_LEN + 1).\r\n\r\nWith this approach the RPMB APIs that need to be implemented will be as follows:\r\n\r\n- **Open:** This API will look through the FAT for the file with full path in question and return a file descriptor representing the file. A new FAT entry can be created for the file if it does not already exist. This can be the index into the FAT table or can be implemented as a separate file descriptor table.\r\n-\t**Close:** Will release any used memory to represent the file.\r\n-\t**Read:** tee_rpmb_fs_read, this API will be changed to accept a file handle instead of a name since there must have been a preceding open call. This API does not require any functional changes. Note that the entire file must be read at once in the current implementation. In the short term this is acceptable because the encryption filesystem reads the entire file from the underlying storage on the open call.\r\n-\t**Write:** tee_rpmb_fs_write, this API will be changed to accept a file handle instead of a name. This API does not require functional changes. Note that the entire file must be written at once in the current implementation. In the short term this is acceptable because the encryption filesystem writes the entire file to the underlying storage during the close call.\r\n-\t**Lseek:** A call to lseek to 0 will return success, everything else will return a failure since the whole file must be written at once. There is a call to lseek to the end in tee_enc_fs.c to get the file length but this can be changed to use stat instead.\r\n-\t**Ftruncate:** Anything other than 0 will return an error to delete the whole file. This will need to be implemented. tee_enc_fs.c currently calls to truncate the file to the new encrypted file size in tee_enc_fs_close() and then is followed by a total write of the new file contents. This can be changed to a truncate(0) followed by a write.\r\n-\t**Rename:** tee_rpmb_fs_rename currently works. This will just need to be changed to take into account the “directory” portion of the name.\r\n-\t**Unlink:** tee_rpmb_fs_rm will work here.\r\n-\t**Mkdir:** This will return a failure and should not be called because the current implementation of tee_svc_storage_create_file calls open first with the full path which will succeed skipping the mkdir call.\r\n-\t**Opendir:** A call to this API will traverse the FAT table and generate a list of entries. A read index will also be stored.\r\n-\t**Closedir:** Will free the generated list of entries.\r\n-\t**Readdir:** Will return the tee_fs_dirent at the current index and increment the index.\r\n-\t**Rmdir:** Will return success if all files in that “directory” have been unlinked.\r\n-\t**Access:** Will return true if that file exits.\r\n\r\n<h4><b>Optimizations</b></h4>\r\nOne performance issue that arrises from the proposed solution is that the entire FAT needs to be traversed to find a particular UUID/OBJECT_ID file instead of just traversing the directory files. This can be improved by:\r\n\r\n 1. Creating a hash of the file names to FAT addresses for quick lookup.\r\n 2. Longer term solution of adding directory entries to the FAT.  In other words having special entries in the FAT that point to files that store the directory entries and their locations.\r\n\r\nCurrently, the RPMB code generates a tee_mm_pool_t to represent the FAT structure on every write call. This can be done during initialization and stored in memory avoiding the need to traverse the FAT and build the pool on every write call. Also an in memory representation of the FAT will save the need to read the FAT and issue RPC calls to normal world.\r\n\r\n<h4><b>Open Questions</b></h4>\r\n1. Is there a current ETA for pull request 231 such that we can take a dependency on it?\r\n2. How is locking handled to the lower level RPCs? In other words can two threads attempt to write to the RPMB block at the same time?\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/OP-TEE/optee_os/issues/comments/104476882","html_url":"https://github.com/OP-TEE/optee_os/issues/296#issuecomment-104476882","issue_url":"https://api.github.com/repos/OP-TEE/optee_os/issues/296","id":104476882,"user":{"login":"youssefesmat","id":11986363,"avatar_url":"https://avatars.githubusercontent.com/u/11986363?v=3","gravatar_id":"","url":"https://api.github.com/users/youssefesmat","html_url":"https://github.com/youssefesmat","followers_url":"https://api.github.com/users/youssefesmat/followers","following_url":"https://api.github.com/users/youssefesmat/following{/other_user}","gists_url":"https://api.github.com/users/youssefesmat/gists{/gist_id}","starred_url":"https://api.github.com/users/youssefesmat/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/youssefesmat/subscriptions","organizations_url":"https://api.github.com/users/youssefesmat/orgs","repos_url":"https://api.github.com/users/youssefesmat/repos","events_url":"https://api.github.com/users/youssefesmat/events{/privacy}","received_events_url":"https://api.github.com/users/youssefesmat/received_events","type":"User","site_admin":false},"created_at":"2015-05-22T02:13:04Z","updated_at":"2015-05-22T02:13:04Z","body":"Thank you very much @jbech-linaro for looking at this. I will apply your feedback."}}