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

WIP: Add enclave information #681

Draft
wants to merge 6 commits into
base: rolling
Choose a base branch
from

Conversation

danthony06
Copy link

Adds information about the security enclave associated with a node to the information. This helps us debug complex security configurations in large systems, and verify that nodes are associated with appropriate enclaves. I'd like to get feedback on the approach before I attempt to fix the unit tests.

Distro A, OPSEC #4584
Signed-off-by: David Anthony [email protected]

ros2node/ros2node/api/__init__.py Outdated Show resolved Hide resolved
ros2node/ros2node/verb/info.py Show resolved Hide resolved
Comment on lines +89 to +91
print(' Enclaves:')
print_enclaves([n.enclave for n in node_names_with_enclaves
if n.name.full_name == args.node_name])
Copy link
Collaborator

Choose a reason for hiding this comment

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

i am okay with either way, but we could consider this is one of the option for ros2 node info.

Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure which is the best option. My rationale for leaving it in the default output is:

  1. Most people are running unsecured systems, and in this case it does not add a lot to the information output
  2. If someone is running a secured system, this can be important information, and users might want to frequently see this when they are examining a system.

Copy link
Author

Choose a reason for hiding this comment

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

Is there any interest in this, or should I withdraw the PR? Internally, we've found that this information would be very useful for our development, and I'd like to help the community if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is helpful. can you open this PR as Non-Draft for review?

Most people are running unsecured systems, and in this case it does not add a lot to the information output

IMO, this is why i came up with option such as --include-enclaves.

@audrow audrow changed the base branch from master to rolling June 28, 2022 14:23
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.

2 participants