Keep-142 AWS GO SDK Develop #3
Conversation
|
Hey, A general observation i noticed is that we are not providing logs when a certain checkpoint is met. For example,
We are providing great error logs for all errors but we do not have debug logs and such much. maybe check and add those if you think they make sense since we can use logs for decryption. |
integrations/aws/ReadME.md
Outdated
|
|
||
| * Supports the Go-Lang Secrets Manager SDK. | ||
| * Requires AWS packages: aws, config, credentials, kms, kms-types | ||
| * Works with just AES/RSA key types with `Encrypt` and `Decrypt` permissions. |
There was a problem hiding this comment.
Work with both Symmetric and Asymmetric(RSA) right?
There was a problem hiding this comment.
Yes Works with both of the types.
| } | ||
| convertedConfig := make(map[string]interface{}) | ||
| for k, v := range a.config { | ||
| convertedConfig[string(k)] = v |
There was a problem hiding this comment.
If a.config is a map[string]interface{} then k should already be a string, Do we really need to convert it string again here string(k)?
There was a problem hiding this comment.
a.config is type of map[core.ConfigKey]interface{}, We need to convert it to map[string]interface{}.
integrations/aws/storage.go
Outdated
| func (a *AWSKeyVaultStorage) Delete(key core.ConfigKey) map[string]interface{} { | ||
| if _, found := a.config[key]; found { | ||
| delete(a.config, key) | ||
| logger.Debugf("%s", "Removed key: "+string(key)) |
There was a problem hiding this comment.
logger.Debugf("Removed key: %s", key) will work if key is already string
There was a problem hiding this comment.
Key is core.Config present in Keeper-golang repository, so converting it into a string.
|
|
||
| func decryptSymmetric(client *kms.Client, keyId string, cipherText []byte) ([]byte, error) { | ||
| if keyId == "" { | ||
| return nil, fmt.Errorf("keyId is empty") |
There was a problem hiding this comment.
We should validate cipherText also if it is null or empty
There was a problem hiding this comment.
Added the condition.
| } | ||
|
|
||
| func (a *AWSKeyVaultStorage) saveConfig(updatedConfig map[core.ConfigKey]interface{}) error { | ||
| configJson, err := json.Marshal(a.config) |
There was a problem hiding this comment.
add the comment in code wherever needed like what the method do see here
https://github.com/Keeper-Security/secrets-manager-go/blob/master/core/core.go
There was a problem hiding this comment.
Added the comments.
| // createHash creates an MD5 hash of the provided config data. | ||
| func (a *AWSKeyVaultStorage) createHash(config []byte) string { | ||
| hash := md5.Sum(config) | ||
| return hex.EncodeToString(hash[:]) |
There was a problem hiding this comment.
Why can't we do return hex.EncodeToString(hash)?
No description provided.