Move domain payload field to Operation token instead PubKey one
#647
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nt CRNs, they have to sign a new pubkey token for every CRN, and this is so bad for the user experience. Solution: Move the `domain` field to the operation token payload instead the pubkey one, just to improve the user experience and maintain the security integrity.
|
This proposal trades security versus convenience, we need to double check it: What happens if the SignedPubKeyPayload is leaked ? With a domain builtin, only VMs on a specific CNR could be messed up with. There is no revocation mechanism with this approach to authentication, so the user would not be able to do much. |
hoh
approved these changes
Jun 26, 2024
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev-confidential #647 +/- ##
====================================================
- Coverage 59.13% 59.12% -0.02%
====================================================
Files 65 65
Lines 5778 5776 -2
Branches 627 627
====================================================
- Hits 3417 3415 -2
- Misses 2215 2216 +1
+ Partials 146 145 -1 ☔ View full report in Codecov by Sentry. |
|
@nesitor can you merge this when you think appropriate ? |
olethanh
added a commit
that referenced
this pull request
Jul 5, 2024
* Implement Start Confidential endpoint (#627) * Problem: The server don't have a directory to save the platform certificates generated by sevctl. Solution: Set that directory field on settings class and ensure to create the folder on initialization step. * Problem: The aren't an endpoint to be able to get the confidential platform certificates to start the VM key exchange. Solution: Create that endpoint and return the platform certificates generated by the `sevctl` command. * Fix: Solved code quality issues. * Fix: Added 2 test cases for that endpoint. * Fix: Added PR suggestions. * Fix: Modified test mock to let the tests work * Problem: Now isn't possible as a VM operator to get the client session certificates to initialize a confidential VM. Solution: Create an operator start endpoint that receive the confidential session files and starts the qemu VM to continue with the certificate exchange methods. * Fix: Remove useless aiofiles import * Fix: Solve test issues after code quality fixes * Fix: Solve code quality issues. * Fix: Solve code quality issues. * Fix: Write file in sync mode to avoid adding a new dependency. Files to write should be so small, so any blocking issue should be here. * Fix: Solved PR comments and wrong conditionals. * Fix: Solved more PR comments. * Fix: Removed unexisting import * Fix: Added useless command requested on the PR review. * Fix: Changed endpoint path and added automatic tests for that endpoint. * Fix: Solved settings singleton issue with testing, adding an `initialize_settings` method. * Fix: Just disable the setting that is failing and remove previous method to initialize the singleton. * Fix: CI Droplet cleanup failed when same name was used When there were multiple Droplets with the same name, cleanup using doctl compute droplet delete -f $NAME would not work. Error: There are 3 Droplets with the name "aleph-vm-ci-XXX"; please provide a specific Droplet ID. [425559566, 425702949, 425703724] (cherry picked from commit b824503) * Problem: Could not install on Python 12 via pip install -e because of deps problem. Solution : upgrade aiohttp version * Problem: Crash in log when VM was printing control char * Raise log level for VM termination in controller so we always display when it finish * comment * Problem: Error were not properly returned in allocation endpoint * Add Qemu confidential controler implementation * remove duplicate endpoint * fix test in test_about_certificates * Add TODO comment Co-authored-by: nesitor <[email protected]> * Only run Confidentifial if is_confidential * Add script to build OVMF file for confidential VMs (#636) * Rename the confidential endpoints (#641) * Rename confidential endpoints * Rename the function too and reorder * isort * Use unified logging system for confidential * Provide an example confidential image construction script and instruction * Prevent the cleanup being run twice but still works with -e * Problem: sudo command was not working inside the VM ensure the setuid bit stay preserved when copying the file" * remove unecesary step * More example instruction * Problem: A user cannot specify which OVMF firmware want to use for they instances. Solution: Use new aleph-message version that includes that data schema and implement it on the qemu confidential resources. * Adapt example confidential message * fix host volume for confidential * Force aleph-message minimal version * Correct problem in HostVolume code * Merge both test_operator * Fix options for HostVolume * Move `domain` payload field to Operation token instead PubKey one (#647) * Problem: If a user wants to manage different operations for a different CRNs, they have to sign a new pubkey token for every CRN, and this is so bad for the user experience. Solution: Move the `domain` field to the operation token payload instead the pubkey one, just to improve the user experience and maintain the security integrity. * Fix: Solved test error message failing. --------- Co-authored-by: Andres D. Molins <[email protected]> * Problem: Failing initialization of AlephQemuConfidentialInstance due to merge problem * fix invoking of sevctl * Update src/aleph/vm/pool.py --------- Co-authored-by: nesitor <[email protected]> Co-authored-by: Hugo Herter <[email protected]> Co-authored-by: Andres D. Molins <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem: If a user wants to manage different operations for a different CRNs, they have to sign a new pubkey token for every CRN, and this is so bad for the user experience.
Solution: Move the
domainfield to the operation token payload instead the pubkey one, just to improve the user experience and maintain the security integrity.