Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protected-FileSystem(PFS) library #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Protected-FileSystem(PFS) library #1

wants to merge 4 commits into from

Conversation

sudharkrish
Copy link
Collaborator

@sudharkrish sudharkrish commented Aug 16, 2019

This library is a plugin for Graphene, that provides file-system shield to Graphene, by incorporating file protection feature from Intel's SGX-SDK's Protected-FS library.

Brief overview of PR, with info on directories and components added:

  1. docs folder has design document. Note: Code is NOT fully upto the design document, CHANGELOG has pending features.
    With GPFS library, the features that have been implemented are: filename confidentiality, filename integrity, file-path integrity and binding of files to a volume,
    on top of existing filesystem protection software (i.e. SGX-Protected Filesystem).

  2. ld_preload_lib: Has sources to intercept and overload file-system apis, and route it to SGX ProtectedFS library or to C filesystem api. Also has sources to augment cryptographic mechanisms
    to add additional features as described in the design doc under docs directory.

  3. pfs_sdk: Has a patch file that gets applied on SGX SDK's protected-fs library, to make it work within graphene, without sgx-sdk dependencies.

  4. pfs_proxy: Has changes, to hook calls from SGX SDK-protectefs library sources, to non-sgx-sdk apis.

  5. apps/protfs_app: Has sources for sample graphene application, that sets the following in the
    application's manifest(specifies path to proteced directory, sets the overloaded GPFS library, sets the type of key to be used for file-protection), and uses GPFS library
    transparently as a file-system shield

For more details, refer to README.


This change is Reviewable

This library is a plugin for Graphene, that provides file-system shield to Graphene, by incorporating file protection feature from
Intel's SGX-SDK's Protected-FS library.
For more details, refer to README
Copy link

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 53 files reviewed, 2 unresolved discussions (waiting on @sudharkrish)

a discussion (no related file):
Please fix the email address in your commit so that GitHub can recognize it (either by changing it in the commit or by adding it to your GitHub account). You can push-force if you choose the former.


a discussion (no related file):
Why the directory is called "protect-fs" when the plugin is named "protected-fs"? The naming in other places is also inconsistent, like "protfs" and "pfs".


Copy link

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 53 files reviewed, 2 unresolved discussions (waiting on @sudharkrish)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

Why the directory is called "protect-fs" when the plugin is named "protected-fs"? The naming in other places is also inconsistent, like "protfs" and "pfs".

There's also "Graphene-ProtectedFileSystem shield", "GPFS" and "Protected-FS". Please unify the naming.


@mkow mkow changed the title Graphene-Protected-FileSystem shield(GPFS) library: Graphene-Protected-FileSystem Shield (GPFS) library Aug 19, 2019
Copy link
Collaborator Author

@sudharkrish sudharkrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 53 files reviewed, 2 unresolved discussions (waiting on @mkow)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

Please fix the email address in your commit so that GitHub can recognize it (either by changing it in the commit or by adding it to your GitHub account). You can push-force if you choose the former.

Completed. I have added my email to GitHub account.


a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

There's also "Graphene-ProtectedFileSystem shield", "GPFS" and "Protected-FS". Please unify the naming.

I agree that having the same convention across, will be better. For example, I can stick to acronym->pfs, which stands for protected-filesystem. The only concern is, if I stick to pfs everywhere, the only place to find out, what pfs stands for, would be the README file. So in some places, like the directory names, it may be better to have partial expansion, like protected-fs. I am using the acronym pfs in code. Option1->Keep directory names as protected-fs, but we can keep the acronym->pfs in other places like code. Option2-> Keep pfs everywhere(directory-names, code, README file), in this case README will have expansion for what PFS stands for. You can let me know, if you have a preference for Option1 or Option2.


Copy link

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 53 files reviewed, 1 unresolved discussion (waiting on @sudharkrish)

a discussion (no related file):

Previously, sudharkrish wrote…

Completed. I have added my email to GitHub account.

Looks good now, thanks


a discussion (no related file):

Previously, sudharkrish wrote…

I agree that having the same convention across, will be better. For example, I can stick to acronym->pfs, which stands for protected-filesystem. The only concern is, if I stick to pfs everywhere, the only place to find out, what pfs stands for, would be the README file. So in some places, like the directory names, it may be better to have partial expansion, like protected-fs. I am using the acronym pfs in code. Option1->Keep directory names as protected-fs, but we can keep the acronym->pfs in other places like code. Option2-> Keep pfs everywhere(directory-names, code, README file), in this case README will have expansion for what PFS stands for. You can let me know, if you have a preference for Option1 or Option2.

Option 1 sounds better to me, but I'd prefer to restrict using pfs to variable and function names only, e.g. when prefixing them like pfs_read_file, so for example comments in the code would use "protected-fs" name (or "Protected-FS" - please decide about the casing and dashing also and make this consistent).


Copy link
Collaborator Author

@sudharkrish sudharkrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 0 of 53 files reviewed, 1 unresolved discussion (waiting on @mkow)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

Option 1 sounds better to me, but I'd prefer to restrict using pfs to variable and function names only, e.g. when prefixing them like pfs_read_file, so for example comments in the code would use "protected-fs" name (or "Protected-FS" - please decide about the casing and dashing also and make this consistent).

Sure, sounds good. Will do the change, per Option1, along with your suggestions.


skris14 and others added 2 commits August 26, 2019 15:43
1. Changes to make PFS acronym(which stands for Protected-Filesystem) consistent
across directory-names, code, README and application sources.
2. Also fixed few issues in pfs_app test sources.
@sudharkrish
Copy link
Collaborator Author

@mkow updated PR, to make acronym->PFS consistent across directory-names, code, README and application sources(pfs_app).
Link to updated commit of PR: 261cc61

@sudharkrish sudharkrish changed the title Graphene-Protected-FileSystem Shield (GPFS) library Graphene-Protected-FileSystem Shield (PFS) library Sep 4, 2019
@sudharkrish sudharkrish changed the title Graphene-Protected-FileSystem Shield (PFS) library Protected-FileSystem Shield (PFS) library Sep 4, 2019
@sudharkrish sudharkrish changed the title Protected-FileSystem Shield (PFS) library Protected-FileSystem (PFS) shield library Sep 4, 2019
@sudharkrish sudharkrish changed the title Protected-FileSystem (PFS) shield library Protected-FileSystem(PFS) library Sep 4, 2019
…s. In order to build this, will need following 2 dependencies: - Hashmap Lib: Using the hashmap library(MIT License, same as mbedtls

 used by Graphene): You can pull the sources from here-> https://github.com/DavidLeeds/hashmap - Also needs list.h in Graphene-PAL, to have few additional features like sorting, and extra field like size.
 Please refer to changes in just 1 file->list.h. gramineproject/graphene#1268
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants