Skip to content

Commit 0f889a9

Browse files
committed
Fix hash copy semantics and add tests
1 parent 17f3332 commit 0f889a9

5 files changed

Lines changed: 139 additions & 30 deletions

File tree

scripts/build_ffi.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,7 @@ def build_ffi(local_wolfssl, features):
575575
int wc_ShaUpdate(wc_Sha*, const byte*, word32);
576576
int wc_ShaFinal(wc_Sha*, byte*);
577577
void wc_ShaFree(wc_Sha*);
578+
int wc_ShaCopy(wc_Sha*, wc_Sha*);
578579
"""
579580

580581
if features["SHA256"]:
@@ -584,6 +585,7 @@ def build_ffi(local_wolfssl, features):
584585
int wc_Sha256Update(wc_Sha256*, const byte*, word32);
585586
int wc_Sha256Final(wc_Sha256*, byte*);
586587
void wc_Sha256Free(wc_Sha256*);
588+
int wc_Sha256Copy(wc_Sha256*, wc_Sha256*);
587589
"""
588590

589591
if features["SHA384"]:
@@ -593,6 +595,7 @@ def build_ffi(local_wolfssl, features):
593595
int wc_Sha384Update(wc_Sha384*, const byte*, word32);
594596
int wc_Sha384Final(wc_Sha384*, byte*);
595597
void wc_Sha384Free(wc_Sha384*);
598+
int wc_Sha384Copy(wc_Sha384*, wc_Sha384*);
596599
"""
597600

598601
if features["SHA512"]:
@@ -603,6 +606,7 @@ def build_ffi(local_wolfssl, features):
603606
int wc_Sha512Update(wc_Sha512*, const byte*, word32);
604607
int wc_Sha512Final(wc_Sha512*, byte*);
605608
void wc_Sha512Free(wc_Sha512*);
609+
int wc_Sha512Copy(wc_Sha512*, wc_Sha512*);
606610
"""
607611
if features["SHA3"]:
608612
cdef += """
@@ -623,6 +627,10 @@ def build_ffi(local_wolfssl, features):
623627
void wc_Sha3_256_Free(wc_Sha3*);
624628
void wc_Sha3_384_Free(wc_Sha3*);
625629
void wc_Sha3_512_Free(wc_Sha3*);
630+
int wc_Sha3_224_Copy(wc_Sha3*, wc_Sha3*);
631+
int wc_Sha3_256_Copy(wc_Sha3*, wc_Sha3*);
632+
int wc_Sha3_384_Copy(wc_Sha3*, wc_Sha3*);
633+
int wc_Sha3_512_Copy(wc_Sha3*, wc_Sha3*);
626634
"""
627635

628636
if features["DES3"]:

tests/test_aesgcmstream.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,3 +135,13 @@ def test_invalid_tag_bytes():
135135
# valid edge cases
136136
AesGcmStream(key, iv, tag_bytes=4)
137137
AesGcmStream(key, iv, tag_bytes=16)
138+
139+
def test_repeated_construction_destruction():
140+
import gc
141+
key = "fedcba9876543210"
142+
iv = "0123456789abcdef"
143+
for _ in range(1000):
144+
gcm = AesGcmStream(key, iv)
145+
gcm.encrypt("hello world")
146+
del gcm
147+
gc.collect()

tests/test_ciphers.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,3 +898,15 @@ def test_chacha_non_block_aligned():
898898
def test_chacha_invalid_key_length():
899899
with pytest.raises(ValueError, match="key must be"):
900900
ChaCha(b"\x00" * 20)
901+
902+
903+
if _lib.RSA_ENABLED:
904+
def test_encrypt_oaep_requires_hash_type(vectors):
905+
rsa = RsaPublic(vectors[RsaPublic].key)
906+
with pytest.raises(WolfCryptError, match="Hash type not set"):
907+
rsa.encrypt_oaep(b"plaintext")
908+
909+
def test_decrypt_oaep_requires_hash_type(vectors):
910+
rsa = RsaPrivate(vectors[RsaPrivate].key)
911+
with pytest.raises(WolfCryptError, match="Hash type not set"):
912+
rsa.decrypt_oaep(b"\x00" * rsa.output_size)

