Unusual way of using java constructor-CodePudding

At some part of the code that I've been working on I am tempted to use the constructor in an unusual way. The goal is to have minimal to none sensitive values inside string pool.

public class MyClass{
    private String mySafeString;

    public MyClass(char[] aCharArray, String salt) {
         this.mySafeString = Encrypt.encrypt(new String(aCharArray), salt);
    }
    
    public String getMySafeString(){
         return this.mySafeString;
    }
}

...
public static void main(String[] args){
    String salt = getSaltFromSomewhere();
    char[] someKey = new char[]{'c', 'x'};
    var myObject = new MyClass(someKey, salt);
    var encryptedSafeString = myObject.getMySafeString();
}

By creating the String object like that I am avoiding the value's presence inside the string pool. Thereby, intending to leave it to GC to perish the object.

However, I fail to find any resource that forbids the usage of constructor like the way I used it. I am keen to know if it is a bad practice in any way?

Could you please be so kind and advise me on my approach?

CodePudding user response：

That's a standard way to convert a char[] to a String.

You might also want to see if your encryption library can work from a char[] or InputStream, so that you can avoid creating any String.

CodePudding user response：

The goal is to have minimal to none sensitive values inside string pool.

That's a non-goal. Strings only get placed in the string pool if they are either compile time constants OR if some code explicitly calls String.intern to put them in the string pool.

Your real goal is to avoid leaving sensitive information somewhere in memory ... or in a swap file ... where a bad guy may be able to retrieve it.

However, I fail to find any resource that forbids the usage of constructor like the way I used it. I am keen to know if it is a bad practice in any way?

I doubt that you will find anything that forbids this¹.

However, it is not clear from the code what it achieves and how you intend to use it.

Is it your intention that the "safe" string be decryptable or not?

If Yes, you have a big problem. In order to decrypt, you need the decryption key in memory at some time. If the cipher algorithm is symmetric, that means that salt is the decryption key. But you have already put that onto the heap as a String object. (Ooops!)

And if you avoid that oops, the next problem is that the bad guy will be able to reverse engineer getSaltFromSomewhere() or the equivalent that generates the decryption key in the case of an asymmetric cipher. They may then be able to recover the key. Note that in order to successfully decrypt, your application must be able to obtain the correct key ... in memory ... to do the decryption.
If No, then it strikes me that the "safe" string could actually be replaced with a cryptographic hash of the original string. You don't necessarily need a salt for that. And if you do need one, you don't need the seed to be generated securely.

Also, it is not sufficient to just use a char[] to avoid storing the unencrypted (etc) secret in a String. The char[] is also in the heap. So it is also imperative that you overwrite the char[] when you are finished with it. A secret in a char[] or byte[] or Buffer, etc can be found just as easily as a secret in a String.

Finally, you probably should be doing a thorough risk assessment to decide whether the effort of protecting strings in the heap is actually going to be worth it. What is the chance that the bad guys can get in to capture a heap dump? If they can get in that way, can they intercept the secrets some other way; e.g. by surreptitiously replacing some of your Java app code, attaching a debugger, stealing keys out of config files, etc.

^{1 - Normal Java coding standards don't concern themselves with protecting secrets, and security standards don't concern themselves with Java style, etc issues.}

In short, there is nothing wrong with the constructor per se, but the chances are that it doesn't solve the real problem.

CodePudding user response：

I think you mean safe means that the string won't be modified outside of the class. As a result, you are making a copy.

You don't need to do this for String because strings are immutable. There's no way to modify the string outside of the class even if there's a reference to the string outside of the class.