Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How safe is my password decrypting class?

I'm a complete beginner to any sort of decrypting. I wrote a class that I think should be quite secure. Can you give me constructive criticism of how could I improve the algorithm.

package main;

import java.util.Random;

public class Main {

   public static void main(String[] args) {
      //we will be playing around with this string
      new Main("1234567890abc");
   }

   private Random rnd;
   private byte[] randoms;

   /**
    * Starts up RNG
    * Prints out a test
    */
   public Main(String password) {
      //random long generated from the password
      long randomLong = randomNumber(password);
      //Random class using randomLong as seed
      rnd = new Random(randomLong);
      randoms = new byte[password.length()];
      //Array of random bytes generated with rnd
      rnd.nextBytes(randoms);

      System.out.println(randomNumber(password));

      String cryped = encrypt(password);
      String decryped = decrypt(cryped);

      System.out.println(cryped);
      System.out.println(decryped);
   }

   /**
    * Encrypts the password.
    */
   private String encrypt(String password) {
      char[] chars = password.toCharArray();      
      for (int i = 0; i < chars.length; i++) {
         chars[i] = (char) (chars[i] + randoms[i]);
      }
      return String.valueOf(chars);
   }

   /**
    * Decrypts an allready encryped password.
    */
   private String decrypt(String crypted) {
      char[] chars = crypted.toCharArray();
      for (int i = 0; i < chars.length; i++) {
         chars[i] = (char) (chars[i] - randoms[i]);
      }
      return String.valueOf(chars);
   }

   /**
    * Finds a random number BASED ON PASSWORD
    */
   private long randomNumber(String password)
   {
      char[] chars = password.toCharArray();
      long number = 0;
      for (char c : chars) {
         number += c;
      }
      number *= chars.length;
      return number;
   }
}

The class is written in Java but should be readable to anyone.

like image 922
Rob Fox Avatar asked Dec 01 '22 02:12

Rob Fox


2 Answers

  1. Don't reinvent your own cryptography in real life (is this an excercise?) Even experts make mistakes. Use something that's been publically scrutinized.
  2. The java random number generator is not cryptographically secure. With a long enough text to encrypt, patterns will emerge that can permit various information leaks upto and including revealing the password (but see point three) and the plaintext.
  3. You use the password to seed the random number generator. This is a standard (and fine) idea, but you do so using an algorithm that is invariant to permutation! i.e. Your encryption treats "sinecure" and "insecure" or other passwords that are anagrams as equivalent (and probably others too). For a strong password of up to 16 letters and no codepoints beyond 255, the highest reachable seed is 255*16*16 = 65280; but there are even fewer possibilities since there are seeds lower than this which are not reachable. On my keyboard, bruteforcing shows just 9734 different seeds for passwords consisting solely of keyboard-writable characters excluding newline (I count 95) of up to 16 characters in length; that's less than 1 bit of entropy per letter.
  4. CodeInChaos has a few additional observations in his answer: you're using a stream cipher (even harder to get right!). You're also encrypting the password which suggests you may be looking for a hash not an encryption function, (or is that just an example?).
  5. By the way if you're trying to store passwords; don't - not even encrypted! See the sony fiasco for why; you may get hacked, you may lose your password database, and your encryption keys may be known to the attacker. Instead, use standard, best-practice password hashing (and prefer a standard preexisting component if possible). Such a system should at least use a secure hash such as sha1 or better; passwords should be individually salted (the salt can be stored plaintext), and the process should be made computationally expensive to make brute-force unattractive. See http://chargen.matasano.com/chargen/2007/9/7/enough-with-the-rainbow-tables-what-you-need-to-know-about-s.html for details.
like image 160
Eamon Nerbonne Avatar answered Dec 05 '22 00:12

Eamon Nerbonne


Horribly broken in more than one way.

  1. Your key is 64 bit, a bit small for today. But that's the least of your worries for now.
  2. I see a non cryptographic PRNG. You need to use a crypto PRNG.
  3. You're reusing a key in a stream cypher. Stream cyphers are notorious for being hard to use correctly. In this mode of operation it basically behaves like a one-time-pad generated by a PRNG. Once you reuse a key in such a mode, your crypto is broken.
    Suppose an attacker knows encrypt(p1) and encrypt(p2). Then he can calculate encrypt(p1)-encrypt(p2) which is identical to p1-p2.
  4. Your effective key size is much smaller than 64bit. The sum of chars in a string is <2^16*length. And for most characters it's even <128. So you're key will usually be a number <1000'000. That's trivial to brute force.
  5. Each element in randoms is a byte i.e. 8 bit. A char is 16 bit. So you're not adding modulo 256. Thus you leak information about the encrypted password.

And to improve it, throw out your own algorithm entirely and use a well know, reviewed algorithm. Inventing your own algorithm is a bad idea unless you're a cryptography expert. And even expert make mistakes relatively often.

And do you really need password decryption (i.e. is it a password store), or is password hashing enough?

My suggestion is to put your master password in a key-derivation-function (PKDF2 is a common choice). Then use the key this function returns to encrypt the rest of your data file using AES.

like image 43
CodesInChaos Avatar answered Dec 05 '22 00:12

CodesInChaos