Uh oh!
There was an error while loading. Please reload this page.
TEE Registry Management CLI tool - #26
Conversation
1935e6f to
7e7c46dCompare@khalifaT can you use a more descriptive PR title? It doesn't look like this PR has anything to do with integration tests |
There was a problem hiding this comment.
Pull request overview
This PR adds a new TEE (Trusted Execution Environment) Registry Management CLI tool under scripts/tee-mgmt-cli/ for interacting with the OpenGradient blockchain's TEE registry smart contract. It also updates the local_tee_workflow.go integration test to point to a remote RPC endpoint instead of localhost.
Changes:
- New
register-mgmt.goCLI providing commands for TEE lifecycle, PCR management, type management, role management, and AWS certificate operations. - New documentation (
Readme.md), configuration example (.env.example), and.gitignorefor the CLI tool. - Updated
RPC_URLconstant inlocal_tee_workflow.gofromhttp://127.0.0.1:8545to a remote IP.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/tee-mgmt-cli/register-mgmt.go | New CLI tool implementing all TEE registry management commands via Ethereum JSON-RPC |
scripts/tee-mgmt-cli/Readme.md | Usage documentation for the CLI |
scripts/tee-mgmt-cli/.env.example | Template environment configuration file |
scripts/tee-mgmt-cli/.gitignore | Ignores built binary and .env secrets file |
scripts/integration/local_tee_workflow.go | Changes the "local" integration test RPC URL from localhost to a remote server IP |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
| func fetchAttestation(url string) (string, error) { | ||
| client := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, Timeout: 30 * time.Second} | ||
| resp, err := client.Get(url) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| defer resp.Body.Close() | ||
| body, _ := io.ReadAll(resp.Body) | ||
| return string(bytes.TrimSpace(body)), nil | ||
| } | ||
| func fetchSigningPublicKey(host string) ([]byte, error) { | ||
| client := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, Timeout: 30 * time.Second} | ||
| resp, err := client.Get(fmt.Sprintf("https://%s/signing-key", host)) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
| body, _ := io.ReadAll(resp.Body) | ||
| var data AttestationResponse | ||
| json.Unmarshal(body, &data) | ||
| if block, _ := pem.Decode([]byte(data.PublicKey)); block != nil { | ||
| return block.Bytes, nil | ||
| } | ||
| if decoded, err := base64.StdEncoding.DecodeString(data.PublicKey); err == nil { | ||
| return decoded, nil | ||
| } | ||
| return nil, fmt.Errorf("invalid key format") | ||
| } | ||
| func fetchTLSCertificate(host, port string) ([]byte, error) { | ||
| conn, err := tls.Dial("tcp", host+":"+port, &tls.Config{InsecureSkipVerify: true}) |
There was a problem hiding this comment.
Both fetchAttestation and fetchSigningPublicKey use InsecureSkipVerify: true in their TLS configurations. While the attestation document itself might serve as an integrity check, skipping TLS verification means that the connection is susceptible to man-in-the-middle attacks. An attacker could intercept the connection and substitute a malicious attestation document or signing key, potentially allowing an unauthorized TEE to be registered. Since this is a dev/script tool, this should at minimum be documented clearly, and ideally the tool should provide an option to supply a CA certificate for proper verification.
There was a problem hiding this comment.
shouldn't the developer pass in the certificate?
There was a problem hiding this comment.
Do you mean that they should be included in the attestation? If so, the attestation already includes the hash of both of them.
Regarding InsecureSkipVerify: this is due to how the certificates are generated by AWS Nitro. I'm not sure whether we can improve this (@kylexqian).
In the precompile, the certificate chain is validated against the AWS Nitro root. The attestation document contains the hashes of the TLS certificate and the signing public key, which are later compared with the values fetched via fetchAttestation / fetchSigningPublicKey. For the public key, this still needs to be added.
- A possible improvement would be to validate that our verifyCOSESignatureES384 implementation works correctly by cross-testing it with AWS’s reference verifier, or by replacing it with a library such as go-cose.
There was a problem hiding this comment.
no i mean shouldn't the certificate be passed in to the script?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| ./tee-cli show <tee_id> | ||
| # Register new TEE from enclave | ||
| ./tee-cli register |
There was a problem hiding this comment.
it feels like it would be more natural to pass in the TEE arguments to this command as opposed to setting them as env vars
| |----------|-------------|---------| | ||
| | `TEE_RPC_URL` | RPC endpoint | `http://13.59.43.94:8545` | | ||
| | `TEE_REGISTRY_ADDRESS` | Contract address | `0x3d641a2791533b4a...` | | ||
| | `TEE_PRIVATE_KEY` | Private key for signing | (uses node account if empty) | |
There was a problem hiding this comment.
If TEE_PRIVATE_KEY is not set, the CLI uses eth_sendTransaction RPC method, which requires the node to have an unlocked account. This is only for development but not for production.
- I may delete that if needed
There was a problem hiding this comment.
i do not understand what is a node/unlocked account, can you clarify in docs or remove?
Uh oh!
There was an error while loading. Please reload this page.
| | `TEE_PRIVATE_KEY` | Private key for signing | (uses node account if empty) | | ||
| | `ENCLAVE_HOST` | Enclave hostname | - | | ||
| | `ENCLAVE_PORT` | Enclave port | `443` | | ||
| | `MEASUREMENTS_FILE` | Path to measurements.txt | `measurements.txt` | |
There was a problem hiding this comment.
can you add more explanation around this? what should be the content of this file?
| ./tee-cli pcr-approve | ||
| # From environment variables | ||
| PCR0=abc123... PCR1=def456... PCR2=789... PCR_VERSION=v1.0.0 ./tee-cli pcr-approve |
There was a problem hiding this comment.
this would be better as cli params ./tee-cli pcr-approve --pcr0 abc1234 ...
There was a problem hiding this comment.
you can use a lib like this https://github.com/spf13/cobra
Description
Adds TEE Registry Management CLI tool for managing Trusted Execution Environment registration, attestation, and role-based access control on OpenGradient blockchain.
This CLI provides a complete command-line interface for: