Skip to content

Conversation

iKapitonau
Copy link

No description provided.

@kvinwang kvinwang marked this pull request as ready for review October 9, 2025 00:30
Copy link
Collaborator

@kvinwang kvinwang left a comment

Choose a reason for hiding this comment

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

Let's also update the client SDKs under sdk/. It is also recommended to add a corresponding Verify() API as a counterpart to Sign().

@kvinwang kvinwang requested a review from h4x3rotab October 9, 2025 01:44
Comment on lines +236 to +238
message SignResponse {
bytes signature = 1;
}
Copy link
Collaborator

@kvinwang kvinwang Oct 16, 2025

Choose a reason for hiding this comment

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

Suggested change
message SignResponse {
bytes signature = 1;
}
message SignResponse {
// the signature of the data
bytes signature = 1;
// The signature chain consists of the following signatures:
// [0] - the signature of the data
// [1] - the k256 signature of the message signing pubkey signed by the app root key
// [2] - the k256 signature of the app root pubkey signed by the KMS root key
repeated bytes signature_chain = 2;
// The public key signing the data
bytes public_key = 3;
}

The signature_chain[1:2] can be optained from the response of GetKey.

// Derived key
bytes key = 1;
// Derived k256 signature chain
// Derived signature chain
Copy link
Collaborator

@kvinwang kvinwang Oct 16, 2025

Choose a reason for hiding this comment

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

Suggested change
// Derived signature chain
// The signature chain consists of the following signatures:
// [0] - the k256 signature of the derived pK signed by the app root key
// [1] - the k256 signature of the app root pK signed by the KMS root key

Comment on lines +316 to +328
let key_response = self
.get_key(GetKeyArgs {
path: "vms".to_string(),
purpose: "signing".to_string(),
algorithm: "ed25519".to_string(),
})
.await?;
let key_bytes: [u8; 32] =
key_response.key.try_into().expect("Key is incorrect");
Ed25519SigningKey::from_bytes(&key_bytes)
.verifying_key()
.to_bytes()
.to_vec()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may be unnecessary. Typically, CVM1 signs a message for verification in CVM2.

Comment on lines +340 to +349
let key_response = self
.get_key(GetKeyArgs {
path: "vms".to_string(),
purpose: "signing".to_string(),
algorithm: request.algorithm,
})
.await?;
let signing_key = SigningKey::from_slice(&key_response.key)
.context("Failed to parse secp256k1 key")?;
signing_key.verifying_key().to_sec1_bytes().to_vec()
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

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