feat: add api to list secrets #141
Conversation
Pull Request Test Coverage Report for Build 1839454157
💛 - Coveralls |
ravisuhag
requested review from
arinda-arif and
kushsharma
and removed request for
arinda-arif
store/postgres/secret_repository.go
Outdated
| return models.SecretItemInfo{}, err | ||
| } | ||
|
|
||
| digest := cryptopasta.Hash("secret", cleartext) |
There was a problem hiding this comment.
As far as I remember, in RFC we mentioned digest should be generated from encrypted secret not plain text otherwise there are chances of exploitation.
There was a problem hiding this comment.
The question we had was for time, when the app key changes, at that time all the secret digests will change.
The hash function we are using is SHA512 which cannot be easily broken by brute-force attacks.
cc @sravankorumilli
There was a problem hiding this comment.
APP_KEY should never change, losing APP_KEY means invalidating all the secrets.
The hash function we are using is SHA512 which cannot be easily broken by brute-force attacks.
What? Yes, it is? The brute force depends on the secret length, short secrets can be broken in less than 10 mins and you don't even need brute force, exploitation will start from a rainbow dictionary attack.
I am really against generating digests from raw passwords.
There was a problem hiding this comment.
Need to check on the rainbow dictionary attack, but I agree it would be good to generate hashes on the encrypted secrets
store/postgres/secret_repository.go
Outdated
| return specs, err | ||
| if err := repo.db.WithContext(ctx).Preload("Namespace"). | ||
| Joins("LEFT JOIN namespace ON secret.namespace_id = namespace.id"). | ||
| Where("secret.project_id = ? and secret.type = 'user'", repo.project.ID).Find(&resources).Error; err != nil { |
There was a problem hiding this comment.
we should use enum here for secret type instead of hardcoded string user
There was a problem hiding this comment.
Sure, will use the enum
kushsharma
changed the title
No description provided.