Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

Extend README and configuration documentation. #24

Merged
merged 3 commits into from
Aug 15, 2022
Merged

Conversation

NullHypothesis
Copy link
Contributor

This commit elaborates on nitriding's features, documents the purpose of
our configuration variables, and explains what tooling is necessary to
get the example application to work.

This commit elaborates on nitriding's features, documents the purpose of
our configuration variables, and explains what tooling is necessary to
get the example application to work.
@NullHypothesis NullHypothesis requested a review from rillian August 12, 2022 14:44
rillian
rillian previously approved these changes Aug 12, 2022
Copy link
Contributor

@rillian rillian left a comment

Choose a reason for hiding this comment

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

Good improvements to clarity. Thanks for writing documentation!

README.md Outdated Show resolved Hide resolved
Comment on lines +92 to +93
// Debug can be set to true to see debug messages, i.e., if you are
// starting the enclave in debug mode by running:
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if the framework could ask the supervisor if it's in debug mode, but I don't see an interface for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One could implement this by asking for an attestation document and checking if its PCR values are all zeroed out. I filed https://github.com/brave/nitriding/issues/25 for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, clever!

Comment on lines +74 to +75
// 2) your enclave application makes HTTP requests over the Internet.
// If so, set SOCKSProxy to "socks5://127.0.0.1:1080".
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention this port number must match the one given to viproxy on the host instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually doesn't have to because this variable only controls the in-enclave IP-to-VSOCK proxy. There's a viproxy instance running on both the host and inside the enclave. We should probably remove this variable from the config struct because it's confusing and there aren't many good reasons to change it.

@NullHypothesis NullHypothesis merged commit 3bf269d into master Aug 15, 2022
@NullHypothesis NullHypothesis deleted the update-readme branch August 15, 2022 18:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants