Simple hashing and salting algorithm

Question

I have devised what I feel to be a simple hash/salting algo for Java, and am looking for some feedback from people more skilled than myself. Please find below my code;

import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.NoSuchProviderException;
import java.security.SecureRandom;
public class PassSecureSuite
{
 /**
 * Method: RecoverPass
 * Function: This class contains methods for authenticating the user against the database and creating
 * a new user with a secure password.
 */
 public static String verifyIput(String submittedPass, String salt)
 {
 MessageDigest soMd = null;
 try
 {
 soMd = MessageDigest.getInstance("SHA-256");
 }
 catch (NoSuchAlgorithmException e)
 {
 e.printStackTrace();
 }
 soMd.update(salt.getBytes());
 byte[] abytesToString = soMd.digest(submittedPass.getBytes());
 StringBuilder buildThis = new StringBuilder();
 for(int i=0; i<abytesToString.length; i++)
 {
 buildThis.append(Integer.toString((abytesToString[i] & 0xff) + 0x100, 16).substring(1));
 }
 String passToVerify = buildThis.toString();
 return passToVerify;
 }
 /**
 * Method: getSalt
 * Function: Generate a secure, randomly-generated string to add extra security to the user password
 */
 public static String getSalt()
 {
 SecureRandom sosRand = null;
 try
 {
 sosRand = SecureRandom.getInstance("SHA1PRNG", "SUN");
 }
 catch (NoSuchAlgorithmException e)
 {
 e.printStackTrace();
 }
 catch (NoSuchProviderException e)
 {
 e.printStackTrace();
 }
 byte[] baniceAndSalty = new byte[16];
 sosRand.nextBytes(baniceAndSalty);
 return baniceAndSalty.toString();
 }
 /**
 * Method: agenerateSecurePass
 * Function: Generates a password string based on a byte feed
 */
 public static String sgenSecurePass(String password, String salt)
 {
 String ageneratedPass = null;
 MessageDigest digest = null;
 try
 {
 digest = MessageDigest.getInstance("SHA-256");
 }
 catch (NoSuchAlgorithmException nsae)
 {
 nsae.printStackTrace();
 }
 digest.update(salt.getBytes());
 byte[] babytes = digest.digest(password.getBytes());
 StringBuilder obuildThis = new StringBuilder();
 for(int i=0; i < babytes.length; i++)
 {
 obuildThis.append(Integer.toString((babytes[i] & 0xff) + 0x100, 16).substring(1));
 }
 ageneratedPass = obuildThis.toString();
 return ageneratedPass;
 }
}

I am looking to make this as secure as possible without using other people's APIs, as I enjoy having total control over my security within applications.

The above example appears fairly robust to me, as I have been unable to break the hash/salt on any application I have built, but I feel that I can improve this, I am just not sure how.

• mat: Could you elaborate your reasons for using that weird looking StringBuilder.append() line.
• Michael Wiggins: @mat please accept my apologies for taking some time to get back to you, I have been very unwell of late. StringBuilder.append() Line you are referencing, builds a string from the associated parameters with less overhead than string concatenation.

Answer 1

I spotted the following problems:

1. Exception handling When you catch an exception, you just print its stack trace but then keep going as if nothing happened, forcing another exception a few lines down the code. You should change this
2. Byte conversion: Your getSalt method converts the byte array holding the salt into a string, the other 2 methods try to convert that string back into a byte array. Firstly, I think that you are doing that conversion wrong. But event if that was not the case, you should not do that. A byte array should be kept as a byte array. Unneccesary conversion can always cause errors. If you absolutely need a string, use Base64 encoding.
3. You should not simply use SHA256(Salt | password). Use HMAC-SHA256 instead.
4. Actually, SHA-256 or HMAC-SHA256 are not good functions to hash a password. You should use a dedicated password hashing KDF, which are much better at resisting brute force attacks. Good examples are scrypt, bcrypt, PKBKDF2, Argon2.
5. I still don't get the strange way you are converting your password hash into a string at the end. Use Base64 instead.
6. Your essential code (the password hashing) is duplicated in two methods. Don't do that.

Answer 2

If you couldn't crack your password hashing, that doesn't mean anything. Others will be happy to crack it for you. Mainly because your hashing function is waaay too fast, which makes it easy to try all common passwords. Good password hashing functions are slow by intention, so use them instead of trying your own.

On another topic: remove all comments from your code. Half of them are wrong and misleading, the other half are redundant.

And what are these weird variable names and their prefixes, such as so and abytes. They are really confusing.

Check for typos in your code. I never heard of the word iput.

• Michael Wiggins: Thanks for the feedback, it is greatly appreciated!! The comments were included as this was in-house notation for software projects. Again, variable denotation such as abytes was also in-house notation. On the topic of speed, I'd love some homework on the matter, always happy to learn.