shiro-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Cho <heyeric....@gmail.com>
Subject Re: An idea on SHIRO-349
Date Tue, 16 Aug 2016 01:42:02 GMT
OK, I made a PR as you said in the repo. Thanks!
Eric

On Mon, Aug 15, 2016 at 10:44 PM, Brian Demers <brian.demers@gmail.com>
wrote:

> 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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message