Red Hat Bugzilla – Bug 474394
crypto: des3_ede single-key doesn't work
Last modified: 2009-09-02 04:59:21 EDT
Description of problem:
Crypto operations performed with des3_ede fail to work correctly when a single key is used. For example, the following ecb des3_ede test vectors...
KEYs = 8001010101010101
PLAINTEXT = 0000000000000000
KEYs = 4001010101010101
PLAINTEXT = 0000000000000000
KEYs = 2001010101010101
PLAINTEXT = 0000000000000000
...all produce the same incorrect result of 8ca64de9c1b123a7. The results should match the results of running these same vectors through ecb des (which respectively, are 95a8d72813daa94d, 0eec1487dd8c26d5 and 7ad16ffb79c45926).
Based on a quick read through des.c (and upstream des_generic.c), I thought I might have happened upon a fix, which is to not reject a weak key in des3_ede_setkey() if the weak key flag is set. This change actually *has* resulted in cbc(des3_ede) test vectors returning the correct results now, but for reasons as yet unknown, ecb(des3_ede) is still returning the bogus results.
FIPS CAVS testing and certification requires that this works as expected.
Version-Release number of selected component (if applicable):
Created attachment 325561 [details]
Patch that got cbc(des3_ede) working
Huh? Weak keys are totally unacceptable to 3DES because they reduce it to DES. I'd like to see where FIPS requires this. Thanks.
Having reread your description, I think your patch does make sense. If the user wants insecurity, we should always give it to them.
Regarding the ecb(des3_ede) problem, what exactly does it produce for you? Do you have some code that I can compile and run? Thanks!
Turns out I was chasing two different problems at the same time. One was simply my test infra not expanding the fips cavs single-key tdes inputs into three keys. The other is the weak key flag. I've got both cbc and ecb mode working reliably now.
However, I think the flag check in des_setkey and the one I added in my patch in comment #1 to des3_ede_setkey are actually inverted. That flag should only be set if we want to permit weak keys, and the check returns true if the flag is set, which I believe is the opposite of what is intended.
I'm building this on RHEL5 w/the following caveats:
-the kernel module also needs rng.h, which isn't yet being installed as part of kernel-devel (Neil is working on a fix for this)
-a few kernel patches atop the 5.3 kernel are required, all of which have been posted to rhkl
-the userspace requires a newer libnl than we ship in RHEL5 (1.1 plus a 2.6.18 compatibility patch).
Haven't actually tested w/a current upstream kernel yet, on my TODO list once everything is ironed out on the RHEL5 side... The dsa signature verification bits definitely have to be disabled for upstream kernels, since the RHEL5 module signing patches aren't upstream.
Once everything's built, load the crypto-tester-kmod.ko, then:
crypto-tester -m cipher -c "cbc(des3_ede)" -e -k 8001010101010101 -i 0000000000000000 0000000000000000
The help text should be reasonably accurate. I can provide RHEL5 libnl and kernel srpms w/most of the necessary bits included, if so desired.
Created attachment 325733 [details]
Updated des weak key patch
So yeah, I'm pretty sure the flag check is inverted now, unless I'm misinterpreting what the flag actually means. I've added a 'reject_weak_keys' flag to my test kmod, and built a new kernel with this patch.
# insmod crypto-tester-kmod.ko
crypto_tester: kernel module loaded
* Weak key checks disabled
[module loaded, sets the weak key flag for all blkcipher ops]
# crypto-tester -m cipher -c "ecb(des)" -e -k 8001010101010101 0000000000000000
[good, got expected result for des here]
# crypto-tester -m cipher -c "ecb(des3_ede)" -e -k 800101010101010180010101010101018001010101010101 0000000000000000
[good, tdes w/the same key 3x returns the same result as des]
# rmmod crypto-tester-kmod.ko ; insmod crypto-tester-kmod.ko reject_weak_keys=1
[module reloaded w/weak key flag not set]
[root@xaphod kmod]# crypto-tester -m cipher -c "ecb(des3_ede)" -e -k 800101010101010180010101010101018001010101010101 0000000000000000
des3_ede_setkey: weak key rejected
[the incorrect result, w/reason in dmesg]
Submitting two patches upstream:
-one to fix the inverted weak key flag check in des_setkey()
-one to add a weak key flag check to des3_ede_setkey()
Created attachment 325761 [details]
v3 of weak keys patch
Just noticed that testmgr's test 5 for des-generic now reports a setkey failure. Updated patch sent upstream, rhel5 version attached here.
The existing WEAK flag convention might seem unusual, but it is self-consistent. All implementations and users follow this rule. So you can't just change it now.
Please submit your original patch instead. Thanks!
Created attachment 325900 [details]
Latest rhel5 version of weak keys patch submitted upstream
Okay, submitted an updated version of the original patch, based on Herbert's comments upstream, which also changes the RES flag to something more appropriate (which matches des_setkey()).
Finally managed to get the patch right, has been committed upstream.
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux maintenance release. Product Management has requested
further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed
products. This request is not yet committed for inclusion in an Update
Updating PM score.
You can download this test kernel from http://people.redhat.com/dzickus/el5
Please do NOT transition this bugzilla state to VERIFIED until our QE team
has sent specific instructions indicating when to do so. However feel free
to provide a comment indicating that this fix has been verified.
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.