1

I am interested in learning different encryption techniques and i am trying to understand one implementation in-particular. I have set the password in the application and now want to understand how i got to the result?

password result 5hfGisIwnlQ=

The application uses java.crypto.cipher and DES encryption.

I have found a string called key = ucnTRFY<jhvbu,fwy8\u00a3329+3yrwiefDY#FGVoD@IOHa?widNCD(ZH3*82YU9%Fjpo

The application seems to encrypt like this:

  private SecretKey getKey() {
    byte[] desKeyData = new byte[8];
    for (int i = 0; i < 7; ++i) {
        int j = i * 3;
        desKeyData[i] = (byte)(key.charAt(j) ^ key.charAt(j + 2));
    }
    return new SecretKeySpec(desKeyData, "DES");
}

public byte[] encrypt(String password) {
    if (password == null) {
        return null;
    }
    byte[] encryptedBytes = password.getBytes();
    if (!doEncryption) {
        return encryptedBytes;
    }
    SecretKey key = this.getKey();
    try {
        Cipher cipher = Cipher.getInstance("DES");
        cipher.init(1, key);
        byte[] bytes = null;
        try {
            bytes = password.getBytes("UTF-8");
        }
        catch (UnsupportedEncodingException ex) {
            // empty catch block
        }
        encryptedBytes = cipher.doFinal(bytes);
        this.encrypted = true;
    }

and decrypts like this

public String decrypt(byte[] encryptedBytes) {
    if (encryptedBytes == null) {
        return null;
    }
    String password = null;
    try {
        password = new String(encryptedBytes, "UTF-8");
    }
    catch (UnsupportedEncodingException ex) {
        // empty catch block
    }
    if (!doEncryption || !this.encrypted) {
        return password;
    }
    SecretKey key = this.getKey();
    try {
        Cipher cipher = Cipher.getInstance("DES");
        cipher.init(2, key);
        password = new String(cipher.doFinal(encryptedBytes));
    }
    catch (NoSuchAlgorithmException e) {
        logger.error((Object)e);
    }
    catch (InvalidKeyException e) {
        logger.error((Object)e);
    }
    catch (NoSuchPaddingException e) {
        logger.error((Object)e);
    }
    catch (IllegalBlockSizeException e) {
        logger.error((Object)e);
    }
    catch (BadPaddingException e) {
        logger.error((Object)e);
    }
    return password;
}

I am most interested in what this is doing? private SecretKey getKey() And that the arguments are in here cipher.init(2, key)?

Before this I implemented an application using Bcrypt. What are the advantages and disadvantages of each method?
FYI this is how i implemented Bcrypt. if (BCrypt.checkpw(userId.getText() + passwordfield.getText(), passwordhashadmin)

  • You didn't implement Bcrypt you used a Bcrypt library function, which is fine. Reading the code of a crypto algorithm (although not impossible) is not the best way of figuring how it works. On the other hand DES has been described in hundreds of places: it is a cypher that builds over the permutation of the cleartext against parts of the key. The wikipedia article is not particularly explanative but [William Stallings has an appendinx in his book about DES](http://mercury.webster.edu/aleshunas/COSC%205130/G-SDES.pdf). His book is well know on cryptography. – grochmal Jan 19 '17 at 01:17

2 Answers2

1

This code is very bad. If you're copying it from elsewhere, you need to stop doing so. If you're writing it yourself, well, you shouldn't use it in any system that's actually security critical—you should seek expert assistance if that's the goal. The only justifiable purpose to code like this is as an educational exercise.

Let's dive in to the problems.

Use of obsolete and insecure DES cipher

DES is a 1970s cipher with 56-bit keys. That is much too small a keyspace for security; brute force search of DES keys was already demonstrated over 20 years ago.

Hardcoded cryptographic key?

I have found a string called key = ucnTRFY<jhvbu,fwy8\u00a3329+3yrwiefDY#FGVoD@IOHa?widNCD(ZH3*82YU9%Fjpo

If this is a string constant in a real-world program, then that's really terrible. Programs should never hardcode cryptographic keys; they should read them at runtime from secure storage. The only scenario where it's acceptable to have a hardcoded key like that is in a trivial demo program.

Even if the string comes from a file instead of being hardcoded, the fact that you're able to read it already should make you wonder whether it was adequately protected. If it's a secret key, how come I'm reading it on the interwebs?

Obfuscated/convoluted key deserialization

DES takes its key as a byte[8]. The program computes that byte[8] from the key string in a really convoluted manner:

private SecretKey getKey() {
    byte[] desKeyData = new byte[8];
    for (int i = 0; i < 7; ++i) {
        int j = i * 3;
        desKeyData[i] = (byte)(key.charAt(j) ^ key.charAt(j + 2));
    }
    return new SecretKeySpec(desKeyData, "DES");
}

This doesn't do anything other than make the code needlessly complicated. A well-written real-world program would obtain the desKeyData by reading it directly from a binary file or by deserializing it from a string representation of a binary array (e.g. hexadecimal digits, Base64).

Passwords should not be encrypted

The code is encrypting a password. This is an insecure method of password storage. See the top answer to of our top passwords questions, "How to securely hash passwords?". That should answer your question about bcrypt as well.

Use of insecure cipher mode

This line of code:

Cipher cipher = Cipher.getInstance("DES");

...initializes the Cipher object in ECB ("electronic codebook") mode. This is almost without an exception a bad idea.

Unauthenticated encryption

As a general rule, you should only use ciphers that protect both the authenticity and confidentiality of the messages. Modern ciphers protect not only against an attacker reading the content of the messages, but also against forgeries.

ECB mode doesn't do that. And you'll find lots of example code using modes like CBC that don't do so either.

Use of numeric literal instead of mode constant

This line is needlessly hard to read:

cipher.init(1, key);

It should be this:

cipher.init(Cipher.ENCRYPT_MODE, key);

Bad exception handling

Lots of lines like these:

catch (UnsupportedEncodingException ex) {
   // empty catch block
}

Or these:

catch (BadPaddingException e) {
    logger.error((Object)e);
}
return password;

Security-critical code needs to be exceptionally clean. If anything fails you should generally assume the worst and cause the whole operation to fail, not swallow an exception and keep going like nothing wrong happened.

(Also, that upcast to Object in the second example is trivial and can be removed.)

Code discipline (method contracts, state avoidance, type safety)

All that logic with the doEncryption and encrypted boolean fields makes me nervous. It sounds like this code is inside a class whose objects can be in four states:

  • Contains encrypted data, does encryption/decryption when requested to
  • Contains unencrypted data, does encryption/decryption when requested to
  • Contains encrypted data, ignores encryption/decryption requests
  • Contains unecrypted data, ignores encryption/decryption requests

...and the class's methods and external clients then have to be mindful about its state so as not to expose an unencrypted password by mistake. The following bit of code in the encrypt() method is the most symptomatic example:

if (!doEncryption) {
    return encryptedBytes;
}

So if I'm using this class and I call encrypt(mySuperSecretData) method, it might actually not encrypt it at all? What is the contract of the class and the methods? Is that contract really confusing or allow for the class's clients to easily misuse it?

Why have such a complicated class with these many self-defeating options? Maybe instead you should have separate types PlaintextPassword and ScrambledPassword so that when you try to pass a PlaintextPassword to a piece of code that expects a ScrambledPassword, it become an error at compilation time and not a security failure. With careful design and discipline you can gain much greater confidence that the program will not expose sensitive data by mistake.

This is part of the same theme as the previous point: security-critical code needs to be exceptionally clean. If I have to pause and read carefully the if statements and a || b conditions across those two methods to figure out whether there's a risk that they were incorrectly written and will reveal secret information in some scenarios, then I have to ask whether all that logic can just be removed.

Conclusion

This is the wrong level from which to approach cryptography. If you're interested in practical uses of cryptography, you should seek out a good, high-level cryptographic library that offers an easy interface, with good, safe default choices made for you an really good documentation that you can count on to tell you exactly how to use it.

The default Java cryptography API isn't that. Java has lots of cryptography support, but it all tends to be terribly hard and dangerous to use. It requires a lot of knowledge to use it safely, and that knowledge cannot be acquired by tinkering around with random code samples that you find somewhere—because if they're not just flat-out wrong, they're at least often outdated and no longer reflect best practices. Learning to use Java cryptography APIs generally requires reading third party materials that allow you to understand sound cryptography fundamentals.

Luis Casillas
  • 10,181
  • 2
  • 27
  • 42
  • Thanks for your detailed response. The code is not mine and is used in a simple application with user credentials to access it. I noticed that users with the same password had the exact encrypted output in the DB so i wanted to understand why the application is written like that. As a complete beginner to Chryptography etc I noticed glaring flaws in the code. ( Encryption instead of hashing, users with the same password results in same result in db, able to log in by copying the encrypted password and entering it as the password etc) so i wanted to research and learn myself. – Display Name Jan 21 '17 at 01:01
  • It appears as through password encryption was not compulsory for legacy users, hence the code allowing for unencrypted passwords. Its all very interesting reading. I feel like i have opened a can of worms thats going to take months of reading to understand! – Display Name Jan 21 '17 at 01:15
0

You can understand theoretical parts of the DES algorithm in many places such as this and this.

What is doing private SecretKey getKey() ?

This method generates the Initial Vector (IV) which is feeding in to the DES algorithm using the SecretKeySpec(desKeyData, "DES") and generates the symmetric key. The loop inside the method just generates same IV when you required. I am not sure why you did it like that but simple you can do the following as well to generate the IV.

String myIv ="this is my IV"
new SecretKeySpec( myIv.getBytes() , "DES");



Cipher cipher = Cipher.getInstance("DES");

This code line instantiates an Object out of Cipher class using DEC algorithm. The init method initialising the cipher object using key and and the mode. I am not sure whetaher the code you have given is working or not. But you need to mention how you are going to use the cipher (ENCRYPT or DECRYPT) with mode as follows.

cipher.init(Cipher.ENCRYPT_MODE, key);

encryptedBytes = cipher.doFinal(bytes);

This line doing the actual encryption. Remember that the doFinal method doing the padding because it is a block cipher.

When decrypting, you need to use the mode as follows.

cipher.init(Cipher.DECRYPT_MODE, key); 

One more thing to mention here. It is better not to use String to store any passwords or sensitive information. Because String are immutable in most languages and can be acquired from memory. Better to use a byte array to store your sensitive data.

user3496510
  • 1,257
  • 2
  • 12
  • 26
  • Thanks for your great response, If copy the encrypted password from my DB and use it to log in i can successfully access the system without knowing the actual password. Can you explain why that happens? – Display Name Jan 19 '17 at 22:01
  • If the code is done properly, the only chance that this could happen is , your hashed value and the plain text value are equal. There is almost zero probability that could happen. There should be a path in your login code logic, the given password performs a comparison with your saved hash value directly. The password must be hashed before that comparison. – user3496510 Jan 20 '17 at 00:04
  • `getKey()` in that code is clearly not generating an IV—a term "IV" means something completely different in the context of ciphers. And there is no actual IV being used in the example code—which is a red flag that the answer doesn't mention. Therefore this answer is clearly wrong. – Luis Casillas Jan 20 '17 at 01:48