5

In Java it is recommended to use char[] for storing passwords or other sensitive information to be able to clear it manually once the data is no longer needed.

How can such an array be cleared across all threads? If I understand it correctly threads might only perform changes in their cache, but not in the shared memory, so the following would not work reliably:

char[] password = ...
...
Arrays.fill(password, '\0');
  • Is this assumption correct or do the threads always write to the shared memory?
  • Is it necessary to use volatile (or other synchronization) to make sure the shared memory is updated?
    • Is a happens-before relationship required for this because the compiler / JVM would otherwise omit memory synchronization due to optimization?
  • Do other threads have to establish a happens-before relationship to clear the array content from their cache, or is this negligible? Possibly because the cache will be used for other more frequently accessed data and the array will be discarded, given that it is not actively used anymore.

Edit: The statement that char[] should be used for passwords was based on Why is char[] preferred over String for passwords?, however after looking at it again, this is also a little bit controversial.

Marcono1234
  • 5,856
  • 1
  • 25
  • 43
  • 1
    Why are you storing the password globally, accessible by all threads? Why not store it some `ThreadLocal` storage? This should get rid of the problem. – Turing85 Jun 30 '19 at 15:33
  • I presume he wants to share it between threads. ThreadLocal works the opposite way, it doesn't allow multiple threads to access the same variable but has a different one for each thread. – jbx Jun 30 '19 at 15:38
  • It seems to me that making the password globally accessible might be a security concern by itself. Also a ThreadLocal is a global variable (they're stored in a big global HashMap object). And finally this feels like a bit of a non-issue to me. If a thread can store a password in its cache then the code could also have made a permanent, separate copy too. Just clearing the main char array feels like it has to be "good enough" in this scenario. (But limiting visibility might be better.) – markspace Jun 30 '19 at 16:34
  • @Turing85 the password is not necessarily global, but it might be hard to guarantee that it is only ever used by the same thread. Simply using `CompletableFuture.runAsync` or similar already introduces a second thread which had access to it. – Marcono1234 Jun 30 '19 at 17:07

2 Answers2

2

Making the array reference volatile won't guarantee volatile access to it's contents. You could use AtomicIntegerArray if you want thread safe shared access. Otherwise you might want to wrap your char array into your custom class with synchronisation around it's methods. Although the latter will be less performant.

Note the using an array of characters instead of a string might not be truly more secure. Dumping the process memory during the time when your char array contains the data is still possible if your attacker has access to your machine, and if he does, you have much more serious concerns than this. Also, garbage collection might move your data elsewhere during it's compaction phase, leaving your password in the freed 'garbage' memory that hasn't been overwritten yet (given you are talking about shared members between threads this is even more likely to happen since your char array would be considered long lived and copied to memory spaces reserved for older generation objects).

jbx
  • 21,365
  • 18
  • 90
  • 144
  • 1. A Direct ByteBuffer is less susceptible to garbage collection. 2. "and if he does" should be "but if he does" – Jonathan Rosenne Jun 30 '19 at 15:51
  • It is an "and" not a "but". If the attacker can dump your process memory so easily to steal passwords then he has access to much more serious stuff of your host machine. – jbx Jun 30 '19 at 16:00
  • 1
    I think the idea behind using a char array for passwords is that a String might persist for a very long time, giving an attacker much much more time where the password is visible. But your point is valid I think: if an attacker has access to your main memory then a password might be the least of your worries. Worrying about L3 cache in this scenario seems to be bordering on hysteria. – markspace Jun 30 '19 at 16:38
  • Yes, I think this issue was a common concern when desktop java applications were more common, and you couldn't know what malware crap the user might have installed. The op hasn't specified whether this is actually a server side application or a desktop one so I guess an answer would be good for educational purposes. – jbx Jun 30 '19 at 17:51
1

I think jbx has a good answer. If an attacker has access to your main memory, you likely have bigger problems than worrying about a stray password string in memory. Worrying also about L3 cache seems rather overwrought.

But in the interest of code, I'll point out that while making an array volatile won't help, nearly every other form of synchronization will help. They all have semantics that require that all writes be made visible. So you can guarantee that changes to an array are visible.

public class Password {

  private final char[] password;

  public Password( char[] p ) {
     password = Arrays.copy( p, p.length );
  }

  public synchronized boolean compare( char[] p ) {
      return Arrays.equal( password, p );
  }

  public synchronized clear() {
    Arrays.fill(password, 42 );
  }
}

Code was not tested.

Here I use synchronized just to provide memory visibility. The atomicity is just a side effect and probably isn't needed.

markspace
  • 10,621
  • 3
  • 25
  • 39
  • The main worry I had with caching is whether a change to the array would only be performed in the cache, but not in the shared memory. Can this even happen or will writes always happen in the shared memory as well (though other threads might not see it, but that is a different story)? – Marcono1234 Jun 30 '19 at 17:10
  • Depends on CPU architecture. For Intel, no the caches are coherent and all CPUs will always see the writes. For other CPUs, it could happen, Java does not preclude it. But the writes to clear main memory should happen "soon" in normal executing code. The code above will prevent this from happening in all architectures, the Java spec requires that these writes are made visible to all threads. – markspace Jun 30 '19 at 17:38