fixing the crypto factory beans

[Ian Young]

A while back (July last year?) I raised a JIRA case about X509CertificateFactoryBean:

   https://issues.shibboleth.net/jira/browse/JSE-1

The essence of this is that most of the classes in this package are factory beans which have to be given a *File*, and whose result is a crypto object of some kind, such as an X509Certificate. This is not very flexible, and in particular is pretty useless if you're trying to process packaged resources in a web application context. Unfortunately I've hit a hard wall on another project where that's exactly what I need to do, so my options are to move forward with a fix for these factories, or just replicate them in a working form in that other project. I hate forks, though, so I'd rather fix spring-extensions.

I think the principal use of these factories is in MDA configurations, and of course we're not at 1.0 there yet so just making all of these classes take a Resource instead of a File isn't a big problem.

There are also some uses of these beans in the IdP, both in places like the testbed and also in places like credentials.xml in idp-conf. That looks like it's going to be exposed to IdP deployers, so if we are going to make changes in this area I'd say it would be better to get it done before the alpha (i.e., ASAP).

Question: do we think I am missing any other significant uses of these classes in the wild, or in our own code?

I think File was the wrong starting point for these classes, so my preference would be to remove the existing methods with the File argument, establish new methods taking Resource instead, and adjust configurations everywhere as required.

If just zapping the setCertificateFile and other similar methods and replacing them with new methods which take Resource was seen as too severe, I'd want to fall back to adding the new methods, reimplementing the old methods in terms of the new interface, and probably deprecating the old interface as well unless we feel that File is actually what we want in some use case.

I'm inclined to make all the new methods setResource, as I don't think that individualising them actually adds much.

Question: any rationale for setCertificateResource/setPrivateKeyResource/etc. vs. setResource?

There's a second issue with these classes in that they don't close the streams they open:

   https://issues.shibboleth.net/jira/browse/JSE-2

Obvious fix to this is try-with-resource, and I think I'd clear this one at the same time.

Question: any reason not to do that too?

Bottom line: I have reasons to want to just get this cleared out now, and apart from those I also think this deserves to be fixed for the IdP alpha. Unless there's a good reason against doing this, I'd like to just get it done.

Cheers,

	-- Ian



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5943 bytes
Desc: not available
Url : http://shibboleth.net/pipermail/dev/attachments/20140529/05464cce/attachment.bin