String Encryption

Question

I was asked to make a String encryption utility without using encryption API I basically had to come up with my own ecryption methodology! the program was to be able to encrypt a string and decrypt a string and to of course make it nearly impossible for anyone to figure out what the encrypted string says!

I would really love to get some advice and suggestions regarding my code! I know it is not fast and could indeed be faster but that wasnt the main focus in this case!

import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.Map;
import java.util.Random;


/**
 * This is a class which can be used to encrypt any sort of string,
 * the class provides a simple encryption program which uses a scrambler/adder/replacer/swapper.
 * The encrypted message is sent as an object which can then be casted
 * into a String LinkedList or a String array which can then be formated and decrypted.
 * If the message is infiltrated while traveling through a socket the thief
 * wont be able to see the content of the message without the key or this class.
 *
 * @author Eudy Contreras
 *
 */

public class Encrypter {
    private static final char[] plain =
            {'A','B','C','D','E','F','G','H',
             'I','J','K','L','M','N','O','P',
             'Q','R','S','T','U','V','W','X',
             'Y','Z','Ö','Ä','Å','0','1','2',
             '3','4','5','6','7','8','9'};
    private static final char[] key   =
            {'D','E','F','G','H','Ä','U','J',
             'A','L','Z','Å','N','C','P','Q',
             'W','S','T','Ö','I','V','R','X',
             'Y','M','K','B','O','4','6','1',
             '8','3','2','9','0','5','7'};
    private static final char[] added  =
            {'L','E','Z','G','H','Ä','U','J',
             'A','D','F','Å','N','C','M','Y',
             'W','S','B','Ö','I','V','R','1',
             'Q','P','K','T','O','4','6','X',
             '8','3','2','9','0','5','7'};

    private static final Map<Character, Character> plainMap;
    static
    {
        plainMap = new HashMap<Character, Character>();
        for(int i = 0; i<plain.length; i++){
            plainMap.put(plain[i],key[i]);
        }
    }
    private static final Map<Character, Character> keyMap;
    static
    {
        keyMap = new HashMap<Character, Character>();
        for(int i = 0; i<key.length; i++){
            keyMap.put(key[i],plain[i]);
        }
    }
    private static final int operationCount = 16;

    /**
     * This method will will used a simple key
     * and encrypt a message
     * @param message
     * @return
     */
    private static String obfuscate(String message) {
        String caseCode = convertCase(message);
        char[] caseBinary = new char[message.length()];
        for(int i = 0; i<caseCode.length(); i++) {
            if(caseCode.charAt(i) == 'A') {
                caseBinary[i] = '1';
            }
            else if(caseCode.charAt(i) == 'a') {
                caseBinary[i] = '0';
            }
        }
        char[] upperCaseMessage = new char[message.length()];
        for(int i = 0; i<message.length(); i++) {
            upperCaseMessage[i] = Character.toUpperCase(message.charAt(i));
        }
        char[] code = upperCaseMessage;
            for (int i = 0; i < code.length; i++) {
                if(plainMap.containsKey(code[i])){
                    code[i] = plainMap.get(code[i]);
                }
                else {code[i] = code[i];}
            }
        return String.valueOf(code)+"¤¤¤¤"+String.valueOf(caseBinary);
    }

    /**
     * this method will decypher and encrypted string message
     * @param message
     * @return
     */
    private static String DecypherEncryption(String message) {
        String[] code_and_case = message.split("¤¤¤¤");
        String codeMessage;
        String caseMessage;
        if(code_and_case.length == 2) {
            codeMessage = code_and_case[0];
            caseMessage = code_and_case[1];
        }
        else {
            codeMessage = message;
            caseMessage = null;
        }

        char[] code = codeMessage.toCharArray();
            for (int i = 0; i < code.length; i++) {
                if(keyMap.containsKey(code[i])){
                    code[i] = keyMap.get(code[i]);
                }
                else {code[i] = code[i];}
            }
        if (code_and_case.length == 2) {
            for (int i = 0; i < caseMessage.length()-1; i++) {
                if (caseMessage.charAt(i) == '1') {
                    code[i] = Character.toUpperCase(code[i]);
                } else if (caseMessage.charAt(i) == '0') {
                    code[i] = Character.toLowerCase(code[i]);
                }
            }
        }
        return String.valueOf(code);
    }
    /**
     * Further increases the encryption level of the message by adding random numbers and characters
     * to the already encrypted message the process can then be reversed with an additional key.
     * @param code: the encrypted message
     * @return
     */
    private static LinkedList<String> scrambler(String code) {
        Random rand1 = new Random();
        Random rand2 = new Random();
        Random rand3 = new Random();
        Random rand4 = new Random();
        String encryptedCode = Encrypter.obfuscate(code);
        String[] case_and_code = encryptedCode.split("¤¤¤¤");
        String cypher = case_and_code[0];
        String codeCase = case_and_code[1];
        LinkedList<String> cypherList = new LinkedList<>();
        char[] cypherCharArr = cypher.toCharArray();
        for (int index = 0; index < cypherCharArr.length; index++){
            cypherList.add(added[rand3.nextInt(added.length)]+""+added[rand2.nextInt(added.length)]+String.valueOf(cypherCharArr[index]) + added[rand4.nextInt(added.length)]+(rand1.nextInt(99 - 10+1)+10));
        }
        cypherList.addFirst(""+(rand1.nextInt(999 - 100 +1 )+100));
        cypherList.add(codeCase+rand1.nextInt(10));
        return swapper(cypherList);

    }
    /**
     * This method will unscramble and rebuild a scrambled string
     * @param cypherList
     * @return
     */
    private static String unScrambler(LinkedList<String> cypherList) {
        StringBuilder stringBuilder = new StringBuilder();
        LinkedList<String> cypherListOutput = new LinkedList<>();
        String caseCode = cypherList.get(cypherList.size()-1);
    for (int index = 0; index < cypherList.size()-1; index++) {
        cypherListOutput.add(String.valueOf(cypherList.get(index).toCharArray()[2]));
        if(index>=1)
        stringBuilder.append(cypherListOutput.get(index));
        continue;
    }
    char[] rawCode = (stringBuilder.toString()+"¤¤¤¤"+caseCode).toCharArray();
    char[] unscrambled = new char[rawCode.length-1];
    for(int i = 0; i<unscrambled.length; i++) {
        unscrambled[i] = rawCode[i];
    }
    return String.valueOf(unscrambled);
    }
    /**
     * Swaps the content of the array the as many times as
     * the operation count.
     * @param code
     * @return
     */
    private static LinkedList<String> swapper(LinkedList<String> code) {
        LinkedList<String> output = new LinkedList<>();
        LinkedList<char[]> chars = new LinkedList<>();
        String[] swapArray = new String[code.size()];
        for (int index = 0; index < code.size(); index++) {
            swapArray[index] = code.get(index);
            chars.add(swapArray[index].toCharArray());
        }
        for (int iteration = 0; iteration < operationCount; iteration++) {
            for (int i = 1; i < chars.size() - 2; i++) {
                for (int r = 0; r < chars.get(i).length - 1; r++) {
                    char tempChar = chars.get(i)[r];
                    chars.get(i)[r] = chars.get(i + 1)[r];
                    chars.get(i + 1)[r] = tempChar;
                }
            }
        }
        for (int i = 0; i < chars.size(); i++) {
            output.add(String.valueOf(chars.get(i)));
        }
        return output;
    }
    /**
     * unswapps the content of the array in order to make it
     * ready to be deciphered
     * @param code
     * @return
     */
    private static LinkedList<String> unswapper(LinkedList<String> code) {
        LinkedList<String> output = new LinkedList<>();
        LinkedList<char[]> chars = new LinkedList<>();
        String[] swapArray = new String[code.size()];

        for (int index = 0; index < code.size(); index++) {
            swapArray[index] = code.get(index);
            chars.add(swapArray[index].toCharArray());
        }
        for (int iteration = 0; iteration < operationCount; iteration++) {
            for (int i = chars.size() - 3; i >= 1; i--) {
                for (int r = chars.get(i).length - 2; r >= 0; r--) {
                    char tempChar = chars.get(i)[r];
                    chars.get(i)[r] = chars.get(i + 1)[r];
                    chars.get(i + 1)[r] = tempChar;
                }
            }
        }
        for (int i = 0; i < chars.size(); i++) {
            output.add(String.valueOf(chars.get(i)));
        }

        return output;
    }
    /**
     * This method will convert a string into case holder
     * which hold which character is upper case and which
     * isn't.
     * @param message
     * @return
     */
    private static String convertCase(String message) {
        char[] caseHolder = new char[message.length()];
        for(int i = 0; i < caseHolder.length; i++) {
            if (Character.isUpperCase(message.charAt(i))) {
                caseHolder[i] = 'A';
            } else if (Character.isLowerCase(message.charAt(i))) {
                caseHolder[i] = 'a';
            }
        }
        return String.valueOf(caseHolder);
    }

    /**
     * This method will take a case code holder
     * and apply the case to the given message
     * @param caseBank
     * @param message
     * @return
     */
    private static String caseConverter(String caseBank,String message){
        StringBuilder input = new StringBuilder(caseBank);
        StringBuilder output = new StringBuilder(message);
        for (int index = 0; index < input.length(); index++) {
            char current = input.charAt(index);
            char formatedString = output.charAt(index);
            if (Character.isLowerCase(current)) {
                output.setCharAt(index, Character.toLowerCase(formatedString));
            } else {
                output.setCharAt(index, Character.toUpperCase(formatedString));
            }
        }
        return output.toString();
    }

