Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

KME Replication at trusted layer #765

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rranjan3
Copy link
Contributor

@rranjan3 rranjan3 commented Apr 7, 2021

Signed-off-by: Rajeev Ranjan [email protected]

@rranjan3 rranjan3 changed the title Enable KME Replication KME Replication at trusted layer Apr 7, 2021
@@ -32,6 +32,14 @@ enum KmeRegistrationStatus {
ERR_UNIQUE_ID_NOT_MATCH = 7 /// WPE unique id didn't match
};

enum KmeReplicationReturnCode { /// KME Replication operation(state-uid, state-request, get-state, set-state) status
Copy link
Contributor

Choose a reason for hiding this comment

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

modify Replication operation to Replication operations

*
* @param in_work_order_data - vector of work order indata.
* It consists of the unique_id and signing key
* @param out_work_order_data - vector of work order outdata
Copy link
Contributor

Choose a reason for hiding this comment

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

Add description about contents of out_work_order_data as well.

// replica KME
ByteArray uid_ba = in_work_order_data[0].decrypted_data;
// Generate random bytes of size 16,
ByteArray nonce_bytes = tcf::crypto::RandomBitString(16);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we define numeric constant for nonce size? It makes easy to modify later.

ByteArray uid_nonce_sig = in_work_order_data[3].decrypted_data;
// Extract replica KME pub signing key from in_data that comes in
// the request
ByteArray pub_sig_key_ba = in_work_order_data[4].decrypted_data;
Copy link
Contributor

Choose a reason for hiding this comment

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

does attestation_info not contain public signing key and encryption key of the replica KME? Or is it only the attestation report?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have not included the attestation_info as of now. Can do away with the public keys once it is added.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright

}

//Cleanup in-memory state-uid
state_uid_hex = "";
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to clean state variables at the end to avoid failures post this step.

serialized_str += ByteArrayToStr(it->first);
serialized_str += ":";
serialized_str += ByteArrayToStr((it->second).serialize());
serialized_str += "|";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we use the same delimiter for serializing both encryrption and signature key maps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The WPEInfo causes the difference which also needs some delimiter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

@@ -33,6 +33,26 @@ typedef struct WPEInfo {

WPEInfo();
WPEInfo(const ByteArray& _sk);
WPEInfo(const uint64_t _wo_c, const ByteArray& _sk);

ByteArray serialize(){
Copy link
Contributor

Choose a reason for hiding this comment

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

Add function comments


return StrToByteArray(serialized);
}
void deserialize(ByteArray ba){
Copy link
Contributor

Choose a reason for hiding this comment

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

Add function comments

@@ -27,6 +27,7 @@ class BaseEnclaveInfo {
const std::string& persisted_sealed_data,
const int num_of_enclaves);
virtual ~BaseEnclaveInfo();
void terminate_enclave();
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to have consistent function naming convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could not follow.

Copy link
Contributor

Choose a reason for hiding this comment

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

We have followed the convention of naming the function like TerminateEnclave() in cpp code. Good to keep the convention aligned. I am sure there are some places this convention is not followed, needs to be fixed.

@@ -79,7 +79,7 @@ EnclaveData::EnclaveData(const uint8_t* inSealedData) {
nullptr, 0, &decrypted_data[0], &decrypted_size);
tcf::error::ThrowSgxError(ret, "Failed to unseal data");
std::string decrypted_data_string(reinterpret_cast<const char*>(&decrypted_data[0]));
DeserializeSealedData(decrypted_data_string);
DeserializePrivateData(decrypted_data_string);
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to rename the function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is essentially deserializing only the private data.

@rranjan3 rranjan3 force-pushed the kme-replication branch 3 times, most recently from 5ded7b8 to c10a4b4 Compare April 9, 2021 04:37
@rranjan3 rranjan3 self-assigned this May 4, 2021
Implementation of workorder targetted for KME (primary/replica)
- state-uid
- state-request
- get-state
- set-state

Signed-off-by: Rajeev Ranjan <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants