Bug 474394 - crypto: des3_ede single-key doesn't work
crypto: des3_ede single-key doesn't work
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: kernel (Show other bugs)
5.4
All Linux
low Severity medium
: rc
: ---
Assigned To: Jarod Wilson
Red Hat Kernel QE team
:
Depends On:
Blocks: FIPS-140-Tracker
  Show dependency treegraph
 
Reported: 2008-12-03 11:55 EST by Jarod Wilson
Modified: 2009-09-02 04:59 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-09-02 04:59:21 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch that got cbc(des3_ede) working (552 bytes, patch)
2008-12-03 11:56 EST, Jarod Wilson
no flags Details | Diff
Updated des weak key patch (1011 bytes, patch)
2008-12-04 14:47 EST, Jarod Wilson
no flags Details | Diff
v3 of weak keys patch (2.10 KB, patch)
2008-12-04 18:28 EST, Jarod Wilson
no flags Details | Diff
Latest rhel5 version of weak keys patch submitted upstream (1.27 KB, patch)
2008-12-05 15:08 EST, Jarod Wilson
no flags Details | Diff

  None (edit)
Description Jarod Wilson 2008-12-03 11:55:04 EST
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):
2.6.18-124.el5
Comment 1 Jarod Wilson 2008-12-03 11:56:12 EST
Created attachment 325561 [details]
Patch that got cbc(des3_ede) working
Comment 2 Herbert Xu 2008-12-04 07:27:04 EST
Huh? Weak keys are totally unacceptable to 3DES because they reduce it to DES.  I'd like to see where FIPS requires this.  Thanks.
Comment 3 Herbert Xu 2008-12-04 07:29:47 EST
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!
Comment 4 Jarod Wilson 2008-12-04 11:11:37 EST
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.


Test code:

http://git.engineering.redhat.com/?p=users/jwilson/fips-cavs-kernel-test.git;a=summary

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.
Comment 5 Jarod Wilson 2008-12-04 14:47:18 EST
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
95a8d72813daa94d

[good, got expected result for des here]

# crypto-tester -m cipher -c "ecb(des3_ede)" -e -k 800101010101010180010101010101018001010101010101 0000000000000000
95a8d72813daa94d

[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
8ca64de9c1b123a7

[the incorrect result, w/reason in dmesg]
Comment 6 Jarod Wilson 2008-12-04 16:39:46 EST
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()
Comment 7 Jarod Wilson 2008-12-04 18:28:37 EST
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.
Comment 8 Herbert Xu 2008-12-04 22:20:42 EST
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!
Comment 9 Jarod Wilson 2008-12-05 15:08:56 EST
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()).
Comment 10 Jarod Wilson 2008-12-17 09:30:13 EST
Finally managed to get the patch right, has been committed upstream.

http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git;a=commit;h=3f46fa71cd6afb238e8fbe39cf23faacd2ddd1e3
Comment 12 RHEL Product and Program Management 2009-01-27 15:38:08 EST
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
release.
Comment 13 RHEL Product and Program Management 2009-02-16 10:44:43 EST
Updating PM score.
Comment 14 Don Zickus 2009-02-23 15:02:55 EST
in kernel-2.6.18-132.el5
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.
Comment 18 errata-xmlrpc 2009-09-02 04:59:21 EDT
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.

http://rhn.redhat.com/errata/RHSA-2009-1243.html

Note You need to log in before you can comment on or make changes to this bug.