Red Hat Bugzilla – Full Text Bug Listing
|Summary:||CVE-2012-2146 python-elixir: weak use of crypto can leak information|
|Product:||[Other] Security Response||Reporter:||Vincent Danen <vdanen>|
|Component:||vulnerability||Assignee:||Red Hat Product Security <security-response-team>|
|Status:||ASSIGNED ---||QA Contact:|
|Version:||unspecified||CC:||a.badger, dcallagh, fw, fweimer, limburgher, mcepl, mitr, security-response-team, sochotni, tmraz|
|Fixed In Version:||Doc Type:||Bug Fix|
|Doc Text:||Story Points:||---|
|oVirt Team:||---||RHEL 7.3 requirements from Atomic Host:|
|Bug Depends On:||923220, 923221|
Description Vincent Danen 2012-04-04 18:10:58 EDT
It was reported that python-elixir, a library for ORM mapping on top of SQLAlchemy with support for encrypting data stored in a database, suffers from weak use of cryptography. It uses Blowfish in CFB mode, which has an additional parameter (IV), which is not specified and thus defaults to zero. CFB mode is only secure if the the IV is unpredictable and different for every message. Because of this, and because the encryption key is shared for each database table (fields and rows), the same plaintext prefix is always encrypted to an identical and corresponding ciphertext prefix. As a result, an attacker with access to the database could figure out the plaintext values of encrypted text.
Comment 20 Stanislav Ochotnicky 2012-04-17 05:25:13 EDT
Created attachment 577963 [details] migration script tests I have created a simple set of scripts to test if data migration would be feasible. It seems that with modifications, data could be migrated like this: 1. prepare migration script, backup database etc 2. stop all database work 3. run migration script 4. install errata for this bug 5. continue working as before I am using monkey-patching to "fix" old python-elixir encryption code without actually touching the source files so we can sort of use fixed code without installing the errata. Migration script would have to use the encryption method expected. In this case I've used the first variant, i.e. keep using python-crypto. We could make the migration script a bit more generic where administrators doing the migration would just have to put in table names that contain encrypted data. I was also thinking about a way to prevent accidental running of the script twice, which would cause the data to be encrypted twice and didn't come up with anything short of creating additional table in the database that would contain already converted table names. Not a hard thing to do actually, but not sure it's worth doing.
Comment 21 Tomas Mraz 2012-04-17 05:39:55 EDT
If it is necessary to migrate, it should be definitely migrated to AES-128 cipher and m2crypto.
Comment 24 Stanislav Ochotnicky 2012-04-18 11:11:53 EDT
Created attachment 578385 [details] Proposed patch to fix the bug After talking to mitr, I've contacted upstream. I have a patch that adds additional encryption methods (turned off by default so that we don't break stuff when users install errata without migrating first). It uses AES/python-crypto to encrypt so we don't have to do major rewrites. Attached patch is a complete proposed change to python-elixir package. I've also updated example migration scripts to reflect this change  http://groups.google.com/group/sqlelixir/browse_thread/thread/efc16227514cffa
Comment 28 Vincent Danen 2012-04-27 16:34:34 EDT
This has also been reported upstream, although currently there are no comments: http://elixir.ematia.de/trac/ticket/119 (and no response to the message that Stanislav notes in comment #24
Comment 29 Vincent Danen 2012-04-27 16:57:40 EDT
CVE and peer review requested: http://www.openwall.com/lists/oss-security/2012/04/27/8
Comment 31 Kurt Seifried 2012-04-29 02:49:52 EDT
Added CVE as per http://www.openwall.com/lists/oss-security/2012/04/29/1
Comment 32 Vincent Danen 2012-04-30 17:55:58 EDT
Florian Weimer noted the following observations: > CFB mode is only secure if the the IV is unpredictable and different > for every message. There are a few additional requirements. Without some form of message authentication, chosen-ciphertext attacks are still possible even with a random IV. > Because of this, and because the encryption key is shared for each > database table (fields and rows), the same plaintext prefix is > always encrypted to an identical and corresponding ciphertext > prefix. As a result, an attacker with access to the database could > figure out the plaintext values of encrypted text. And you can group by encrypted column values in the database. That's why I'm not sure if it's actually possible to address this issue in a satisfying manner. Thoughts? I imagine, if he's accurate (no idea, not familiar with the software but I don't see why he wouldn't be spot-on here), then we may need to revist how we're handling this.
Comment 33 Tomas Mraz 2012-05-02 02:38:46 EDT
(In reply to comment #32) > Florian Weimer noted the following observations: > > And you can group by encrypted column values in the database. That's > why I'm not sure if it's actually possible to address this issue in a > satisfying manner. I do not understand how would you group by encrypted column if the IV is random - or what he really means by this sentence. Also I am not really sure if the chosen-ciphertext attack applies - it would require that the attacker has full write access to the database columns that are later decrypted and plaintext shown to the attacker - I am not sure it is really purpose of python-elixir encryption to protect against such attack although using OFB or CTR mode would be better in this regard.
Comment 34 Stanislav Ochotnicky 2012-05-02 04:25:32 EDT
(In reply to comment #32) > Florian Weimer noted the following observations: > > > CFB mode is only secure if the the IV is unpredictable and different > > for every message. The fix uses CBC mode not CFB > There are a few additional requirements. Without some form of message > authentication, chosen-ciphertext attacks are still possible even with > a random IV. python-elixir is a thin ORM mapping layer on top of SQLAlchemy. Protection against chosen-ciphertext would be out of scope for this library. > > Because of this, and because the encryption key is shared for each > > database table (fields and rows), the same plaintext prefix is > > always encrypted to an identical and corresponding ciphertext > > prefix. As a result, an attacker with access to the database could > > figure out the plaintext values of encrypted text. Not true. We produce random IV for each row in the database. Even if you have 2 same plaintext fields they will be completely different in ciphertext. Yes, users will have to migrate their data to be able to use new cipher algorithms. There is no other way to properly protect them unfortunately. > > And you can group by encrypted column values in the database. That's > why I'm not sure if it's actually possible to address this issue in a > satisfying manner. It's not without database migration. > Thoughts? I imagine, if he's accurate (no idea, not familiar with the > software but I don't see why he wouldn't be spot-on here), then we may > need to revist how we're handling this. Our approach still seems OK to me. Did I miss anything?
Comment 35 Tomas Mraz 2012-05-02 04:39:30 EDT
(In reply to comment #34) > The fix uses CBC mode not CFB CBC has the same properties in regards to random IV and chosen ciphertext attacks as CFB. But as the IV is random in the fix and chosen ciphertext attack is not considered in the model I think we are OK. > Our approach still seems OK to me. Did I miss anything? Nope, it seems OK to me as well.
Comment 36 Vincent Danen 2012-05-02 13:29:41 EDT
Thanks for the clarification on the given questions. Very much appreciated.
Comment 37 Florian Weimer 2012-05-04 08:40:07 EDT
(In reply to comment #33) > I do not understand how would you group by encrypted column if the IV is random > - or what he really means by this sentence. You could do this before, when the encryption was convergent. I'm concerned that the fix (while appropriate from a crypto point of view, even though an AEAD would be better) breaks application functionality. After all, it does break the encryption test case.
Comment 38 Miloslav Trmač 2012-05-04 09:18:35 EDT
(In reply to comment #37) > I'm concerned > that the fix (while appropriate from a crypto point of view, even though an > AEAD would be better) breaks application functionality. After all, it does > break the encryption test case. I was unsure what this refers to, so, to save others some time: the distributed tarball contains a "test" directory, but most tests are only in svn. Among them is a test to the effect of: > class Person(Entity): > name = Field(String(50)) > password = Field(String(40)) ... > acts_as_encrypted(for_fields=['password', 'ssn'], with_secret='secret') and (shortened) > def test_two_consecutive_updates(self): > jonathan = Person( > name='Jonathan LaCour', > password='s3cr3tw0RD', > ) ... > p = Person.get_by(name='Jonathan LaCour') > assert p.password == 's3cr3tw0RD' > session.flush() > assert p.password == 'r\\x9d\\xa8\\xb4\\x8d|\\xffp\\xf5\\x0e' > session.flush() > assert p.password == 'r\\x9d\\xa8\\xb4\\x8d|\\xffp\\xf5\\x0e' testing that the ciphertext will have a specific, constant, value. IMHO: * Sorting on the encrypted values while the encryption is incorrect can't be helped; migrating the database will stop that attack from working. * The patch does not change the encryption mode by default, so nothing is broken by a mere package update. OTOH, if we want to fix the crypto, we simply have to break this test, and applications that would be broken by the change need to be modified at the time with_aes is set to True.
Comment 39 Miloslav Trmač 2012-05-04 09:20:49 EDT
Created attachment 582122 [details] A possible test case patch This is how the test case could be updated for with_aes=True.
Comment 40 Florian Weimer 2012-05-15 11:02:26 EDT
(In reply to comment #38) > I was unsure what this refers to, so, to save others some time: the distributed > tarball contains a "test" directory, but most tests are only in svn. Oops. I wasn't aware of that. > IMHO: > * Sorting on the encrypted values while the encryption is incorrect can't be > helped; migrating the database will stop that attack from working. I'm not sure if it is incorrect if it works. But if you think this is really important to fix (even though upstream appears unlikely to pick up the patch), I think the proposed opt-in approach looks okay.
Comment 43 Stefan Cornelius 2013-03-19 09:08:19 EDT
Created python-elixir tracking bugs for this issue Affects: fedora-all [bug 923220]
Comment 44 Stefan Cornelius 2013-03-19 09:09:24 EDT
Created python-elixir tracking bugs for this issue Affects: epel-5 [bug 923221]
Comment 45 Stefan Cornelius 2013-03-19 09:10:22 EDT
Statement: The Red Hat Security Response Team has rated this issue as having moderate security impact. A future update may address this issue. For additional information, refer to the Issue Severity Classification: https://access.redhat.com/security/updates/classification/.
Comment 46 Toshio Ernie Kuratomi 2013-03-31 09:35:11 EDT
I'm thinking of orphaning elixir (I don't use it anymore) but wanted to see this wrapped up before doing so. sochotni, your patch looks good to me. elixir upstream seems to be dying. how do you feel about applying your patch locally? would you want me to do that our would you like to just take over ( you already have commit)?
Comment 47 Miloslav Trmač 2013-04-02 08:56:28 EDT
(In reply to comment #46) > I'm thinking of orphaning elixir (I don't use it anymore) but wanted > to see this wrapped up before doing so. > > sochotni, your patch looks good to me. elixir upstream seems to > be dying. how do you feel about applying your patch locally? > would you want me to do that our would you like to just take > over ( you already have commit)? Earlier I have looked for uses of python-elixir in Fedora, and found none. Would it make sense to not only orphan the package, but also remove it from the distribution (and modify TurboGears not to require it)? It's rather draconian (and unkind to out-of-Fedora uses of elixir through TurboGears), but perhaps still worth it - an unmaintained package doing crypto makes me nervous. I don't know. I'm not at all certain that dropping the package altogether is a good idea - I'm just noting this as a possible option.
Comment 48 Toshio Ernie Kuratomi 2013-04-02 09:48:32 EDT
I would be fine dropping the package in f19/ rawhide if nothing requires it. but we still need to fix this as we can't drop packages in released versions of fedora. there's no way to remove the package from the machines of users who may have installed it. I'm leery off making promises to retire this before looking at why turbogears needs it but I suspect it's just their preferred package for doing database migrations. if so, python-alembic can fill the need for new software.
Comment 49 Jon Ciesla 2014-08-19 10:43:09 EDT
An progress on this? I maintain moodle which needs this several layers down the chain in el5 where it's orphaned. Is it worth my picking unorphaning it?
Comment 50 Miloslav Trmač 2014-08-19 10:51:51 EDT
(In reply to Jon Ciesla from comment #49) > An progress on this? I maintain moodle which needs this several layers down > the chain in el5 where it's orphaned. Do the layers down the chain actually use acts_as_encrypted?
Comment 51 Dan Callaghan 2014-08-20 18:56:05 EDT
Stano, in comment 42 you mention having a patch, could you please attach it here? I would also be in favour of just patching TurboGears not to require python-elixir and letting the package be retired. I had a look through TurboGears, there is no hard runtime requirement on elixir (it's imported with except ImportError: pass). The quickstart tool can optionally include Elixir support in the generated project, so we could either patch that option out entirely, or just leave it up to the user to satisfy their application's dependency if they do pass --elixir. But really nobody should be starting a new project with TurboGears1 nowadays anyway. Jon, do you know if your application (is it Moodle, or some other transitive dep?) is actually *using* Elixir or is it just in the dependency chain because of TurboGears?
Comment 52 Dan Callaghan 2014-08-20 18:59:28 EDT
(In reply to Dan Callaghan from comment #51) > Stano, in comment 42 you mention having a patch, could you please attach it > here? Ah, I clicked through to the CVE and found the upstream issue Stano filed with his patch: http://elixir.ematia.de/trac/ticket/119 https://sochotni.fedorapeople.org/python-elixir-aes-encryption-addition.patch
Comment 53 Dan Callaghan 2014-08-20 19:38:38 EDT
Oops, I just realised the patch is already attached here. Sorry for the noise. So I took the EPEL7 branch of python-elixir a few weeks back, since I need it for the TurboGears1 stack which I am now maintaining in EPEL7 (I'm not actually using Elixir anywhere). The patch applies cleanly on Elixir 0.7.1, and I have backported it to Elixir 0.5.0 for EPEL5 as well. So I will take this package in EPEL5 and all Fedora branches and apply the patch, unless there are any objections.
Comment 54 Dan Callaghan 2014-08-24 23:21:31 EDT
(In reply to Dan Callaghan from comment #53) > So I will take this package in EPEL5 and all > Fedora branches and apply the patch Done.
Comment 55 Fedora Update System 2014-09-09 18:17:33 EDT
python-elixir-0.7.1-14.fc20 has been pushed to the Fedora 20 stable repository. If problems still persist, please make note of it in this bug report.
Comment 56 Fedora Update System 2014-09-09 18:21:10 EDT
python-elixir-0.7.1-14.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report.