zopefoundation / cipher.encryptingstorage

ZODB storage wrapper for encryption of database records
Other
5 stars 8 forks source link

Modus encrypti not documented #3

Closed enkore closed 7 months ago

enkore commented 7 years ago

Although this is delivered by keas.kmi I think it should be documented what's used:

Items 1.), 3.) and 4.) in combination are rather problematic, since they enable padding oracle attacks, possibly allowing recovery of the plaintext. It's a reasonable assumption that service-like applications would be vulnerable here, since this raises an exception and no attempt at reducing the timing delta is made in keas.kmi, so it should be easy to mount this attack.

Also the combination of first zlib'ing the contents and then encrypting them might be problematic, but I didn't looked into that. Just saying.

Actually in keas.kmi it would seem from a cursory glance that EncryptionService.encrypt() uses a hard-coded IV (self.initializationVector, which doesn't seem to be modified, ever[1]) which would of course be completely unsafe.

[1]

$ ag initializationVector
src/keas/kmi/facility.py
53:    initializationVector = '0123456789ABCDEF'
81:            IV=self.initializationVector)
155:            IV=self.initializationVector)

The call chain to that is

    def __init__(self, base, *args, **kw):
        self.base = base

        if (lambda encrypt=True: encrypt)(*args, **kw):
            self._encrypt = True
            self._transform = encrypt  # Refering to module func below!
        else:
            self._encrypt = False
            self._transform = lambda data: data

and encrypt:

def encrypt(data):
    try:
        if data[:2] == '.e':
            return data
    except TypeError:
        # a ZODB test passes None as data, be forgiving about that
        return data

    # 1. compress
    data = compress(data)

    # 2. encrypt here!!!
    data = encrypt_util.ENCRYPTION_UTILITY.encryptBytes(data)
    return '.e'+data

encryptBytes

    def encryptBytes(self, data):
        return self.facility.encrypt(self.key, data)

EncryptionService.encrypt

    def encrypt(self, key, data):
        """See interfaces.IEncryptionService"""
        # 1. Extract the encryption key
        encryptionKey = self._bytesToKey(self.getEncryptionKey(key))
        # 2. Create a cipher object
        cipher = self.CipherFactory.new(
            key=encryptionKey, mode=self.CipherMode,
            IV=self.initializationVector)
        # 3. Apply padding.
        data = self._pkcs7Encode(data)
        # 4. Encrypt the data and return it.
        return cipher.encrypt(data)

With self.initializationVector which doesn't seem to be touched ever. (Please please tell me it is and I just missed it).

getEncryptionKey() seems to be static for a given key, and the key used in encrypt_util seems also to be static.

mgedmin commented 7 years ago

I've never heard of keas.kmi being audited by a crypto specialist, so I wouldn't be surprised to find it's completely unsafe.

enkore commented 7 years ago

It's also not entirely clear what the encryption should protect against (defining an attack(er) model helps with this [technically this should be done before designing the crypto protocol]).

Since the implementation sits at the storage layer the storage service (ZEO or in-process) is trusted (they could always execute code on the clients anyway).

Since mixed-mode operation with unencrypted and encrypted objects is allowed it also cannot protect against an attacker that is able to modify the physical database (Data.fs, RelStorage tables), since they could inject code through unencrypted objects. That also implies that authentication wouldn't really provide much benefit.

I believe the major kind of attack this is able to protect against (ignoring the question marks raised above for a second) would be theft of the physical database in any form (read-only access or actually stealing the drives).

mgedmin commented 4 years ago

It's also not entirely clear what the encryption should protect against (defining an attack(er) model helps with this [technically this should be done before designing the crypto protocol]).

Both Keas and CipherHealth were (are?) US-based tech companies involved in health care. AFAIU the goal was HIPAA compliance, specifically, protecting personally identifying information. The encryption keys were supposed to be stored on a different server, so that anyone stealing the encrypted database would be unable to decrypt it offline.