    private static String printBytes(byte[] binaryEncription){
        return new String(binaryEncription, Charset.forName("UTF-8"));
    }

    private static LinkedList<String> fromStringToList(String list){
        String[] newList = list.split(", ");
        LinkedList<String> code = new LinkedList<String>();
        for(int i = 0; i<newList.length; i++){
            code.add(newList[i]);
        }
        return code;
    }
    /**
     * Can be used to further increase the level of
     * encryption;
     * @param encryption
     * @return
     */
    private static byte[] byteSwapper(byte[] encryption){
        // TODO: write code here!!!
        // Further swapping may not be needed!
        return encryption;
    }
    /**
     * To unswapp the byte array back to normal
     * @param encryption
     * @return
     */
    private static byte[] byteUnswapper(byte[] encryption){
        //TODO: write code here!
        return encryption;
    }
    public static byte[] encrypt(String message){
        String cypher = scrambler(message).toString();
        byte[] encryption = cypher.getBytes(Charset.forName("UTF-8"));
        byte[] swappedBytes = byteSwapper(encryption);
        return swappedBytes;
    }
    public static String decrypt(byte[] encryption){
        byte[] unswappedBytes = byteUnswapper(encryption);
        LinkedList<String> list  = fromStringToList(new String(unswappedBytes, Charset.forName("UTF-8")));
        LinkedList<String> unSwapped = Encrypter.unswapper(list);
        String unScrambled = Encrypter.unScrambler(unSwapped);
        String decyphered =  Encrypter.DecypherEncryption(unScrambled);
        return decyphered;
    }
    public static void main(String[] args) {
        System.out.println(Encrypter.caseConverter("AaaaAA", "eudycr"));
        System.out.println(Encrypter.convertCase("Whats up man, how are you doing today?"));
        System.out.println(Encrypter.obfuscate("Whats up man, how are you doing today?"));
        System.out.println(Encrypter.scrambler("Whats up man, how are you doing today?"));
        System.out.println(Encrypter.unswapper(Encrypter.scrambler("Whats up man, how are you doing today?")));
        System.out.println(Encrypter.unScrambler(Encrypter.unswapper(Encrypter.scrambler("Whats up man, how are you doing today?"))));
        System.out.println("");
        System.out.println(Encrypter.DecypherEncryption(Encrypter.unScrambler(Encrypter.unswapper(Encrypter.scrambler("Whats up man, how are you doing today?")))));
        System.out.println(Encrypter.DecypherEncryption("HIGYFS"));
        System.out.println("");
        System.out.println("");
        System.out.println("");
        byte[] encryption = Encrypter.encrypt("Whats up man, how are you doing today?");
        System.out.println(printBytes(encryption));
        System.out.println(Arrays.toString(encryption));
        System.out.println(Encrypter.decrypt(encryption));
    }
}

Answer 1

Disclaimer: I don't use Java a lot and I'm not a cryptographer.

What you've got here is essentially a substitution cipher, with some additional obfuscation steps (inserting random characters and swapping/cycling the results).

Code formatting

• Not all code is properly indented. This makes it harder to read and understand.
• Some code blocks are on the same line as their preceding else statement, again making the code harder to read and understand.
• Use verbs for method names: scramble instead of scrambler, swap instead of swapper, and so on.
• Inconsistent naming: all method names start with a lower-case letter, except for DecypherEncryption. Most variable names use camelCase, but a few use underscores instead.
• Use (more) descriptive names. For example, consider names like getCasePattern and applyCasePattern instead of convertCase and caseConverter (those names in particular are so similar that it looks like they do the same thing), or applySubstitution instead of obfuscate.
• The code could use a little more 'breathing space' - separating methods (or 'blocks' of code within methods) with an empty line can improve readability.
• Don't forget to document the public methods of your class - those are the methods that other programmers will look at. Be sure to document any peculiarities (such as only letters and digits being encrypted, and so on).

Implementation