wolfcrypt/ciphers.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -396,9 +396,6 @@ class AesGcmStream(object):
396396
block_size = 16
397397
_key_sizes = [16, 24, 32]
398398
_native_type = "Aes *"
399-
_aad = bytes()
400-
_tag_bytes = 16
401-
_mode = None
402399

403400
def __init__(self, key, IV, tag_bytes=16):
404401
"""
@@ -408,20 +405,25 @@ def __init__(self, key, IV, tag_bytes=16):
408405
IV = t2b(IV)
409406
if tag_bytes < 4 or tag_bytes > 16:
410407
raise ValueError("tag_bytes must be between 4 and 16")
408+
# Per-instance state: AAD, tag length, and current mode (enc/dec).
409+
self._aad = bytes()
411410
self._tag_bytes = tag_bytes
411+
self._mode = None
412412
if len(key) not in self._key_sizes:
413413
raise ValueError("key must be %s in length, not %d" %
414414
(self._key_sizes, len(key)))
415+
self._init_done = False
415416
self._native_object = _ffi.new(self._native_type)
416417
ret = _lib.wc_AesInit(self._native_object, _ffi.NULL, -2)
417418
if ret < 0:
418419
raise WolfCryptError("AES init error (%d)" % ret)
420+
self._init_done = True
419421
ret = _lib.wc_AesGcmInit(self._native_object, key, len(key), IV, len(IV))
420422
if ret < 0:
421423
raise WolfCryptError("Init error (%d)" % ret)
422424

423425
def __del__(self):
424-
if hasattr(self, '_native_object'):
426+
if getattr(self, '_init_done', False):
425427
_lib.wc_AesFree(self._native_object)
426428

427429
def set_aad(self, data):
@@ -446,11 +448,11 @@ def encrypt(self, data):
446448
aad = self._aad
447449
elif self._mode == _DECRYPTION:
448450
raise WolfCryptError("Class instance already in use for decryption")
449-
self._buf = _ffi.new("byte[%d]" % (len(data)))
450-
ret = _lib.wc_AesGcmEncryptUpdate(self._native_object, self._buf, data, len(data), aad, len(aad))
451+
buf = _ffi.new("byte[%d]" % (len(data)))
452+
ret = _lib.wc_AesGcmEncryptUpdate(self._native_object, buf, data, len(data), aad, len(aad))
451453
if ret < 0:
452454
raise WolfCryptError("Encryption error (%d)" % ret)
453-
return bytes(self._buf)
455+
return bytes(buf)
454456

455457
def decrypt(self, data):
456458
"""
@@ -463,11 +465,11 @@ def decrypt(self, data):
463465
aad = self._aad
464466
elif self._mode == _ENCRYPTION:
465467
raise WolfCryptError("Class instance already in use for encryption")
466-
self._buf = _ffi.new("byte[%d]" % (len(data)))
467-
ret = _lib.wc_AesGcmDecryptUpdate(self._native_object, self._buf, data, len(data), aad, len(aad))
468+
buf = _ffi.new("byte[%d]" % (len(data)))
469+
ret = _lib.wc_AesGcmDecryptUpdate(self._native_object, buf, data, len(data), aad, len(aad))
468470
if ret < 0:
469471
raise WolfCryptError("Decryption error (%d)" % ret)
470-
return bytes(self._buf)
472+
return bytes(buf)
471473

