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
744 stars 67 forks source link

Use MessageDigest.isEquals() instead of Arrays.equals() when comparing MAC #10

Closed kaworu closed 7 years ago

kaworu commented 7 years ago

In order to avoid timing attacks, comparing MAC digest must be performed in constant time.

see https://codahale.com/a-lesson-in-timing-attacks/

chvote-etat-de-geneve commented 7 years ago

We definitely agree it is more in line with best practices, and a vulnerability might occur in other settings. However, several other layers of defense would prevent this attack to be carried out on our website...

Most of the source code published is only ever used on one laptop, disconnected from the network, and with all tasks being carried out by either sworn-in personnel, or elected representatives. In that case, timing attacks are mostly a moot point.

The specific code you mention, is however also used on the application running the voting website, which is exposed to the public. There, timing attacks are far more of a concern, but thresholds mechanisms on invalid requests would prevent this particular flaw from resulting in an exploitable vulnerability.

Nonetheless, it is important to us to be in line with best practices, and to conform to a defense in depth principle. We therefore thank you for pointing this out and will take it into account.