From 4b5a656b006402feec034b14486b122094b03843 Mon Sep 17 00:00:00 2001 From: kakaxi Date: Fri, 3 May 2024 10:21:28 +0800 Subject: [PATCH 1/3] add SIP-keypairs-des-128 --- .idea/.gitignore | 8 ++ .idea/modules.xml | 8 ++ .idea/sips.iml | 9 ++ .idea/vcs.xml | 6 ++ sips/sip-temporary_keypairs-des-128.md | 123 +++++++++++++++++++++++++ 5 files changed, 154 insertions(+) create mode 100644 .idea/.gitignore create mode 100644 .idea/modules.xml create mode 100644 .idea/sips.iml create mode 100644 .idea/vcs.xml create mode 100644 sips/sip-temporary_keypairs-des-128.md diff --git a/.idea/.gitignore b/.idea/.gitignore new file mode 100644 index 0000000..13566b8 --- /dev/null +++ b/.idea/.gitignore @@ -0,0 +1,8 @@ +# Default ignored files +/shelf/ +/workspace.xml +# Editor-based HTTP Client requests +/httpRequests/ +# Datasource local storage ignored files +/dataSources/ +/dataSources.local.xml diff --git a/.idea/modules.xml b/.idea/modules.xml new file mode 100644 index 0000000..867ea16 --- /dev/null +++ b/.idea/modules.xml @@ -0,0 +1,8 @@ + + + + + + + + \ No newline at end of file diff --git a/.idea/sips.iml b/.idea/sips.iml new file mode 100644 index 0000000..5e764c4 --- /dev/null +++ b/.idea/sips.iml @@ -0,0 +1,9 @@ + + + + + + + + + \ No newline at end of file diff --git a/.idea/vcs.xml b/.idea/vcs.xml new file mode 100644 index 0000000..94a25f7 --- /dev/null +++ b/.idea/vcs.xml @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/sips/sip-temporary_keypairs-des-128.md b/sips/sip-temporary_keypairs-des-128.md new file mode 100644 index 0000000..5349280 --- /dev/null +++ b/sips/sip-temporary_keypairs-des-128.md @@ -0,0 +1,123 @@ +| SIP-Number | | +| ---: |:-----------------------------------------------------------------------------------------------| +| Title | Keypairs in files should be aes-128 encode | +| Description | Currently Sui store keypair in file only using base64 which should using aes-128 encode better | +| Author | oday0311@hotmail.com | +| Editor | | +| Type | Standard | +| Category | Core | +| Created | 2024-05-03 | +| Comments-URI | | +| Status | | +| Requires | | + +## Abstract + +Sui currently store keypairs in files[sui.keystore,network.yaml,fullnode.yaml] only using base64 which should using aes-128 encode better. + +## Motivation + +base64 is not secure enough to store keypair in files, it should use aes-128 encode better. +there are some cases, we don't want to display the base64 directly in the files, +by add the default aes-encode, developers can compile the bin files with their own encrypt password, +so instead of save base64 content , I think aes-encode content with user-custom aes-128 result is more reasonable. +in fact, similar cases exist in the config files, network.yaml, fullnode.yaml, all the key-pair currently only save as base-64, +but key-pairs content is less used in normal developer, but cli-key tool is widly use for +dapps developers in their server for deploy dapps. + +## Specification +1. for keystore files, we should add a step before we save base64 content to files, + FileBasedKeystore + //add aes-128-cbc default encryption + let encode_data = default_des_128_encode(store.as_bytes()); + fs::write(path, encode_data)?; + and before new from files, keystore shoud try to decode from des-128: + if contents.starts_with("[") { + kp_strings = serde_json::from_str(&*contents) + .map_err(|e| anyhow!("Can't deserialize FileBasedKeystore from {:?}: {e}", path))?; + }else { + let decode_data = default_des_128_decode(contents); + kp_strings = serde_json::from_str(&*decode_data) + .map_err(|e| anyhow!("Can't deserialize FileBasedKeystore from {:?}: {e}", path))?; + } +2. for fullnode.yaml , network.yaml files, we should change the serder function + #[serde(default = "default_authority_key_pair")] + #[serde(serialize_with = "serialize_with_aes_encode_authoritykey")] + #[serde(deserialize_with = "deserialize_with_aes_encode_authoritykey")] + pub protocol_key_pair: AuthorityKeyPairWithPath, + + #[serde(default = "default_key_pair")] + #[serde(serialize_with = "serialize_with_aes_encode")] + #[serde(deserialize_with = "deserialize_with_aes_encode")] + pub worker_key_pair: KeyPairWithPath, + #[serde(default = "default_key_pair")] + #[serde(serialize_with = "serialize_with_aes_encode")] + #[serde(deserialize_with = "deserialize_with_aes_encode")] + pub account_key_pair: KeyPairWithPath, + #[serde(default = "default_key_pair")] + #[serde(serialize_with = "serialize_with_aes_encode")] + #[serde(deserialize_with = "deserialize_with_aes_encode")] + pub network_key_pair: KeyPairWithPath, + + the deserialize_with_aes_encode_xxx is a little complex, for we need to support both string and mapping type, + fn deserialize_with_aes_encode_authoritykey<'de, D>(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let value: serde_yaml::Value = match serde_yaml::Value::deserialize(deserializer) { + Ok(value) => value, + Err(_) => return Err(D::Error::custom("Failed to deserialize value as YAML")), + }; + + match value { + serde_yaml::Value::String(mut s) => { + if s.starts_with(DEFAULT_AES_PREFIX) { + s = node_des_128_decode(s); + } + let keypair = ::decode_base64(&s); + + Ok(AuthorityKeyPairWithPath::new(keypair.unwrap())) + } + serde_yaml::Value::Mapping(map) => { + let path = map.get(&Value::String("path".parse().unwrap())).ok_or_else(|| D::Error::custom("Missing path"))?; + println!("path: {:?}", path); + let keypair = read_authority_keypair_from_file(path.as_str().unwrap()).map_err(|e| D::Error::custom(format!("Failed to read keypair from file: {}", e)))?; + Ok(AuthorityKeyPairWithPath::new(keypair)) + } + _ => { + Err(D::Error::custom("Invalid value type, expected string or mapping")) + } + } + + +} + + +## Rationale +1.sui.keystore files comes from FileBasedKeystore::save , it should be aes-128 encode before save to files. also add decode step before new from files. +2. for network.yaml,and fullnode.yaml, we should change the serder function to support both string and mapping type, +and add aes-128 encode/decode step before save to files and new from files. + + + +## Backwards Compatibility + +There are no issues with backwards compatability. + + + +## Test Cases + +1. rosetta test : read_prefunded_account + +## Reference Implementation + + + +## Security Considerations + + + +## Copyright +[CC0 1.0](../LICENSE.md). + From 9a19a93af9590e87046b2c2456d850ad64df2096 Mon Sep 17 00:00:00 2001 From: eis Date: Sat, 10 Aug 2024 13:33:06 +0300 Subject: [PATCH 2/3] Set to Draft --- sips/sip-temporary_keypairs-des-128.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sips/sip-temporary_keypairs-des-128.md b/sips/sip-temporary_keypairs-des-128.md index 5349280..8f051aa 100644 --- a/sips/sip-temporary_keypairs-des-128.md +++ b/sips/sip-temporary_keypairs-des-128.md @@ -1,15 +1,15 @@ -| SIP-Number | | +| SIP-Number | 21 | | ---: |:-----------------------------------------------------------------------------------------------| -| Title | Keypairs in files should be aes-128 encode | +| Title | Store keypairs with aes-128 encryption | | Description | Currently Sui store keypair in file only using base64 which should using aes-128 encode better | | Author | oday0311@hotmail.com | -| Editor | | +| Editor | Alex Tsiliris | | Type | Standard | | Category | Core | | Created | 2024-05-03 | -| Comments-URI | | -| Status | | -| Requires | | +| Comments-URI | https://sips.sui.io/comments-21 | +| Status | Draft | +| Requires | - | ## Abstract From f32b4d45be41bdcdc8a49a37ffea60861d061934 Mon Sep 17 00:00:00 2001 From: eis Date: Mon, 19 Aug 2024 17:06:20 +0300 Subject: [PATCH 3/3] Fix title, description and filename --- sips/{sip-temporary_keypairs-des-128.md => sip-21.md} | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) rename sips/{sip-temporary_keypairs-des-128.md => sip-21.md} (94%) diff --git a/sips/sip-temporary_keypairs-des-128.md b/sips/sip-21.md similarity index 94% rename from sips/sip-temporary_keypairs-des-128.md rename to sips/sip-21.md index 8f051aa..4952cec 100644 --- a/sips/sip-temporary_keypairs-des-128.md +++ b/sips/sip-21.md @@ -1,7 +1,8 @@ | SIP-Number | 21 | | ---: |:-----------------------------------------------------------------------------------------------| -| Title | Store keypairs with aes-128 encryption | -| Description | Currently Sui store keypair in file only using base64 which should using aes-128 encode better | +| Title | Encrypt keypairs with aes-128 before storing them on disk. | +| Description | Currently Sui stores keypair in a file only using base64 which is not safe. It should be using | +| aes-128 encryptiption for better security. | | Author | oday0311@hotmail.com | | Editor | Alex Tsiliris | | Type | Standard |