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

SGX powHSM #274

Merged
merged 53 commits into from
Jan 22, 2025
Merged

SGX powHSM #274

merged 53 commits into from
Jan 22, 2025

Conversation

amendelzon
Copy link
Collaborator

This PR includes the following already reviewed and merged PRs:

amendelzon and others added 30 commits October 22, 2024 16:55
- SGX docker image for building
- powHSM layer adjustments
- Factored out Ledger Signer difficulty and initial block hash argument parsing
- HAL adjustments and additional x86 functions
- SGX implementation
- Added SGX build (debug and non debug) scripts
- Added SGX support to firmware test framework
- Added HAL layer common sources unit tests to our CI test runner
- Added simulator-only seal and unseal implementations (seal/unseal is unsupported in simulation mode and errors out)
- Added simulation build script
- Added separate simulation configuration file
- Ignoring SGX data files
- Renamed manager and adm to manager_ledger and adm_ledger
- Renamed manager-tcp to manager_tcp for consistency (python doesn't like hyphens in module names)
- Renamed build scripts
- Protocol now accepts configurable "restart the powHSM" messages
- Updated tcpsigner bundle artifacts
- Updated unit tests
- Updated documentation
- Updated github workflows
- Updated .gitignore(s)
- Incidentally removed unnecessary file
- Added password changing capabilities to the SGX powHSM implementation
- Added main SGX manager script
- Added build script for the SGX manager
- Added password change and password retries getter commands to the HSM2DongleSGX
- Updated user options to make names more generic
- Added unit tests
- Updated middleware docker do script to expose the host to the container (allowing for running tests against e.g. an SGX powHSM simulator)
* Added onboard method to HSM2DongleSGX

* Added Platform abstraction to manage common platform parameters

* Replaced protocol platform-specific configuration for text messages with Platform messages

* Implemented SGX admin tool

- Implemented main SGX admin tool entrypoint with onboard, unlock, pin change and public key gathering commands
- Misc get_hsm function now accounts for current Platform to acquire a connection
- Updated specific admin modules to account for difference between platforms for specific operations
- Now setting Platform within Ledger admin tool

* Updated build scripts to account for SGX manager and admin

* Added and updated unit tests
- Moved dist to dist/ledger
- Added dist/sgx with setup and run scripts
- New SGX distribution building script (build-dist-sgx)
- Added and updated distribution documentation for both Leger and SGX
- Additional HSM2DongleSGX unit tests for the onboarding operation
- Added integration tests for SGX simulator to the testing workflow
- Bumped integration tests version to 5.3.0.plus
* Added unit tests for SGX HAL nvmem module
* Added SGX HAL unit tests to the CI
* Added check to avoid for buffer overrun on nvmem.c
- Added secret store unit tests
- Minor changes to secret_store.c to allow for easier unit testing
- Removed mock files from test coverage report
Unit testing endian and keccak256 modules
- Added bip32 derivation tests on separate source file
- Added hmac_sha512 tests using 3rd party test vectors
- Added test helpers to common x86 test artifacts
- Moved common.mk to common directory
- Updated existing Makefile(s) to account for moved common.mk
- Bumped version to 5.3.0
- Updated version references in firmware, middleware, unit tests, CI workflows and documentation
- Updated CHANGELOG
- Bumped version to 5.3.1
- Updated version references in firmware, middleware and unit tests
- Updated CHANGELOG
- Updates SGX Docker base image to `2024.10.2391`
- Removes unnecessary dependencies from SGX Dockerfile
- Freezes open-enclave version to `0.19.4`
- Ensures deterministic build order
- Implemented new attestation protocol in firmware
- Moved attestation context definition to attestation header file
- Updated admin tooling to gather and validate new attestation format, keeping support for legacy format
- Factored out attestation gathering logic from HSM2Dongle
- New semantics for code hash and public key gathering functions in existing endorsement module
- Additional endorsement module functions to allow for envelope gathering
- New platform module to provide platform id and timestamp
- Added and updated unit tests
- Updated attestation documentation
- Added SGX endorsement library's envelope function stubs
- Triggers the coverage workflow for pushes to master and feature/sgx branches
- Adds optional exec argument unit tests scripts
- Some additional fixes to unit tests
- Added endorsement initialisation and finalisation functions for SGX
- Added system finalisation ecall, used within the untrusted code's finalisation routine
- Removed old stubs for SGX's HAL platform and endorsement
- Added der_utils library to aid with endorsement signature encoding
- Implemented SGX endorsement library
- Implemented SGX platform library
- Adjusting sync acquire lock's function to account for different return types
- Removed now unused hmac_sha256 library from SGX's HAL
- Added SGX's HAL der_utils and endorsement unit tests
- Triggers the coverage workflow for pushes to master and feature/sgx branches
- Adds optional exec argument unit tests scripts
- Some additional fixes to unit tests
- Bumped version to 5.3.2
- Updated version references in firmware, middleware and unit tests
- Updated CHANGELOG
- Unified certificates in certificate module
- Moved existing HSMCertificate and HSMCertificateElement to certificate_v1
- Added new HSMCertificateV2 and HSMCertificateV2Element (and subclasses)
- Added new attestation gathering operation for adm_sgx
- Added new attestation gathering module used within adm_sgx
- Factored out user-defined value gathering into admin/misc module
- Current SGX attestation saving to file is directly translated from value grabbed with HSM2Dongle
- Added and updated unit tests
- Added individual certificate v2 element types with serialization logic: SgxQuote, SgxAttestationKey and X509
- Added CStruct abstraction to simplify representing C structs
- Added struct definitions as CStruct subtypes based on OpenEnclave's endorsement type definitions
- Updated SGX attestation gathering logic to parse the envelope and generate the actual certificate elements
- Added and updated unit tests
- Ignoring long line linting in envelope unit tests
- Added version mapping and parsing logic to certificate version 1
- Added dict based initialisation functions to HSMCertificateV2Element and its subclasses
- Updated sgx attestation gathering certificate generation
- Added and updated unit tests
- Ignoring long line linting in certificate v2 resources file
amendelzon and others added 23 commits December 24, 2024 00:30
- Replacing plain public key in validate_and_get_values method for a root of trust object with get_pubkey method
- Using inherited validate_and_get_values in HSMCertificateV2
- Added HSMCertificateRoot to certificate version 1 to act as root of trust in existing validations
- Updated verify ledger attestation accordingly
- Added and updated unit tests
- Added message parsing capabilities to HSMCertificateV2ElementSGXQuote
- Added file certificate loading capabilities to HSMCertificateV2ElementX509
- Factored out common behavior between ledger and SGX attestation verification into helper library functions
- Added helper library for assorted attestation utils, including a v5+ attestation message parsing class
- Added SGX do_verify_attestation function with its corresponding module
- Added verify_attestation operation to adm_sgx tool
- Added and updated unit tests
- Ignoring long line warnings for test attestation utils resources module
- HSMCertificateV2Element base class now raises not implemented errors for is_valid and get_pubkey
- Implemented is_valid in HSMCertificateV2ElementSGXQuote
- HSMCertificateV2ElementSGXQuote's message method now returns an SgxQuote
- Mocking is_valid and get_pubkey in the rest of HSMCertificateV2Element's subclasses
- Added and updated unit tests
- Implemented HSMCertificateV2ElementSGXAttestationKey's key and is_valid methods
- HSMCertificateV2ElementSGXAttestationKey's message now returns an instance of SgxReportBody
- HSMCertificateV2ElementX509 now parses the raw certificate into an cryptography x509 certificate object
- Implemented HSMCertificateV2ElementX509's get_pubkey method
- Added cryptography dependency to middleware docker image
- Added and updated unit tests
- Incidentally removing some garbage
* Certificate V2 x509_pem type element validation

- Implemented HSMCertificateV2ElementX509's is_valid method
- Added root of trust element validation in SGX attestation verification
- Added and updated unit tests

* Changes as per PR review
- Updated setup script to gather and verify attestation after onboarding is completed
- Added runtime attestation dependencies to the SGX distribution Dockerfile
- Throwing ERR_INS_NOT_SUPPORTED for the heartbeat operation in SGX only
- Added test case to system module unit tests
- Updated main README
- Added SGX stuff to attestation documentation
- Added reproducible builds information to build README
- Added note to heartbeat documentation stating that it is currently unsupported for SGX
- Added corresponding unsupported notes in protocol operations within the protocol documentation
- Including sgx code in lint-c/format-c scripts
- Fixed reported sgx linting errors
Sets the value of uninitialized file descriptors on io.c to -1 instead of 0.
Fixes the signature of finalise function so that it conforms with
the expected signal handler function signature
- Sanitizes the key before using it for file operations.
- Added unit tests for keyvalue_store module
- Using oe_is_outside_enclave to validate the APDU buffer in system_init
- Added and updated unit tests cases
Signal handler now only sets a flag that is checked in main
* Fixes coverage report for feature/sgx branch (#224)

- Triggers the coverage workflow for pushes to master and feature/sgx branches
- Adds optional exec argument unit tests scripts
- Some additional fixes to unit tests

* Install SGX powHSM as a systemd service (#226)

* Version 5.3.2 ALPHA release (#227)

- Bumped version to 5.3.2
- Updated version references in firmware, middleware and unit tests
- Updated CHANGELOG

* Fix middleware docker image build (#252)

* Fixed C linting to include sgx code (#261)

- Including sgx code in lint-c/format-c scripts
- Fixed reported sgx linting errors

* Sets uninitialized socket file descriptors to -1 (#248)

Sets the value of uninitialized file descriptors on io.c to -1 instead of 0.

* Removes .gitignore from distribution builds (#264)

* Fixes signature for finalise function (#249)

Fixes the signature of finalise function so that it conforms with
the expected signal handler function signature

* Fixes formatting error identified by C linter (#266)

* Sanitizes key for kvstore (#247)

- Sanitizes the key before using it for file operations.
- Added unit tests for keyvalue_store module

* Added APDU buffer pointer validation to SGX enclave init sequence (#267)

- Using oe_is_outside_enclave to validate the APDU buffer in system_init
- Added and updated unit tests cases

* Moves finalise logic out of signal handler (#268)

Signal handler now only sets a flag that is checked in main

* Fixed C linting errors

---------

Co-authored-by: Italo Sampaio <[email protected]>
Copy link

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 1 package(s) with unknown licenses.
See the Details below.

License Issues

docker/mware/requirements.txt

PackageVersionLicenseIssue Type
cryptography44.0.0NullUnknown License

OpenSSF Scorecard

PackageVersionScoreDetails
actions/actions/checkout 3.*.* 🟢 6.8
Details
CheckScoreReason
Maintained🟢 56 commit(s) and 0 issue activity found in the last 90 days -- score normalized to 5
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
Code-Review🟢 10all changesets reviewed
Binary-Artifacts🟢 10no binaries found in the repo
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
License🟢 10license file detected
Fuzzing⚠️ 0project is not fuzzed
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
Pinned-Dependencies🟢 3dependency not pinned by hash detected -- score normalized to 3
Security-Policy🟢 9security policy file detected
Packaging🟢 10packaging workflow detected
SAST🟢 9SAST tool detected but not run on all commits
Vulnerabilities🟢 82 existing vulnerabilities detected
pip/cryptography 44.0.0 🟢 8.2
Details
CheckScoreReason
Code-Review🟢 10all changesets reviewed
Security-Policy🟢 10security policy file detected
Maintained🟢 1030 commit(s) and 15 issue activity found in the last 90 days -- score normalized to 10
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
License🟢 9license file detected
Signed-Releases⚠️ -1no releases found
Token-Permissions🟢 9detected GitHub workflow tokens with excessive permissions
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: githubv4.Query: Resource not accessible by integration
Fuzzing🟢 10project is fuzzed
Binary-Artifacts🟢 10no binaries found in the repo
Packaging🟢 10packaging workflow detected
Pinned-Dependencies🟢 4dependency not pinned by hash detected -- score normalized to 4
Vulnerabilities🟢 73 existing vulnerabilities detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0

Scanned Files

  • .github/workflows/run-tests.yml
  • docker/mware/requirements.txt

HEADER_REGEX = re.compile(b"^POWHSM:(5.[0-9])::")

@classmethod
def is_header(kls, value):

Check notice

Code scanning / CodeQL

First parameter of a class method is not named 'cls' Note

Class methods or methods of a type deriving from type should have 'cls', rather than 'kls', as their first parameter.
import secp256k1 as ec
import hashlib
from .utils import is_nonempty_hex_string
from .certificate_v1 import HSMCertificate, HSMCertificateRoot, HSMCertificateElement

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'HSMCertificateRoot' is not used.
Import of 'HSMCertificateElement' is not used.
Comment on lines +24 to +26
from .certificate_v2 import HSMCertificateV2, HSMCertificateV2ElementSGXQuote, \
HSMCertificateV2ElementSGXAttestationKey, \
HSMCertificateV2ElementX509

Check notice

Code scanning / CodeQL

Unused import Note

Import of 'HSMCertificateV2ElementSGXQuote' is not used.
Import of 'HSMCertificateV2ElementSGXAttestationKey' is not used.
Import of 'HSMCertificateV2ElementX509' is not used.
ELEMENT_FACTORY = HSMCertificateElement

@classmethod
def from_jsonfile(kls, path):

Check notice

Code scanning / CodeQL

First parameter of a class method is not named 'cls' Note

Class methods or methods of a type deriving from type should have 'cls', rather than 'kls', as their first parameter.
raise NotImplementedError("Cannot instantiate a HSMCertificateV2Element")

@classmethod
def from_dict(kls, element_map):

Check notice

Code scanning / CodeQL

First parameter of a class method is not named 'cls' Note

Class methods or methods of a type deriving from type should have 'cls', rather than 'kls', as their first parameter.
return klass._platform == Platform.SGX

@classmethod
def options(klass, key):

Check notice

Code scanning / CodeQL

First parameter of a class method is not named 'cls' Note

Class methods or methods of a type deriving from type should have 'cls', rather than 'klass', as their first parameter.
return klass._options[key]

@classmethod
def message(klass, key):

Check notice

Code scanning / CodeQL

First parameter of a class method is not named 'cls' Note

Class methods or methods of a type deriving from type should have 'cls', rather than 'klass', as their first parameter.
Comment on lines +182 to +183
"Installed Signer hash: ffffffffffffffffffffffffffffffffffffffffffffffff"
"ffffffffffffffff",

Check warning

Code scanning / CodeQL

Implicit string concatenation in a list Warning test

Implicit string concatenation. Maybe missing a comma?
Comment on lines +186 to +187
"UD value: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
"aaa",

Check warning

Code scanning / CodeQL

Implicit string concatenation in a list Warning test

Implicit string concatenation. Maybe missing a comma?
Comment on lines +188 to +189
"Best block: bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
"bbbbb",

Check warning

Code scanning / CodeQL

Implicit string concatenation in a list Warning test

Implicit string concatenation. Maybe missing a comma?
Copy link
Collaborator

@italo-sampaio italo-sampaio left a comment

Choose a reason for hiding this comment

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

LGTM!

@amendelzon amendelzon merged commit 865641c into master Jan 22, 2025
8 checks passed
@amendelzon amendelzon deleted the feature/sgx-up-to-date branch January 22, 2025 13:49
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