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

Playbook signature validation #8

Merged
merged 6 commits into from Dec 8, 2020

Conversation

adamruzicka
Copy link
Collaborator

@adamruzicka adamruzicka commented Nov 19, 2020

Requires:

Copy link
Collaborator

@ares ares left a comment

Choose a reason for hiding this comment

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

Looks good to me, one comment about the configurability, not sure if you want to push that into this PR. Also I guess we need to rely on cloud side to know if we want signed or unsigned playbooks (unsigned for older Satellites without this patch)

@@ -64,10 +66,11 @@ async def run(self):

await self.satellite_api.init_session()
try:
self.queue.ack(self.playbook_run_id)
self.playbook = playbook_verifier.verify(self.playbook)
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 we'll need to make this optional, however the setting should default to true.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we? We rely on cloud to know if a client can or cannot process signed playbooks, I assumed this would be based solely on receptor-satellite version. If we make this optional, then cloud would have to know about version and value of the setting

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In case we want to disable it at the last minute, we can switch the commented and uncommented lines here https://github.com/project-receptor/receptor-satellite/blob/a96253b05daaddbc07de8cef1d50a1a6d391bbab/receptor_satellite/playbook_verifier_adapter.py

@ares
Copy link
Collaborator

ares commented Dec 8, 2020

Thanks @adamruzicka, merging!

@ares ares merged commit 559ea6c into project-receptor:master Dec 8, 2020
@adamruzicka adamruzicka deleted the signature branch May 26, 2021 14:14
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.

None yet

2 participants