Keyfile Generator

This is a Java program that generates a random keyfile (for OTP, for example).

package kfgen;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.security.SecureRandom;
import java.util.Random;
import java.util.Scanner;

public class Kfgen {

    // At least it's not a magic number.
    protected static final int MEGABYTE = 1048576;

    public static void main(String[] args) {

    System.out.println("Name?\n>");

    // choice: Scanner to get filename.
    // bos: output stream to write the keyfile
    try (final Scanner choice = new Scanner(System.in);
        final BufferedOutputStream bos = new BufferedOutputStream(
            new FileOutputStream(
                new File(choice.next()))
        )) {

        // Slow prng, but hey, this is cryptography, so ... IANAC
        final Random sr = SecureRandom.getInstanceStrong();

        System.out.println("Enter size in GB:\n>");

        // MultiplyExact to throw ugly exception if too large.
        // Dunno if this ever happens.
        int bytesize = Math.multiplyExact(choice.nextInt(), 1024);

        // Tell how many bytes.
        System.out.println(bytesize * MEGABYTE + " bytes!");

        // Write all that data in 1 MB chunks
        // (otherwise byte[] can't hold that much).
        // Given that I'm pseudo-buffering at 1 MB, should I use
        // unbuffered fileout?
        for (int i = 0; i < bytesize; i++) {
        final byte[] rand = new byte[MEGABYTE];
        sr.nextBytes(rand);
        bos.write(rand);
        }
    } catch (Exception ex) {
        throw new Error(ex); // I don't want to deal with it.
    }

    }

}

Please evaluate this for security, coding practice, and efficiency.

Your code is pretty good :-)

However, there a few things that could be improved.

// At least it's not a magic number.
protected static final int MEGABYTE = 1048576;

is still pretty magic, 1024 * 1024 would seem more readable. It should not be protected without reason. For constants, use private if unsure, public if you need access from other classes. protected should be used if it should be visible to subclasses and there is a specific reason not to use public.

} catch (Exception ex) {
    throw new Error(ex); // I don't want to deal with it.
}

Don't throw Errors. Use some kind of RuntimeException. If you are in your main method and you do not care, just rethrow it.

It is generally advisable to separate concerns. In this case you read some user input and use this to write (crypto-usable) random data to a file.

Applying some object orientation would go a long way to separate these two unrelated tasks.

Please consider the following code:

package kfgen;

import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Scanner;

public class Kfgen {
    private static final int MEGABYTE = 1024 * 1024;
    private static final long GIGABYTE = MEGABYTE * 1024;

    private final SecureRandom prng;

    public Kfgen(SecureRandom prng) {
        this.prng = prng;
    }

    public void generate(File keyFile, int gigabytes) throws IOException {
        // MultiplyExact to throw ugly exception if too large.
        int megabytes = Math.multiplyExact(gigabytes, 1024);

        try (OutputStream out = new BufferedOutputStream(new FileOutputStream(
                keyFile))) {
            final byte[] buffer = new byte[MEGABYTE];
            for (int i = 0; i ");
            String filename = scanner.next();

            System.out.println("Enter size in GB:\n>");
            int gigabytes = scanner.nextInt();

            // Tell how many bytes.
            System.out.println(gigabytes * GIGABYTE + " bytes!");

            Kfgen kfgen = new Kfgen(SecureRandom.getInstanceStrong());
            kfgen.generate(new File(filename), gigabytes);
        }
    }
}

StackExchange Code Review Q#51178, answer score: 3