en passant: is IndexingObjectStore safe?

[Brent Putman]

On 1/15/14 9:37 AM, Ian Young wrote:
> While reviewing Rod's list of orphaned or near-orphaned classes in java-support, I took a look at the IndexingObjectStore class. The intention of this, looking at its Javadoc, is to compress the in-memory storage of large objects which are repeated by using the objects' hash as an index into a reference-counted storage system. I think the prototypical application is X.509 certs in metadata.

Yes, this was a Chad-introduced thing from way back.  He put it in v2 a
long time ago (Dec 2008) after some profiling of the IdP showed that we
were storing a lot of cert data in memory.    So it's been in the IdP
for 5+ years at this point (unfortunately).  I had never really looked
at it in detail and had mostly forgotten that it was even there...


>
>
> In other words, if there are two objects which have different hashes, they had better have identical functionality or IndexedObjectStore will Do The Wrong Thing.
>
> That seems to be a dangerous assumption for a class to be making (and it is an assumption, as far as I can tell the code doesn't validate it). I can't see how one can guarantee that the assumption will hold for X509-certs-as-strings, which seems to be the OpenSAML use case. Of course, for most real data sets, the assumption will not be violated, but I am more than a little concerned that things might go wrong in hard to understand ways here.


It's a dangerous and invalid assumption, I agree.  OpenSAML can't
guarantee this assumption.  And IMHO we would never figure out this sort
of hash collision in a real world scenario, and worse, even if we could,
the only fix would be to ask someone to change their cert in some
trivial way to make the hash different (which is unacceptable).

IMHO program correctness always trumps optimization.  If we can't devise
a safe way to do this compression/memory optimization, then I think we
need to pull it and just revert cert and CRL impl classes to just
storing their strings like standard XMLObjects.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://shibboleth.net/pipermail/dev/attachments/20140115/f33aaf6a/attachment-0001.html