Closed BSI-TF-CWA closed 3 years ago
agaricusbisporus
commented
3 years ago Thank you for feedback. We already implemented a new procedure requested by our security department (having the same requirements) which will solve this issue. Now we are testing and hope to be able to deploy next days.
agaricusbisporus
commented
3 years ago @BSI-TF-CWA pls see https://github.com/corona-warn-app/cwa-quick-test-backend/pull/70 - I think it could close some open point. Any other findings or feedback welcome.
agaricusbisporus
commented
3 years ago
Rating: Medium
Description: The quick test backend server stores sensitive user data linked to a quick test in a database. The entries of this database are then protected by encryption. For this, the server uses AES in CBC mode. The encryption implementation shows the following flaws: Misleading Variable and Configuration Name The encryption key is internally called Password, both in code in form of a variable as well as in the server’s configuration. However, passwords and keys have different requirements in terms of complexity, length, or storage. During future development, this confusion could lead to false assumptions and potential security problems. To give an example, the fact that the key’s length is checked to be either 16, 24, or 32 is an unusual, rather insecure requirement for a password, but valid for a block cipher key. CBC Mode with Hardcoded Static IV The CBC mode in combination with a static hardcoded initialization vector leads to the issue that the same values (plaintexts) result in the same ciphertexts. Given that each column is encrypted separately, all positive test results, for example, have the same value in the database. This also applies to the other fields, such as first names, last names, email, phone number, or the date of birth. This highly impacts the confidentiality of the encryption. Various statistical attacks could be run against the database, such as a frequency distribution over the first names. In combination with the many different properties of a test result, it can be possible to infer sensitive user information.
Proof of Concept: The following lines show, taken from src/main/.../DbEncryptionService.java:54 show the usage of the term password:
The following lines in the code, taken from src/main/.../ DbEncryptionService.java:245 show that a static hardcoded initialization vector is used.
Controls: Variable Naming It is recommended to rename the variables and configuration items to better reflect that they represent a key. While this may not necessarily lead to an actual problem, it is advisable to still mitigate it as soon as possible, to prevent potential damage in the future. Encryption Mechanism In general, a static initialization vector should not be used. This is especially important for the CBC mode. A solution could be to use an initialization vector per test result row in the database. This would ensure that, for example, the same names in different tests do not result in the same cipher texts. However, in general, encrypting each column value separately might introduce an unnecessarily high overhead. Another option could be to encrypt the whole database. This would rule out any statistical attacks against individual values of the database.