shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Demers <brian.dem...@gmail.com>
Subject Re: An idea on SHIRO-349
Date Mon, 15 Aug 2016 13:44:16 GMT
Thanks Eric!

I've been thinking a little bit about this too.  #2 and #3 are
defiantly something we should think about for 2.0, as they both are
not backwards compatible.

#1 could potentially still break things in 1.x, so something like that
would need some heavy test coverage.

Can you send your diff as a github pull request?  (an we can chat
about it further there?) https://github.com/apache/shiro

-Brian

On Mon, Aug 15, 2016 at 2:54 AM, Eric Cho <heyeric.cho@gmail.com> wrote:
> Hi All,
>
> I was looking into the Shiro's code-base and Jira issues. One issue that
> caught my eye was SHIRO-349, for eliminating sensitive information in byte
> array, such as password, key. It seems obvious that we should zero out the
> memory as soon as we're done with it to not expose it to potential
> attackers, e.g., memory dump.
> For the issue, I came to writing a small piece to mitigate that and suggest
> a new way of managing/zeroing out the variables. So far I found three
> methods to achieve the goal as follows:
>
>
> 1. Wiping out manually
>
> As soon as we're done with the byte array, we zero out the space by using
> CollectionUtils#wipe method I made. The finally block will ensure the memory
> clean up even if it throws any exceptions in the try block.
>
> byte[] decrypted = plainOut.toByteArray();
> try {
>     assertTrue(Arrays.equals(plaintext, decrypted));
> } finally {
>     CollectionUtils.wipe(decrypted);
> }
>
>
> 2. Try-with-resources idiom (Java 7+)
>
> Adding try-finally block on every usage of the sensitive byte array can be
> too much overhead. We can bring the try-with-resources idiom which used for
> releasing certain resources at end of the try block automatically. An
> example would be:
>
> try (ByteWrapper temp = ByteWrapper.wrap(byteSource.getBytes())) {
>     user.use(temp.getBytes());
> } catch (IOException e) {
>     // ignore
> }
>
> The byte array, temp.getBytes, should be deleted as soon as the try block is
> completed.
>
>
> 3. Limit access to the value by class design
>
> Advice developers to follow a certain rule like above isn't enough.
> Sometimes we want to make more powerful guideline that no one can break. I
> thought of ByteSourceBroker class which doesn't expose the byte array,
> rather we need to deliver how we'll use the value in our scenario
> (ByteSourceUser interface):
>
> ByteSourceBroker broker = aes.decrypt(ciphertext.getBytes(), key);
> broker.useBytes(new ByteSourceUser() {
>     @Override
>     void use(byte[] bytes) {
>         assertTrue(Arrays.equals(plaintext, bytes));
>     }
> });
> if (broker instanceof Destroyable) {
>     ((Destroyable) broker).destroy();
> }
>
> * If we use Java 8's lambda:
> broker.useBytes((bytes) -> assertTrue(Arrays.equals(plaintext, bytes)));
>
>
> I hope the above code snippets are pretty self-explanatory, but in case you
> need to see whole changes I made, please check the attachment.
>
> I'm looking forward to hearing your opinion on my approach for SHIRO-349,
> what you'd think on my code changes to improve it. Any comments would be
> appreciated.
>
> Thanks,
> Eric

Mime
View raw message