Election-Tech-Initiative / Election-Tech-Initiative/electionguard-python

🐞 HashedElGamal is not compliant with the spec

Đang mở
#646 2 bình luận 0 reaction 0 người được giao Xem trên GitHub
bug enhancement
Ngôn ngữ chính
Python
Star
168
Fork
103
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Mô tả

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current Behavior

The code in `hmac.py` (in particular, the `get_hmac()` function) is being used as part of `HashedElGamalCiphertext` for two very different purposes.

For the first purpose, computing an HMAC, it seems to be correct. Example, where the spec says to use HMAC (e.g., $c_2 = HMAC(k_0, c_0 | c_1)$), the code says:
```python
mac = get_hmac(mac_key, to_mac)
```
This is fine.

The ElectionGuard spec notes that the actual encryption of the message ($c_1 = m_1 \oplus k_1 | m_2 \oplus k_2 | \cdots | m_{b_m} \oplus k_{b_m}$) is supposed to use a NIST 800-108-compliant *key derivation function* (KDF). The Python code is not compliant with the NIST spec. In particular, the code that creates those $k_i$ values does this:

```python
data_key = get_hmac(
session_key.to_hex_bytes(),
encryption_seed.to_hex_bytes(),
bit_length,
(i + 1),
)
```

So, what's going on inside `get_hmac`? Its input to the HMAC function is ultimately the byte concatenation of three values: `start_byte + msg + end_byte` where `start_byte` is the index $i$, and `end_byte` is the message length. The NIST spec says:
```
K(i) := PRF (KI, [i] || Label || 0x00 || Context || [L])
```

Suggested fixes below.

### Expected Behavior

It's confusing to use `get_hmac` to serve two radically different purposes. First and foremost, it would be good to have a `KDF` class that implements the NIST spec correctly and then use it to compute the $k_i$'s. Anyway, even if you don't do that, there are a few things that you do need to do:

- You're not doing anything for the NIST-defined `Label` or `Context` strings (okay for a start, but these strings serve a purpose and should be defined and used)
- You're not using the zero byte after the `Label` (Suggestion: add the byte.)
- You're using precisely 32 bits to encode the two numbers ($i$ and $L$), when it's possible (if unlikely) that this code might be used to encrypt more than 4 billion blocks. (Suggestion: use a variable-length encoding for lengths.)
- You should not be converting keys to hexadecimal bytes. Those are originally `ElementModQ` values, which already fit into 32 bytes. This encoding process yields more bits than HMAC-SHA256 normally supports for its key values. (Suggestion: extract the internal BigInteger as exactly 32 big-endian bytes.)

### Steps To Reproduce

_No response_

### Environment

_No response_

### Anything else?

_No response_

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.