Replace hard-coded GCM tag length with named constant #13162
Conversation
- Add GCM_STANDARD_TAG_SIZE constant (16 bytes) with NIST SP 800-38D reference - Replace magic numbers in tag allocation and validation logic - Update error message to use constant value for consistency - Improves code maintainability and follows crypto best practices Tested: GCM functionality preserved, no regressions Resolves TODO comments about hard-coded GCM tag length values
- Add GCM_STANDARD_TAG_SIZE constant (16 bytes) with NIST SP 800-38D reference - Replace magic numbers in tag allocation and validation logic - Update error message with defensive handling comment - Apply Rust formatting and linting standards Tested: GCM functionality preserved, no regressions Resolves TODO comments about hard-coded GCM tag length values
Oblivionsage
force-pushed
the
fix-gcm-tag-length-constants
branch
from
29a2cc3 to
1a10ce0
Compare
| // XXX: do not hard code 16 | ||
| let tag = pyo3::types::PyBytes::new_with(py, 16, |t| { | ||
| // Allocate buffer for GCM tag | ||
| let tag = pyo3::types::PyBytes::new_with(py, GCM_STANDARD_TAG_SIZE, |t| { |
There was a problem hiding this comment.
Sorry, but this isn't actually addressing the comment: the comment is saying that the PyAEADEncryptionContext should work for other tag sizes.
There was a problem hiding this comment.
@alex Thanks for the feedback I focused too much on eliminating the hard-coded value and overlooked the broader intent of supporting variable tag sizes
I'm happy to work on a more comprehensive solution, but I'd love some guidance on the approach you'd prefer should I:
- Research the tag size requirements for different AEAD algorithms(ChaCha20-Poly1305, AES-OCB, etc.) and make the allocation dynamic?
- Or would it be better to revert this and tackle the broader architectural change in a separate discussion first?
This is a great learning experience for me thanks for pointing me in the right direction
There was a problem hiding this comment.
I think it may just need to check against https://docs.rs/openssl/latest/openssl/cipher_ctx/struct.CipherCtx.html#method.tag_length
Tested: GCM functionality preserved, no regressions Resolves TODO comments about hard-coded GCM tag length values