republique-et-canton-de-geneve / chvote-1-0

The Geneva electronic vote system, version 1.
https://republique-et-canton-de-geneve.github.io/chvote-1-0
GNU Affero General Public License v3.0
750 stars 68 forks source link

Same key used in encrypt() and BuildMAC() #22

Closed EdOverflow closed 7 years ago

EdOverflow commented 7 years ago

You do not appear to be using separate keys for AES encryption and the HMAC.

BuildMAC()

mac.init(config.getSecretKey());

Link to source code: https://github.com/republique-et-canton-de-geneve/chvote-1-0/blob/master/commons-base/commons-crypto/src/main/java/ch/ge/ve/commons/crypto/SensitiveDataCryptoUtils.java#L180

encrypt()

SecretKey secretKey = config.getSecretKey();
cipher.init(Cipher.ENCRYPT_MODE, secretKey, SecureRandomFactory.createPRNG());

Link to source code: https://github.com/republique-et-canton-de-geneve/chvote-1-0/blob/master/commons-base/commons-crypto/src/main/java/ch/ge/ve/commons/crypto/SensitiveDataCryptoUtils.java#L272

chvote-etat-de-geneve commented 7 years ago

Hi, Once again, thank you for your interest and feedback.

As mentioned in the javadoc header of that class, those elements are only ever used for securing the sensitive elements stored in the database. See https://github.com/republique-et-canton-de-geneve/chvote-1-0/blob/master/commons-base/commons-crypto/src/main/java/ch/ge/ve/commons/crypto/SensitiveDataCryptoUtils.java#L51

In this case, the one thing we'd like to avoid is leakage of sensitive data in the case of a compromised database. The key is held by the server, which has a different set of administrators and credentials than the database.

Some elements are simply hashed, because they only need to be verified, while others are encrypted, because they must be read at one point or the other. (Mostly for exporting anonymous statistics, like participation levels across age ranges.)

In this instance, we are not aware of a reason to use separate keys, since the trust boundaries are identical in both cases.

Should you have any information to the contrary, we would gladly look into it.

EdOverflow commented 7 years ago

In that case, everything looks good to me. I just wanted to double check with you.