• obfuscate should also accept a substitution key as argument (see below).
• swapper essentially cycles items. Repeated swapping results in items being shifted to the left, and the first item becoming last (except for the last character of each item, but that's a meaningless random digit anyway). Note that for inputs of length 1, 2, 4, 8 or 16, the output will be the same as the input.
• Your code operates on strings, but it only substitutes a small range of characters. Try operating on byte arrays instead, so any piece of text (or image, or other kind of data) can be fully encrypted.
• There's a lot of string assembling and splitting going on. Consider using tuples, or a custom class, instead.
• Strings and char-arrays are frequently converted to each other. Consider only using char arrays internally (or better, byte arrays).
• caseConverter doesn't check if the message and case-string have equal length. It's a private method, but still.
• scrambler returns a LinkedList, and calling toString on that produces a string like "[abc, def, ghi]". Using that as end-result exposes a lot of information to attackers.
• unScrambler doesn't strip those [ and ] characters, which could cause problems at a later stage.

Security

• The substitution key is hard-coded. Any system that uses this code becomes insecure when an attacker gets access to your code.
• The obfuscation steps (adding random characters, swapping) are 'security through obscurity': they provide no additional security once an attacker has figured out your algorithm.
• Substitution ciphers are vulnerable to frequency analysis. For example, characters that occur frequently in an encrypted message are more likely to represent an 'e' or 's' than an 'x' (depending on language).
• Uppercasing text reduces the length of the substitution key, which makes it easier to break.
• Not substituting whitespace, punctuation characters and others causes patterns in the encrypted message that can help an attacker to figure out your obfuscation steps.
• The encrypted message is a comma-separated list of strings. This gives attackers possible information about how your algorithm works.
• The 'case string' at the end also provides information: the spaces in it can easily be interpreted as word boundaries, and they can also be matched with unsubstituted characters. The length of the case string also matches the number of preceding items.

Relying on an algorithm to remain secret severely restricts its use. You can't release applications that use it, because those applications can be reverse-engineered. You can use it on your servers, but an attacker only has to gain access to one of them, once, to completely defeat your encryption. And how do you know that you've got strong encryption if you can't even publish it for a code review? ;)

Relying on a key to remain secret is better: even when a key gets compromised, only that system becomes insecure. Other systems (using other keys) may still be secure.

Also keep in mind that even though a single encrypted message may look secure, having access to multiple ciphertexts may reveal patterns or other kinds of information that can help an attacker. If an attacker is able to control (parts of) the input they'll be able to get even more information (your random character insertion step would be quite vulnerable to this).

You may find the challenges at http://cryptopals.com/ interesting if you want to get a better understanding of cryptography. Their authors certainly know a lot more about it than I do.

Answer 2

Random is not cryptographically secure

        Random rand1 = new Random();
        Random rand2 = new Random();
        Random rand3 = new Random();
        Random rand4 = new Random();

Eek. See the documentation for java.util.Random which says:

Instances of java.util.Random are not cryptographically secure. Consider instead using SecureRandom to get a cryptographically secure pseudo-random number generator for use by security-sensitive applications.

More Random is not more random

Another issue is that you are creating four separate random number generators here. Why? Assuming each gets a different seed (apparently true in Java), you are essentially creating four opportunities for the sequence to repeat. Once the sequence starts repeating, it no longer looks random but planned. As a result, four generators from the same sequence are less secure than just using one generator.

It could be worse. Some implementations use the time as a random seed. In that case, you'd likely have only one or two distinct generators of the four.

It's more common to create one static random number generator and allow it to run for the life of the program. That way there is only one entry point to the sequence, minimizing the chance of overlapping subsequences.

Why loop twice to make one array?

        String caseCode = convertCase(message);
        char[] caseBinary = new char[message.length()];
        for(int i = 0; i<caseCode.length(); i++) {
            if(caseCode.charAt(i) == 'A') {
                caseBinary[i] = '1';
            }
            else if(caseCode.charAt(i) == 'a') {
                caseBinary[i] = '0';
            }
        }

Why does convertCase exist? Why not just change this code to say

        char[] caseBinary = new char[message.length()];
        for(int i = 0; i < message.length(); i++) {
            if (Character.isUpperCase(message.charAt(i))) {
                caseBinary[i] = '1';
            } else if (Character.isLowerCase(message.charAt(i))) {
                caseBinary[i] = '0';
            }
        }

Then you don't need caseCode or convertCase at all.

No-ops

                else {code[i] = code[i];}

This doesn't seem to do anything.

And earlier

        char[] code = upperCaseMessage;

Why have both code and upperCaseMessage?

It's not uncommon to obfuscate cryptographic code, but I'm not sure how helpful it is to get a review of the obfuscated code. Many of the responses are going to be like this. Trim the code down to the essentials and it will be easier to review.

Don't reinvent the wheel

    private static LinkedList<String> fromStringToList(String list){
        String[] newList = list.split(", ");
        LinkedList<String> code = new LinkedList<String>();
        for(int i = 0; i<newList.length; i++){
            code.add(newList[i]);
        }
        return code;
    }

You could simplify this to

    private static LinkedList<String> fromStringToList(String list) {
        return new LinkedList<String>(Arrays.asList(list.split(", ")));
    }

Or even easier

    private static List<String> fromStringToList(String list) {
        return Arrays.asList(list.split(", "));
    }

You don't actually use any of the benefits of a LinkedList, e.g. easy insertion and removal in the middle of the list. Why not use an array-backed data structure if you're going to directly manipulate the contents?