Shibboleth XML Configuration file Validation

[Brent Putman]

On 9/28/12 11:04 AM, Mark O'Quinn wrote:
>
> Well, I'm only catching the ServiceException, if something goes wrong,
> during the reload 
> of the services (using the HttpServletHelper class) I trigger the
> operation of replacing the
> original and valid configuration files for the bad ones (the step 9 of
> my previous email), but this
> is something I want to leave it to Shibboleth, I mean, call the
> methods and classes Shibboleth uses  
> to handle this situation, instead of trying to recover from a failure
> by copying and replacing the files myself. 
>
> Right now I'm studying the way Shibboleth, like you said, prevents
> corrupt config objects from replacing 
> live/working ones, in order to do this the right way.


The way this is handled in the service class impls is, if any exception
is thrown when it's processing the new configuration, the old stuff is
(depending on where the exception is thrown) either left in place or put
back in place (all under the write lock).  See loadContext() and
onNewContextCreated(ApplicationContext newServiceContext) of the
reloadable services.  If in loadContext() the new Spring context is not
successfully created (i.e. throws), then it's never put in place in the
first place.  If the context is successfully created, then
onNewContextCreated is called.  If there's an exception there, the
'finally' block puts back the old values of the class-specific internal
properties that were being replaced.

This is all internal to the classes, so there's not really any external
methods you can call to accomplish the same thing.  You'd just have to
put the good versions of the files back and call reload() since that's
the only public API that you have. (Calling reload() might seemingly be
redundant, since the old config will still be in effect, but there's
internal state tracking that might need to be reset by calling reload()).  


>
>> Right, well, the other feedback is that all the work is in concurrency
>> safety.
>
> I'm little confuse here, so I'll explain myself through two examples:
>
> 1) Since Shibboleth can be setup to reload the configuration files by
> time intervals; let's say for example,
> that I start working with the files (reading, copying) and precisely
> in the middle of my operations, Shibboleth 
> starts reloading the files (because the time interval matches the
> time) through the services (HttpServletHelper)
> and since Shibboleth puts a Lock, If I didn't put a Lock first, I
> believe a problem will occur.


Yes, this would be a problem. :-)


>
> So I'm looking for a way in which I can put a Lock or use Shibboleth
> to put a Lock in order to prevent 
> any problems.
>



Yeah, I had been operating under the assumption that you would only do
time-based via Shib support, or only your on-demand reloading, but not
both.  The reader-writer locking methodology of the service classes is
internal to them (i.e. the ReadWriteLock is only exposed as protected,
not public), so I don't think you're going to be able to mix the 2
approaches as-is and get proper concurrency handling.  If you want
time-based reloading, you'll have to forgo enabling the Shib built-in
support for that and instead implement time-based reloading externally
along with your on-demand reloading.  There you will want to share a
lock between the time-based and on-demand reloading mechanisms.  And I
*think* you probably don't need the reader-writer semantic, so you can
probably get by with a simple mutex lock or perhaps even plain old
synchronized methods.

As far as implementing time-based reloading externally:  I think this is
fairly simple, just take a look at how BaseReloadableService sets up the
timer in initialize() using the ResourceChangeWatcher and
ResourceChangeListener.  I think the only difference is that you'll need
to supply a different impl of the ResourceChangeListener, since the
internal impl class of that (ConfigurationResourceListener) is
protected. In the onResource* methods just call the public method
reload() rather than loadContext().




> 2) I'm designing the provisioning of SPs to be dynamic and that it can
> be done by multiples users at the same time,
> so I working in putting a Lock to prevent concurrency issues.

Yeah, definitely need a lock for that.  I think the same mutex lock or
synchronized approach for the above would handle this case as well.