Keeping sensitive data encrypted in memory

Question

I'm working in a Electronic Funds Transfer (EFT) system and need be very careful with sensitive data like credit card numbers.

We need this information do mount the ISO 8583 data before sending to EFT Gateways.

We use char[] and byte[] to keep sensitive data and use Array.fill when We don't need this information anymore.

Thinking in improve security a thought in create a class to wrap this sensitive information and maybe keep data encrypted until needed.

Here's the code I plan to use with a random generated key.

import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.charset.Charset;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Arrays;

import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.KeyGenerator;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.SecretKey;
import javax.security.auth.Destroyable;

public final class SensitiveChars implements CharSequence, Destroyable {

    private static final int KEYSIZE = 56;
    private static final String DES = "DES";
    private static final String DES_ECB_PKCS5_PADDING = DES + "/ECB/PKCS5Padding";

    private byte[] data;
    private Charset charset;
    private int lenght;

    private SecretKey secretKey;

    public SensitiveChars(char[] data) {
        super();
        this.charset = Charset.defaultCharset();
        ByteBuffer byteBuffer = charset.encode(CharBuffer.wrap(data));
        lenght = data.length;

        try {
            KeyGenerator keyGenerator = KeyGenerator.getInstance(DES);

            SecureRandom secureRandom = new SecureRandom();
            int keyBitSize = KEYSIZE;
            keyGenerator.init(keyBitSize, secureRandom);
            secretKey = keyGenerator.generateKey(); 

            setRawData(byteBuffer.array());
        } catch (NoSuchAlgorithmException e) {
            throw new IllegalStateException(e);
        }
    }

    public static SensitiveChars of(byte[] bytes) {
        return of(bytes, Charset.defaultCharset());
    }   

    public static SensitiveChars of(byte[] bytes, Charset charset) {
        return of(bytes, charset, false);
    }

    public static SensitiveChars of(byte[] bytes, Charset charset, boolean cleanBytes) {
        char[] chars = toChar(bytes, charset, cleanBytes);
        return new SensitiveChars(chars);
    }

    @Override
    public int length() {
        return lenght;
    }

    @Override
    public char charAt(int index) {
        char[] tempArray = getChars();
        char c = tempArray[index];
        clearData(tempArray);
        return c;
    }

    @Override
    public CharSequence subSequence(int start, int end) {
        char[] dataTmp = getChars();
        char[] range = Arrays.copyOfRange(dataTmp, start, end);
        clearData(dataTmp);
        return new SensitiveChars(range);
    }

    public char[] getChars() {
        if (lenght == 0) {
            return new char[0];
        } else {
            byte[] rawData = getRawData();
            ByteBuffer byteBuffer = ByteBuffer.wrap(rawData);
            CharBuffer charBuffer = charset.decode(byteBuffer);
            char[] result = Arrays.copyOf(charBuffer.array(), lenght);
            clearData(charBuffer.array());
            clearData(rawData);
            return result;
        }
    }

    public void append(char[] chars) {
        char[] dataTmp = getChars();
        int lengthData = dataTmp.length;
        char[] newData = Arrays.copyOf(dataTmp, lengthData + chars.length);
        clearData(data);

        int posIni = lengthData;
        for (int i = 0; i < chars.length; i++) {
            newData[i + posIni] = chars[i];
        }

        CharBuffer charBuffer = CharBuffer.wrap(newData);
        ByteBuffer byteBuffer = charset.encode(charBuffer);
        byteBuffer.compact();
        setRawData(byteBuffer.array());
        lenght = newData.length;

        clearData(dataTmp);
        clearData(newData);
    }

    private void setRawData(byte[] data) {
        this.data = encrypt(data);
    }

    private byte[] getRawData() {
        return decrypt(data);
    }

    @Override
    public void destroy() {
        clearData(data);
        data = new byte[0];
        lenght = 0;
    }

    private byte[] encrypt(byte[] bytes) {
        try {
            Cipher cipher = Cipher.getInstance(DES_ECB_PKCS5_PADDING);
            cipher.init(Cipher.ENCRYPT_MODE, secretKey);
            return cipher.doFinal(bytes);
        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException
                | BadPaddingException e) {
            throw new IllegalStateException(e);
        }
    }

    private byte[] decrypt(byte[] bytes) {
        try {
            Cipher cipher = Cipher.getInstance(DES_ECB_PKCS5_PADDING);
            cipher.init(Cipher.DECRYPT_MODE, secretKey);
            return cipher.doFinal(bytes);
        } catch (InvalidKeyException | NoSuchAlgorithmException | NoSuchPaddingException | IllegalBlockSizeException
                | BadPaddingException e) {
            throw new IllegalStateException(e);
        }
    }   

    private static void clearData(byte[] cs) {
        Arrays.fill(cs, (byte) 0); 
    }   

    private static void clearData(char[] cs) {
        CharsUtils.clearData(cs);
    }       

}

It's a good idea or I must use another approach?

Edit 1: Our application need be comply with https://www.pcisecuritystandards.org/

Answer 1

When you are talking about security, it's important to clearly define what you are worried about. This is typically called a 'threat model'. I gather that you are concerned with someone capturing the memory of your application and retrieving the card numbers from it.

If that is correct, I see a fundamental issue in that your key is also being stored in that same memory space as well as the code that decrypts it. You are only making it slightly harder to get the data. I would judge this equal to a basic obfuscation technique.

On a side note, DES is an obsolete cipher. I would suggest that if you are interested in this topic, you may want to do some more research. It's also important to keep up-to-date on such developments in this space. Old articles may suggest using approaches that are not considered secure even when they were authoritative at the time.

Answer 2

OK, what's your threat model?

• The data gets to the pagefile when the memory is overloaded, and some other process peeks at the pagefile. Your solution improves your odds to not get a decrypted page into the pagefile, but does not eliminate such a chance, since you still have to decrypt the data somewhere in RAM. The right solution would be no paging to disk at all, or an encrypted pagefile / swap partition.

• A rouge process captures control of the system enough to directly observe the RAM of other processes, e.g. by gaining admin privileges. This is sort of game over for every process on this box; the rouge process now can detect where you store the decryption key, and decrypt the protected info, in RAM or in the DB. You solution becomes irrelevant.

• You run not entirely trusted code inside your server, e.g. some sort of third-party plugin, and you want to prevent it from peeking information it should not have access to. See above.

The correct solution would be based on clear privilege separation, giving the code the least privilege it needs, and keeping the amount of trusted code as small as possible.

For instance, I'd keep the sensitive data in the main application encrypted at all times, likely even never fetched into RAM. It has nothing to do with it, except maybe checking its presence. This process should not have the access to the decryption key at all. The machine(s) that run the database should not have access to the decryption key either.

I'd run the process of export as a separate process, or maybe a separate VM, or even a separate physical box. That process would be a very small piece of code that takes the IDs of records to export, fetches them from the DB (where they are stored encrypted), decrypts them, and sends to the receiving end, presumably using a differently encrypted channel (e.g. via a TLS connection, or writing an encrypted file).

The exporter should alone be able to access the decryption key, presumably stored elsewhere, and have barely enough privileges to read (not write) the relevant part of the database, and to output to the export channel. Likely you already have some infra for storing, accessing, and rotating secret keys. The exporter should run for as short time as possible, and be shut down when not being used. Certainly it should not be accessible from the Internet, and minimally accessible from your internal network, including the machines running the other parts of the application.

This would reduce the remote attack surface, and add a layer or two for an attacker to pierce if e.g. your main app server, or a database server, is compromised.