Key derivation without salt #529

New issue

Open

opened 2024-10-30 09:17:09 +00:00 by potyarkin · 0 comments

potyarkin commented

2024-10-30 09:17:09 +00:00

Member

This has a small potential to turn out to be a security issue, but most likely it's benign.

 func deriveKey(secret []byte) ([]byte, error) {
 	hash := sha256.New
 	kdf := hkdf.New(hash, secret, nil, nil)
 	key := make([]byte, 32)
 	_, err := io.ReadFull(kdf, key)
 	return key, err
 }

Currently we derive a 32-byte encryption key from a 32-byte ECDH shared secret without adding any salt or app info. This means that we map every ECDH SK to a single encryption key, 1-to-1, which negates most of the benefits that would be provided by using KDF. KDF still scrambles the input in a pseudo-random way, so it's not entirely useless here.

Adding salt would require us to store it somewhere inside AccessBox and thus would be a breaking change. I do not see any exploitation path here, so it's likely OK to be left as it is.

Bringing it up for discussion, hope others will chime in with their expertise.

This has a small potential to turn out to be a security issue, but most likely it's benign. https://git.frostfs.info/TrueCloudLab/frostfs-s3-gw/src/commit/424038de6cf583a1d49bb11418e493f99a923f21/creds/accessbox/accessbox.go#L276-L282 Currently we derive a 32-byte encryption key from a 32-byte ECDH shared secret without adding any salt or app info. This means that we map every ECDH SK to a single encryption key, 1-to-1, which negates most of the benefits that would be provided by using KDF. KDF still scrambles the input in a pseudo-random way, so it's not entirely useless here. Adding salt would require us to store it somewhere inside AccessBox and thus would be a breaking change. I do not see any exploitation path here, so it's likely OK to be left as it is. Bringing it up for discussion, hope others will chime in with their expertise.