Conversation
71d72b2 to
9e47b3a
Compare
karel-m
left a comment
There was a problem hiding this comment.
I have committed my suggestions as separate commits.
|
The updated version with my changes not only passes all libtomcrypt tests, but also my perl CryptX test suite. |
sjaeckel
left a comment
There was a problem hiding this comment.
Thanks a lot for picking this up and working on it! Very much appreciated :)
For now just one question regarding your latest changes.
Why the switch back to the resolved index instead of using the hash name? I like the string-based "way of working" a bit better nowadays, even though it complicates the internal implementation a bit. (Which could certainly be improved by some nifty helpers.)
I still hope we get rid of the descriptors at one point, so I thought we could already future-proof the implementation, but maybe there's another reason I'm not aware of?
|
ad hash idx vs hash name - it's just that I didn't like those extra round-trips name>>idx>>name (I have no problem with reverting it) |
hmm, I guess we should think in light of "how would resp. could it work if there was no registry" ... which will be "interesting" ... In that context I just had a look at #515 again and I'm still in favor of merging that one, but now I'm not so sure anymore whether we should go forward and remove all registries, since as @levitte put it "[...] it does cut away the possibility for more complex systems, where there's a part that needs to be able to find implementations by name [...]", which we currently see here. Therefore let's keep this as it is now and we will see how it evolves. |
Update PKCS#1-PSS and RSA APIs that allow passing a separate hash index for the MGF1 hash. Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
Slightly minimize both space and time when importing a
SubjectPublicKeyInfo. Time for ECC keys stays the same.
Those tests were done with X.509 support already available, but later these
commits were split up to be independent of the X.509 feature.
Running the entire set of pem files through `x509_verify` via [0]
resp. the timing app via [1] resulted in the following data:
Before this patch:
[0]
```
==1031519== HEAP SUMMARY:
==1031519== in use at exit: 0 bytes in 0 blocks
==1031519== total heap usage: 424,057 allocs, 424,057 frees, 73,527,730 bytes allocated
```
[1]
```
x509 cert-rsa-pss.pem : 50021 cycles
x509 LTC_CA.pem : 10335 cycles
x509 LTC_S0.pem : 47284 cycles
x509 LTC_SS0.pem : 36687 cycles
x509 secp384r1.pem : 1985416 cycles
x509 secp521r1.pem : 3287773 cycles
x509 LTC_SSS0.pem : 25086 cycles
x509 secp224r1.pem : 775807 cycles
```
After this patch:
[0]
```
==1043548== HEAP SUMMARY:
==1043548== in use at exit: 0 bytes in 0 blocks
==1043548== total heap usage: 337,244 allocs, 337,244 frees, 65,047,463 bytes allocated
```
[1]
```
x509 cert-rsa-pss.pem : 32568 cycles
x509 LTC_CA.pem : 5478 cycles
x509 LTC_S0.pem : 36093 cycles
x509 LTC_SS0.pem : 23351 cycles
x509 secp384r1.pem : 1984030 cycles
x509 secp521r1.pem : 3303396 cycles
x509 LTC_SSS0.pem : 13220 cycles
x509 secp224r1.pem : 781534 cycles
```
[0] find tests/x509 -name '*.pem' -exec valgrind --leak-check=full --show-leak-kinds=all './x509_verify' {} \+
[1] ./timing x509
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
This also: a) deprecates the old RSA and PKCS#1 API. b) reverts the changes done to them in order to make the now deprecated API compatible again with the last release. The fixes commit mentioned below is the testcase for the Bleichenbacher attack, which works now again as expected. Fixes: 9d03c38 ("add flags to `der_decode_sequence()`") Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
…ers; now lives on rsa_key where it belongs)
Introduce the RSA API v2.
Follow-up of #699 which already contains some comments regarding the new API.
Checklist