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

Security concern for private key in tvm repo #1189

Closed
kevinthesun opened this issue May 29, 2018 · 10 comments
Closed

Security concern for private key in tvm repo #1189

kevinthesun opened this issue May 29, 2018 · 10 comments

Comments

@kevinthesun
Copy link
Contributor

There is a private key stored in tvm repo:https://github.com/dmlc/tvm/blob/fdba6cc9bd3bec9ccd0592fa3900b7fe25d6cb97/apps/sgx/enclave_private.pem Do we have any potential security concern?Can we remove it?

@tqchen
Copy link
Member

tqchen commented May 29, 2018

I think it is purely used for demo purposes to run SGX, cc @nhynes

@nhynes
Copy link
Member

nhynes commented May 30, 2018

yep, it's for the SGX demo. It's not a security concern but it can't be removed otherwise the demo won't run.

@tqchen tqchen closed this as completed May 30, 2018
@mnuyens
Copy link

mnuyens commented Jun 1, 2018

Is there a reason not to generate the key at the beginning and destroy it at the end?

@nhynes
Copy link
Member

nhynes commented Jun 1, 2018

reason not to generate the key

No, but generating the key is non-trivial

@mnuyens
Copy link

mnuyens commented Jun 4, 2018

Leaving the key in the repo means that it will be flagged by many security tools, even if the key is not used for anything but the demo. I think the simplest fix would be move the demo into a separate repo for demos and examples, which would let TVM itself not be flagged, but not require the work to generate the key each. I think long term we should probably generate the key each time.

@kevinthesun
Copy link
Contributor Author

If we just don't want any private key in TVM repo, a quick workaround might be create a gist to store the private key file.

@nhynes
Copy link
Member

nhynes commented Jun 4, 2018

Hmm. I wonder if a fake key can be auto-generated from random bytes instead. I'll look into that

many security tools

Which ones, exactly? Do they check the file extension or do they look in the file, itself?

@mnuyens
Copy link

mnuyens commented Jun 5, 2018

In this case it was specifically an internally developed one and I assumed that others would flag it as well. And I believe it looks in the file itself, this part is also supposed to flag keys that are hard coded as well.

@mnuyens
Copy link

mnuyens commented Jun 15, 2018

This change is not enough to not have it flagged by the security tools as they are looking for any private keys hardcoded in the repo, and once the key is removed we should also rewrite the commit history to remove it there as well, otherwise it is still in the repo. Here is a link that details easy ways to do that. https://help.github.com/articles/removing-sensitive-data-from-a-repository/

@nhynes
Copy link
Member

nhynes commented Jun 15, 2018

Rewriting history for a public repo is a no-go. I suggest that you whitelist TVM in your security checker. This should be relatively easier since your tool is developed in-house.

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

No branches or pull requests

4 participants