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.
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.
If it is necessary to migrate, it should be definitely migrated to AES-128 cipher and m2crypto.
Created attachment 578385 [details] Proposed patch to fix the bug After talking to mitr, I've contacted upstream[1]. 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 [1] http://groups.google.com/group/sqlelixir/browse_thread/thread/efc16227514cffa
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
CVE and peer review requested: http://www.openwall.com/lists/oss-security/2012/04/27/8
Added CVE as per http://www.openwall.com/lists/oss-security/2012/04/29/1
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.
(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.
(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?
(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.
Thanks for the clarification on the given questions. Very much appreciated.
(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.
(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.
Created attachment 582122 [details] A possible test case patch This is how the test case could be updated for with_aes=True.
(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.
Created python-elixir tracking bugs for this issue Affects: fedora-all [bug 923220]
Created python-elixir tracking bugs for this issue Affects: epel-5 [bug 923221]
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)?
(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.
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.
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?
(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?
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?
(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
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.
(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.
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.
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.
This bug is now closed. Further updates for individual products will be reflected on the CVE page(s): https://access.redhat.com/security/cve/cve-2012-2146