472474
def final(self, authTag=None):
473475
"""
@@ -505,7 +507,9 @@ class ChaCha(_Cipher):
505507
_IV_nonce = b""
506508
_IV_counter = 0
507509

508-
def __init__(self, key="", size=32):
510+
def __init__(self, key="", size=32): # pylint: disable=unused-argument
511+
# size is kept for backwards compatibility; key length is now
512+
# derived from the actual key and validated against _key_sizes.
509513
self._native_object = _ffi.new(self._native_type)
510514
self._enc = None
511515
self._dec = None

wolfcrypt/hashes.py

Lines changed: 94 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,26 @@ def copy(self):
5656
Returns a separate copy of this hashing object. An update
5757
to this copy won't affect the original object.
5858
"""
59-
copy = self.new("")
60-
61-
_ffi.memmove(copy._native_object, # pylint: disable=protected-access
62-
self._native_object,
63-
self._native_size)
59+
# Bypass __init__ to avoid calling _init() on a state that _copy
60+
# immediately overwrites (which would leak internal resources in
61+
# async/HW-accelerated builds). Mark as shallow up front so __del__
62+
# skips the free if we bail out before the copy completes.
63+
copy = type(self).__new__(type(self))
64+
copy._shallow_copy = True # pylint: disable=protected-access
65+
copy._native_object = _ffi.new(self._native_type) # pylint: disable=protected-access
66+
67+
if hasattr(self, '_copy'):
68+
ret = self._copy(self._native_object,
69+
copy._native_object) # pylint: disable=protected-access
70+
if ret < 0: # pragma: no cover
71+
raise WolfCryptError("Hash copy error (%d)" % ret)
72+
copy._shallow_copy = False # pylint: disable=protected-access
73+
else:
74+
_ffi.memmove(copy._native_object, # pylint: disable=protected-access
75+
self._native_object,
76+
self._native_size)
77+
# Keep _shallow_copy = True: memmove shares internal state with
78+
# self, so __del__ must not free it separately.
6479

6580
return copy
6681

@@ -87,12 +102,26 @@ def digest(self):
87102

88103
if self._native_object:
89104
obj = _ffi.new(self._native_type)
90-
91-
_ffi.memmove(obj, self._native_object, self._native_size)
92-
93-
ret = self._final(obj, result)
94-
if ret < 0: # pragma: no cover
95-
raise WolfCryptError("Hash finalize error (%d)" % ret)
105+
used_deep_copy = hasattr(self, '_copy')
106+
107+
if used_deep_copy:
108+
ret = self._copy(self._native_object, obj)
109+
if ret < 0: # pragma: no cover
110+
raise WolfCryptError("Hash copy error (%d)" % ret)
111+
else:
112+
_ffi.memmove(obj, self._native_object, self._native_size)
113+
114+
try:
115+
ret = self._final(obj, result)
116+
if ret < 0: # pragma: no cover
117+
raise WolfCryptError("Hash finalize error (%d)" % ret)
118+
finally:
119+
# Only free when we did a deep copy; memmove'd temps share
120+
# internal resources with self and must not be separately freed.
121+
if used_deep_copy:
122+
delete = getattr(self, '_delete', None)
123+
if delete:
124+
delete(obj)
96125

97126
return _ffi.buffer(result, self.digest_size)[:]
98127

@@ -117,9 +146,11 @@ class Sha(_Hash):
117146
_native_type = "wc_Sha *"
118147
_native_size = _ffi.sizeof("wc_Sha")
119148
_delete = _lib.wc_ShaFree
149+
_copy = _lib.wc_ShaCopy
120150

121151
def __del__(self):
122-
self._delete(self._native_object)
152+
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
153+
self._delete(self._native_object)
123154

124155
def _init(self):
125156
return _lib.wc_InitSha(self._native_object)
@@ -143,9 +174,11 @@ class Sha256(_Hash):
143174
_native_type = "wc_Sha256 *"
144175
_native_size = _ffi.sizeof("wc_Sha256")
145176
_delete = _lib.wc_Sha256Free
177+
_copy = _lib.wc_Sha256Copy
146178

147179
def __del__(self):
148-
self._delete(self._native_object)
180+
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
181+
self._delete(self._native_object)
149182

150183
def _init(self):
151184
return _lib.wc_InitSha256(self._native_object)
@@ -169,9 +202,11 @@ class Sha384(_Hash):
169202
_native_type = "wc_Sha384 *"
170203
_native_size = _ffi.sizeof("wc_Sha384")
171204
_delete = _lib.wc_Sha384Free
205+
_copy = _lib.wc_Sha384Copy
172206

173207
def __del__(self):
174-
self._delete(self._native_object)
208+
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
209+
self._delete(self._native_object)
175210

176211
def _init(self):
177212
return _lib.wc_InitSha384(self._native_object)
@@ -195,9 +230,11 @@ class Sha512(_Hash):
195230
_native_type = "wc_Sha512 *"
196231
_native_size = _ffi.sizeof("wc_Sha512")
197232
_delete = _lib.wc_Sha512Free
233+
_copy = _lib.wc_Sha512Copy
198234

