Skip to content
This repository was archived by the owner on Nov 10, 2022. It is now read-only.

Trust domain intro 00002#3

Closed
MikeCamel wants to merge 33 commits into
enarx-archive:masterfrom
MikeCamel:Trust-domain-intro-00002
Closed

Trust domain intro 00002#3
MikeCamel wants to merge 33 commits into
enarx-archive:masterfrom
MikeCamel:Trust-domain-intro-00002

Conversation

@MikeCamel

Copy link
Copy Markdown
Contributor

Initial drafts for rfc#00002 and rfc#00003 (related topics).

@MikeCamel MikeCamel added the documentation Improvements or additions to documentation label Feb 12, 2020

@lkatalin lkatalin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I found a few typos and had a few questions. Otherwise this looks good and informative!

Comment thread rfc#00002 Outdated
Comment thread rfc#00003 Outdated
Comment thread rfc#00003 Outdated
Comment thread rfc#00003 Outdated
Comment thread rfc#00003 Outdated
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
@MikeCamel MikeCamel force-pushed the Trust-domain-intro-00002 branch from 4cdd407 to 568f680 Compare February 13, 2020 21:50

@axelsimon axelsimon left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lots of suggested changes, hope they help.

Very interesting stuff, one thing i think is missing is a bit of a higher-level abstraction of what trust domains are, maybe as a general introduction. Something along the lines of trust domains: what falls within the realm of things you need to consider in term of trust relationships, and what doesn't.

This should also make it easier to help people grok the power of trust domains as a tool for considering not only what is and out of the domain but also what moves in and out of the domains.

Comment thread rfc#00002 Outdated
Comment thread rfc#00002 Outdated
Comment thread rfc#00002 Outdated
Comment thread rfc#00002 Outdated
Comment thread rfc#00002 Outdated
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
Comment thread rfc#00003
@axelsimon

Copy link
Copy Markdown
Collaborator

Quick note, as per the RFC process (which is now live), this RFC should use the number of its PR, so it should be RFC 00003 :)

Comment thread rfc#00002 Outdated

@MikeCamel MikeCamel left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

All changes reviewed.

@npmccallum npmccallum left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm reading through the RFCs now. However, there are three major process issues that need to be resolved:

  1. You can't file more than one RFC in a single PR.
  2. The RFC MUST have the number of the PR that adds it.
  3. The file name format is incorrect.

Please follow the instructions detailed here:

https://github.com/enarx/rfcs/blob/master/contributing.md#how-to-propose-an-rfc

@npmccallum

Copy link
Copy Markdown
Contributor

Having now read them, I think the best way forward is to combine them. The first RFC feels too light and should just be a preface to the second one. After merging them, please use RFC#00003.

@MikeCamel

Copy link
Copy Markdown
Contributor Author

I'm reading through the RFCs now. However, there are three major process issues that need to be resolved:

1. You can't file more than one RFC in a single PR.

This was my first PR. Won't do it again.

2. The RFC MUST have the number of the PR that adds it.

Includes or adds? I think we've established (in this thread: #19) that this doesn't work if we want to have consecutive RFCs, because of how the numbering of PRs is assigned.

3. The file name format is incorrect.

See below.

Please follow the instructions detailed here:

https://github.com/enarx/rfcs/blob/master/contributing.md#how-to-propose-an-rfc

This was not complete at time of submission, which is why it was wrong. Will change the file name format, however.

@MikeCamel

Copy link
Copy Markdown
Contributor Author

Having now read them, I think the best way forward is to combine them. The first RFC feels too light and should just be a preface to the second one. After merging them, please use RFC#00003.

Disagree. #2 stands on its own, and I expect to reference it from other RFCs. #3 is one implementation (the default), but not the only one.

@lkatalin

Copy link
Copy Markdown
Contributor

This version is a lot more clear than previously. 🎉 There are still issues with consistent capitalization in the lists in Enarx use case states; not sure how much we care.

MikeCamel and others added 16 commits March 24, 2020 14:43
typo

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>
capitalisation

Co-Authored-By: Lily Sturmann <lkatalin@users.noreply.github.com>

@lkatalin lkatalin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good aside from a couple of markdown rendering issues as noted below.
I assume the commits will be squashed at some point as well.

Comment thread rfc#00003.md
- entities running processes on the host (includes other workloads
and compromised processes)
- man-in-the-middle entities
Clearly, the first two are the key actors - the others are included for

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This shows up as part of the "man-in-the-middle" bullet point in markdown.

Comment thread rfc#00003.md
- Provisioned Keep
- tenant workload
- Other workloads
Not all of these exist at all points in the process.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This line also needs a space in order to render properly in markdown.

Suggested change
Not all of these exist at all points in the process.
Not all of these exist at all points in the process.

Comment thread rfc#00003.md

## Future Possibilities

- Threat models should be documented and published as RFCs [Issue 15](https://github.com/enarx/rfcx/issues/15)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Threat models should be documented and published as RFCs [Issue 15](https://github.com/enarx/rfcx/issues/15)
- Threat models should be documented and published as RFCs [Issue #15 ](https://github.com/enarx/rfcx/issues/15)

For consistency with the below bullet point.

@mbestavros mbestavros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

While I have no qualms with the content of these RFCs (indeed, it's really well done), it needs some organizational love, including:

  • a rebase
  • organizing the READMEs according to the criteria outlined in #22 (admittedly, yet to be merged)

Additionally, this RFC currently contains two READMEs. If the tests allow it, this might be fine -- but it might also be prudent to either split them into separate PRs, or combine them.

@axelsimon

Copy link
Copy Markdown
Collaborator

I suggest closing this PR as our process has been refined since we started off and his PR doesn't fit.
PR #47 addresses the Trust Domain Introduction part of this PR.
A new PR will be opened soon for the Trust domains and Enarx part of this PR.

@mbestavros

Copy link
Copy Markdown
Contributor

Closing per @axelsimon

@mbestavros mbestavros closed this Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants