3
\$\begingroup\$

In an attempt to learn Java and SHA-2 I created a very simple String-to-SHA512 generator. Here is the code:

package main;
import java.util.Scanner;
import java.math.BigInteger;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

public class SHA512_String_Hash {
    public static void main(String args[]) throws NoSuchAlgorithmException {
        Scanner inputScanner = new Scanner(System.in);
        System.out.print("Text Input: ");
        String input = inputScanner.next();

        MessageDigest objSHA = MessageDigest.getInstance("SHA-512");
        byte[] bytSHA = objSHA.digest(input.getBytes());
        BigInteger intNumber = new BigInteger(1, bytSHA);
        String strHashCode = intNumber.toString(16);

        while (strHashCode.length() < 128) {
            strHashCode = "0" + strHashCode;
        }
        System.out.println("SHA-512: \n" + strHashCode);
    }
}

Since I am new to SHA-512 in Java, I do not know the optimal (for performance and more importantly security) way to create a SHA-512 hash generator.

TLDR— How could the performance and security of this code be improved?

\$\endgroup\$
  • 3
    \$\begingroup\$ We can't help you improve security if you don't tell us the purpose of this code. \$\endgroup\$ – CodesInChaos Aug 2 '16 at 10:06
4
\$\begingroup\$
  1. You should extract the hex encoding into its own method.

    See How to convert a byte array to a hex string in Java? for various implementations.

  2. input.getBytes() uses the locale dependent legacy encoding. So you won't get reproducible results on different systems. Specify an encoding explicitly, I recommend utf-8.

\$\endgroup\$
2
\$\begingroup\$
    String strHashCode = intNumber.toString(16);

    while (strHashCode.length() < 128) {
        strHashCode = "0" + strHashCode;
    }

This seems needlessly expensive.

    String strHashCode = intNumber.toString(16);

    if (strHashCode.length() < 128) {
        char[] characters = strHashCode.toCharArray();
        char[] newString = new char[128];
        Arrays.fill(newString, '0');
        System.arraycopy(characters, 0, newString, 128-characters.length, characters.length);
        strHashCode = new String(newString);
    }

The problem with prepending 0 in a while loop is that you're constantly creating a new String, which involves a lot of copying characters around. This method I just described in code should work just fine, but perhaps there's faster methods around. That said, this should still be faster than padding the String one character at a time. Of course, the chance we actually need this code is pretty small... so you could leave it as is. Or maybe opt for a StringBuilder, which would be a lot less messy.


As for security... There are no security concerns! A hash function like this calculates a hash, then returns it as String. That's perfectly fine for now. This application as is doesn't have to hide anything, and thus there are no security concerns. It's only once you combine this with other applications that there may be security concerns. That said... SHA-512 is not suited for passwords as it's a fast-hashing algorithm, so if you were thinking in that direction; stop.

\$\endgroup\$
2
\$\begingroup\$

I just commenting on these lines:

String strHashCode = intNumber.toString(16);

while (strHashCode.length() < 128) {
    strHashCode = "0" + strHashCode;
}

I would use this instead:

String strHashCode = String.format("%0128x", intNumber);
\$\endgroup\$
0
\$\begingroup\$

Characters vs. Bytes

Hash algorithms usually operate on byte sequences, not on character sequences. The difference between the two is that bytes are a just bit octets without a canonical meaning while characters carry a semantic. To extract semantic from a sequence of bits or bit octets (i. e. bytes) one must interpret it, which requires an assumption about the encoding of that sequence. Since there are plenty of character encodings we cannot make a valid assumption here and we don't need to and shouldn't since hash algorithms operate on bytes anyway – at least those you mention and which are supported by MessageDigest.

Read bytes directly

When you extract bytes from System.in, an implementation of InputStream which operates on bytes, you read exactly one line (according to the current character encoding) from a Scanner backed by System.in and convert it back up bytes. Instead you should read the bytes directly and feed them to the MessageDigest instance. I'll use a separate method for that to decouple the stream digestions from the digest creation and output.

public static byte[] digestStream( MessageDigest md, InputStream in )
    throws IOException
{
    byte[] buffer = new byte[8 << 10];
    int n;
    while ((n = in.read(buffer)) != -1)
        md.update(buffer, 0, n);
    return md.digest();
}

You can then use another function to convert the returned byte array to a hexadecimal digit string (e. g. from How to convert a byte array to a hex string in Java? as CodeInChaos suggests). Or you can roll your own since it's so simple:

public static void appendHexDigits( byte[] bytes, Appendable ap )
    throws IOException
{
    for (byte b: bytes)
    {
        ap.append(Character.forDigit((b >>> 4) & 0xF, 16));
        ap.append(Character.forDigit(b & 0xF, 16));
    }
}

A main method tying the two together could look like this (minus exception handling):

public static void main(String[] args)
{
    byte[] digest = digestStream(MessageDigest.getInstance("SHA-512"), System.in);
    appendHexDigits(digest, System.out);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.