Skip to content

Commit

Permalink
Improving initialization of the trust filter for metadata - filter is…
Browse files Browse the repository at this point in the history 
… now combined with any pre-existing filters.

Solved potential deadlock issues in MetadataManager.
All entities are now internally stored as ExtendedMetadataDelegates in MetadataManager.
Removed obsolete initialization from the security xml configs.
  • Loading branch information
@vschafer
vschafer committed Apr 3, 2011
1 parent b557c90 commit b4f19c3
Show file tree
Hide file tree
Showing 11 changed files with 171 additions and 77 deletions.
26 changes: 23 additions & 3 deletions ...re/src/main/java/org/springframework/security/saml/metadata/AbstractMetadataDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public abstract class AbstractMetadataDelegate implements ObservableMetadataProv
/**
* Wrapped entity the calls are delegated to.
*/
private AbstractMetadataProvider delegate;
private MetadataProvider delegate;

/**
* Observers, loaded from the provider.
Expand All @@ -49,7 +49,7 @@ public abstract class AbstractMetadataDelegate implements ObservableMetadataProv
*
* @param delegate delegate to use, can't be null
*/
public AbstractMetadataDelegate(AbstractMetadataProvider delegate) {
public AbstractMetadataDelegate(MetadataProvider delegate) {
Assert.notNull(delegate, "Delegate can't be null");
this.delegate = delegate;
if (delegate instanceof ObservableMetadataProvider) {
Expand Down Expand Up @@ -102,8 +102,28 @@ public List<Observer> getObservers() {
/**
* @return original object the calls are delegated to
*/
public AbstractMetadataProvider getDelegate() {
public MetadataProvider getDelegate() {
return delegate;
}

/**
* Equality is based on the object this class delegates to.
* @param obj object
* @return true when obj equals delegate, in case obj is a wrapper itself only its delegate is compared
*/
@Override
public boolean equals(Object obj) {
if (obj instanceof ExtendedMetadataDelegate) {
ExtendedMetadataDelegate del = (ExtendedMetadataDelegate) obj;
return delegate.equals(del.getDelegate());
} else {
return delegate.equals(obj);
}
}

@Override
public int hashCode() {
return delegate.hashCode();
}

}
43 changes: 39 additions & 4 deletions ...re/src/main/java/org/springframework/security/saml/metadata/ExtendedMetadataDelegate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

import org.opensaml.saml2.metadata.EntityDescriptor;
import org.opensaml.saml2.metadata.provider.AbstractMetadataProvider;
import org.opensaml.saml2.metadata.provider.MetadataProvider;
import org.opensaml.saml2.metadata.provider.MetadataProviderException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Map;
import java.util.Set;
Expand All @@ -27,6 +30,9 @@
*/
public class ExtendedMetadataDelegate extends AbstractMetadataDelegate implements ExtendedMetadataProvider {

// Class logger
protected final Logger log = LoggerFactory.getLogger(ExtendedMetadataDelegate.class);

/**
* When true metadata will only be accepted if correctly signed.
*/
Expand Down Expand Up @@ -61,12 +67,18 @@ public class ExtendedMetadataDelegate extends AbstractMetadataDelegate implement
*/
private Map<String, ExtendedMetadata> extendedMetadataMap;

/**
* Flag indicates that delegated metadata already contains all information required to perform signature
* and trust verification of the included metadata.
*/
private boolean trustFiltersInitialized;

/**
* Uses provider for normal entity data, for each entity available in the delegate returns given defaults.
*
* @param delegate delegate with available entities
*/
public ExtendedMetadataDelegate(AbstractMetadataProvider delegate) {
public ExtendedMetadataDelegate(MetadataProvider delegate) {
this(delegate, null, null);
}

Expand All @@ -76,7 +88,7 @@ public ExtendedMetadataDelegate(AbstractMetadataProvider delegate) {
* @param delegate delegate with available entities
* @param defaultMetadata default extended metadata, can be null
*/
public ExtendedMetadataDelegate(AbstractMetadataProvider delegate, ExtendedMetadata defaultMetadata) {
public ExtendedMetadataDelegate(MetadataProvider delegate, ExtendedMetadata defaultMetadata) {
this(delegate, defaultMetadata, null);
}

Expand All @@ -86,7 +98,7 @@ public ExtendedMetadataDelegate(AbstractMetadataProvider delegate, ExtendedMetad
* @param delegate delegate with available entities
* @param extendedMetadataMap map, can be null
*/
public ExtendedMetadataDelegate(AbstractMetadataProvider delegate, Map<String, ExtendedMetadata> extendedMetadataMap) {
public ExtendedMetadataDelegate(MetadataProvider delegate, Map<String, ExtendedMetadata> extendedMetadataMap) {
this(delegate, null, extendedMetadataMap);
}

Expand All @@ -98,7 +110,7 @@ public ExtendedMetadataDelegate(AbstractMetadataProvider delegate, Map<String, E
* @param defaultMetadata default extended metadata, can be null
* @param extendedMetadataMap map, can be null
*/
public ExtendedMetadataDelegate(AbstractMetadataProvider delegate, ExtendedMetadata defaultMetadata, Map<String, ExtendedMetadata> extendedMetadataMap) {
public ExtendedMetadataDelegate(MetadataProvider delegate, ExtendedMetadata defaultMetadata, Map<String, ExtendedMetadata> extendedMetadataMap) {
super(delegate);
if (defaultMetadata == null) {
this.defaultMetadata = new ExtendedMetadata();
Expand Down Expand Up @@ -143,6 +155,21 @@ public ExtendedMetadata getExtendedMetadata(String entityID) throws MetadataProv

}

/**
* Method performs initialization of the provider it delegates to.
*
* @throws MetadataProviderException in case initialization fails
*/
public void initialize() throws MetadataProviderException {
if (getDelegate() instanceof AbstractMetadataProvider) {
log.debug("Initializing delegate");
AbstractMetadataProvider provider = (AbstractMetadataProvider) getDelegate();
provider.initialize();
} else {
log.debug("Cannot initialize delegate, doesn't extend AbstractMetadataProvider");
}
}

/**
* If set returns set of keys which can be used to verify whether signature of the metadata is trusted. When
* not set any of the keys in the configured KeyManager can be used to verify trust.
Expand Down Expand Up @@ -211,4 +238,12 @@ public void setForceMetadataRevocationCheck(boolean forceMetadataRevocationCheck
this.forceMetadataRevocationCheck = forceMetadataRevocationCheck;
}

protected boolean isTrustFiltersInitialized() {
return trustFiltersInitialized;
}

protected void setTrustFiltersInitialized(boolean trustFiltersInitialized) {
this.trustFiltersInitialized = trustFiltersInitialized;
}

}
120 changes: 84 additions & 36 deletions .../saml2-core/src/main/java/org/springframework/security/saml/metadata/MetadataManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public class MetadataManager extends ChainingMetadataProvider implements Extende
// Timer used to refresh the metadata upon changes
private Timer timer;

// Internal of metadata refresh checking
// Internal of metadata refresh checking in ms
private long refreshCheckInterval = 10000l;

// Flag indicating whether metadata needs to be reloaded
Expand All @@ -78,7 +78,7 @@ public class MetadataManager extends ChainingMetadataProvider implements Extende
protected KeyManager keyManager;

// All providers which were added, not all may be active
private List<MetadataProvider> availableProviders;
private List<ExtendedMetadataDelegate> availableProviders;

/**
* Set of IDP names available in the system.
Expand Down Expand Up @@ -112,7 +112,7 @@ public MetadataManager(List<MetadataProvider> providers) throws MetadataProvider
this.idpName = new HashSet<String>();
this.spName = new HashSet<String>();
this.defaultExtendedMetadata = new ExtendedMetadata();
availableProviders = new LinkedList<MetadataProvider>();
availableProviders = new LinkedList<ExtendedMetadataDelegate>();

setProviders(providers);
getObservers().add(new MetadataProviderObserver());
Expand Down Expand Up @@ -159,15 +159,20 @@ public void setProviders(List<MetadataProvider> newProviders) throws MetadataPro
lock.writeLock().lock();

availableProviders.clear();
availableProviders.addAll(newProviders);
setRefreshRequired(true);
if (newProviders != null) {
for (MetadataProvider provider : newProviders) {
addMetadataProvider(provider);
}
}

} finally {

lock.writeLock().unlock();

}

setRefreshRequired(true);

}

/**
Expand Down Expand Up @@ -202,12 +207,14 @@ public void refreshMetadata() {
spName = new HashSet<String>();
aliasSet = new HashSet<String>();

for (MetadataProvider provider : availableProviders) {
for (ExtendedMetadataDelegate provider : availableProviders) {

try {

log.debug("Refreshing metadata provider {}", provider.toString());
initializeProviderFilters(provider);
initializeProvider(provider);
initializeProviderData(provider);

// Make provider available for queries
super.addMetadataProvider(provider);
Expand Down Expand Up @@ -257,34 +264,75 @@ public void addMetadataProvider(MetadataProvider newProvider) throws MetadataPro
try {

lock.writeLock().lock();
availableProviders.add(newProvider);
setRefreshRequired(true);

ExtendedMetadataDelegate wrappedProvider = getWrappedProvider(newProvider);
availableProviders.add(wrappedProvider);

} finally {
lock.writeLock().unlock();
}

setRefreshRequired(true);

}


private ExtendedMetadataDelegate getWrappedProvider(MetadataProvider provider) {
if (!(provider instanceof ExtendedMetadataDelegate)) {
log.debug("Wrapping metadata provider {} with extendedMetadataDelegate", provider);
return new ExtendedMetadataDelegate(provider);
} else {
return (ExtendedMetadataDelegate) provider;
}
}

@Override
public void removeMetadataProvider(MetadataProvider provider) {

if (provider == null) {
return;
}

try {

lock.writeLock().lock();
availableProviders.remove(provider);
super.removeMetadataProvider(provider);
setRefreshRequired(true);

ExtendedMetadataDelegate wrappedProvider = getWrappedProvider(provider);
availableProviders.remove(wrappedProvider);
super.removeMetadataProvider(wrappedProvider);

} finally {
lock.writeLock().unlock();
}

setRefreshRequired(true);

}

private void initializeProvider(MetadataProvider provider) throws MetadataProviderException {
/**
* Method is expected to make sure that the provider is properly initialized. Also all loaded filters should get
* applied.
*
* @param provider provider to initialize
* @throws MetadataProviderException error
*/
protected void initializeProvider(ExtendedMetadataDelegate provider) throws MetadataProviderException {

initializeProviderFilters(provider);
// Initialize provider and perform signature verification
log.debug("Initializing extendedMetadataDelegate {}", provider);
provider.initialize();

}

/**
* Method populates local storage of IDP and SP names and verifies any name conflicts which might arise.
*
* @param provider provider to initialize
* @throws MetadataProviderException error
*/
protected void initializeProviderData(ExtendedMetadataDelegate provider) throws MetadataProviderException {

log.debug("Initializing provider data {}", provider);

List<String> stringSet = parseProvider(provider);

Expand Down Expand Up @@ -368,40 +416,40 @@ private void initializeProvider(MetadataProvider provider) throws MetadataProvid
* @param provider provider to check
* @throws MetadataProviderException in case initialization fails
*/
protected void initializeProviderFilters(MetadataProvider provider) throws MetadataProviderException {

AbstractMetadataProvider metadataProvider;

if (provider instanceof ExtendedMetadataDelegate) {
metadataProvider = ((ExtendedMetadataDelegate) provider).getDelegate();
} else if (provider instanceof AbstractMetadataProvider) {
metadataProvider = (AbstractMetadataProvider) provider;
} else {
throw new MetadataProviderException("Metadata provider doesn't extend AbstractMetadataProvider class");
}
protected void initializeProviderFilters(ExtendedMetadataDelegate provider) throws MetadataProviderException {

if (metadataProvider.isInitialized()) {
if (provider.isTrustFiltersInitialized()) {

log.debug("Metadata provider was already initialized, signature validation will be skipped");
// TODO check our filter is included, warn/fail otherwise

} else {

boolean requireSignature = false;
boolean requireSignature = provider.isMetadataRequireSignature();
SignatureTrustEngine trustEngine = getTrustEngine(provider);
SignatureValidationFilter filter = new SignatureValidationFilter(trustEngine);
filter.setRequireSignature(requireSignature);

if (provider instanceof ExtendedMetadataDelegate) {

ExtendedMetadataDelegate metadata = (ExtendedMetadataDelegate) provider;
requireSignature = metadata.isMetadataRequireSignature();
log.debug("Created new trust manager for metadata provider {}", provider);

// Combine any existing filters with the signature verification
MetadataFilter currentFilter = provider.getMetadataFilter();
if (currentFilter != null) {
if (currentFilter instanceof MetadataFilterChain) {
log.debug("Adding trust filter into existing chain");
MetadataFilterChain chain = (MetadataFilterChain) currentFilter;
chain.getFilters().add(filter);
} else {
log.debug("Combining filter with the existing in a new chain");
MetadataFilterChain chain = new MetadataFilterChain();
chain.getFilters().add(currentFilter);
chain.getFilters().add(filter);
}
} else {
log.debug("Adding signature filter");
provider.setMetadataFilter(filter);
}

SignatureValidationFilter filter = new SignatureValidationFilter(trustEngine);
filter.setRequireSignature(requireSignature); // TODO combine existing filters
provider.setMetadataFilter(filter);

metadataProvider.initialize();
provider.setTrustFiltersInitialized(true);

}

Expand Down
5 changes: 2 additions & 3 deletions ...ml/saml2-core/src/test/resources/org/springframework/security/saml/SAMLEntryPointTest.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,13 @@
<bean id="metadata" class="org.springframework.security.saml.metadata.MetadataManager" depends-on="bootstrap">
<constructor-arg index="0">
<list>
<bean class="org.opensaml.saml2.metadata.provider.FilesystemMetadataProvider" init-method="initialize">
<bean class="org.opensaml.saml2.metadata.provider.FilesystemMetadataProvider">
<constructor-arg>
<value type="java.io.File">classpath:testIDP.xml</value>
</constructor-arg>
<property name="parserPool" ref="parserPool"/>
</bean>
<bean class="org.opensaml.saml2.metadata.provider.FilesystemMetadataProvider"
init-method="initialize">
<bean class="org.opensaml.saml2.metadata.provider.FilesystemMetadataProvider">
<constructor-arg>
<value type="java.io.File">classpath:testSP.xml</value>
</constructor-arg>
Expand Down
Loading

0 comments on commit b4f19c3

Please sign in to comment.