199235
def __del__(self):
200-
self._delete(self._native_object)
236+
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
237+
self._delete(self._native_object)
201238

202239
def _init(self):
203240
return _lib.wc_InitSha512(self._native_object)
@@ -232,14 +269,27 @@ class Sha3(_Hash):
232269
64: _lib.wc_Sha3_512_Free,
233270
}
234271

272+
_SHA3_COPY = {
273+
28: _lib.wc_Sha3_224_Copy,
274+
32: _lib.wc_Sha3_256_Copy,
275+
48: _lib.wc_Sha3_384_Copy,
276+
64: _lib.wc_Sha3_512_Copy,
277+
}
278+
235279
def __del__(self):
236-
if hasattr(self, '_delete'):
280+
# Unlike the SHA-1/2 classes, Sha3's _delete is set per-instance
281+
# from a size->function dict and is None for invalid sizes, so
282+
# we need the extra truthiness check.
283+
if (hasattr(self, '_native_object')
284+
and not getattr(self, '_shallow_copy', False)
285+
and getattr(self, '_delete', None)):
237286
self._delete(self._native_object)
238287

239288
def __init__(self, string=None, size=SHA3_384_DIGEST_SIZE): # pylint: disable=W0231
240289
self._native_object = _ffi.new(self._native_type)
241290
self.digest_size = size
242291
self._delete = self._SHA3_FREE.get(size)
292+
self._copy = self._SHA3_COPY.get(size)
243293
ret = self._init()
244294
if ret < 0: # pragma: no cover
245295
raise WolfCryptError("Sha3 init error (%d)" % ret)
@@ -251,8 +301,24 @@ def new(cls, string=None, size=SHA3_384_DIGEST_SIZE):
251301
return cls(string, size)
252302

253303
def copy(self):
254-
c = Sha3(size=self.digest_size)
255-
_ffi.memmove(c._native_object, self._native_object, self._native_size)
304+
# Bypass __init__ to avoid calling _init() on a state that _copy
305+
# immediately overwrites (which would leak internal resources in
306+
# async/HW-accelerated builds). Mark as shallow up front so
307+
# __del__ skips the free if we bail out before the copy completes.
308+
c = type(self).__new__(type(self))
309+
c._shallow_copy = True
310+
c._native_object = _ffi.new(self._native_type)
311+
c.digest_size = self.digest_size
312+
c._delete = self._delete
313+
c._copy = self._copy
314+
if self._copy:
315+
ret = self._copy(self._native_object, c._native_object)
316+
if ret < 0: # pragma: no cover
317+
raise WolfCryptError("Hash copy error (%d)" % ret)
318+
c._shallow_copy = False
319+
else:
320+
_ffi.memmove(c._native_object, self._native_object, self._native_size)
321+
# Keep _shallow_copy = True: memmove shares state with self.
256322
return c
257323

258324
def _init(self):
@@ -309,14 +375,23 @@ class _Hmac(_Hash):
309375
"""
310376
A **PEP 247: Cryptographic Hash Functions** compliant
311377
**Keyed Hash Function Interface**.
378+
379+
Note: wolfSSL does not provide a `wc_HmacCopy` equivalent, so
380+
`copy()` falls back to a byte-level memmove. In default builds the
381+
Hmac struct is self-contained and this is safe. In async or
382+
hardware-accelerated builds where the struct contains internal
383+
pointers, the copy shares those pointers with the original; the
384+
copy must not outlive the original or be used after the original
385+
is freed.
312386
"""
313387
digest_size = None
314388
_native_type = "Hmac *"
315389
_native_size = _ffi.sizeof("Hmac")
316390
_delete = _lib.wc_HmacFree
317391

318392
def __del__(self):
319-
self._delete(self._native_object)
393+
if hasattr(self, '_native_object') and not getattr(self, '_shallow_copy', False):
394+
self._delete(self._native_object)
320395

321396
def __init__(self, key, string=None): # pylint: disable=W0231
322397
key = t2b(key)

0 commit comments

Comments
 (0)