Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump SAML libs #2927

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Bump SAML libs #2927

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ buildscript {
common_utils_version = System.getProperty("common_utils.version", '3.0.0.0-SNAPSHOT')
kafka_version = '3.5.0'
apache_cxf_version = '4.0.2'
open_saml_version = '4.3.0'
one_login_java_saml = '2.9.0'

if (buildVersionQualifier) {
opensearch_build += "-${buildVersionQualifier}"
Expand All @@ -42,6 +44,8 @@ buildscript {
maven { url "https://plugins.gradle.org/m2/" }
maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" }
maven { url "https://d1nvenhzbhpy0q.cloudfront.net/snapshots/lucene/" }
maven { url "https://build.shibboleth.net/nexus/content/groups/public" }
maven { url "https://build.shibboleth.net/nexus/content/repositories/releases" }
}

dependencies {
Expand Down Expand Up @@ -373,6 +377,7 @@ repositories {
maven { url "https://plugins.gradle.org/m2/" }
maven { url "https://aws.oss.sonatype.org/content/repositories/snapshots" }
maven { url "https://d1nvenhzbhpy0q.cloudfront.net/snapshots/lucene/" }
maven { url "https://build.shibboleth.net/nexus/content/repositories/releases" }
}

tasks.withType(Checkstyle) {
Expand Down Expand Up @@ -486,8 +491,6 @@ dependencies {
implementation 'com.github.wnameless:json-flattener:0.5.0'
implementation 'com.flipkart.zjsonpatch:zjsonpatch:0.4.4'
implementation "org.apache.kafka:kafka-clients:${kafka_version}"
implementation 'com.onelogin:java-saml:2.5.0'
implementation 'com.onelogin:java-saml-core:2.5.0'

runtimeOnly 'net.minidev:accessors-smart:2.4.7'

Expand All @@ -509,23 +512,26 @@ dependencies {

testImplementation 'org.apache.camel:camel-xmlsecurity:3.14.2'

implementation 'net.shibboleth.utilities:java-support:7.5.1'
implementation 'org.opensaml:opensaml-core:3.4.5'
implementation 'org.opensaml:opensaml-security-impl:3.4.5'
implementation 'org.opensaml:opensaml-security-api:3.4.5'
implementation 'org.opensaml:opensaml-xmlsec-api:3.4.5'
implementation 'org.opensaml:opensaml-xmlsec-impl:3.4.5'
implementation 'org.opensaml:opensaml-saml-api:3.4.5'
implementation ('org.opensaml:opensaml-saml-impl:3.4.5') {
//OpenSAML
implementation 'net.shibboleth.utilities:java-support:8.4.0'
implementation "com.onelogin:java-saml:${one_login_java_saml}"
implementation "com.onelogin:java-saml-core:${one_login_java_saml}"
implementation "org.opensaml:opensaml-core:${open_saml_version}"
implementation "org.opensaml:opensaml-security-impl:${open_saml_version}"
implementation "org.opensaml:opensaml-security-api:${open_saml_version}"
implementation "org.opensaml:opensaml-xmlsec-api:${open_saml_version}"
implementation "org.opensaml:opensaml-xmlsec-impl:${open_saml_version}"
implementation "org.opensaml:opensaml-saml-api:${open_saml_version}"
implementation ("org.opensaml:opensaml-saml-impl:${open_saml_version}") {
exclude(group: 'org.apache.velocity', module: 'velocity')
}
implementation "org.opensaml:opensaml-messaging-api:${open_saml_version}"
runtimeOnly "org.opensaml:opensaml-profile-api:${open_saml_version}"
runtimeOnly "org.opensaml:opensaml-soap-api:${open_saml_version}"
runtimeOnly "org.opensaml:opensaml-soap-impl:${open_saml_version}"
implementation "org.opensaml:opensaml-storage-api:${open_saml_version}"

implementation "com.nulab-inc:zxcvbn:1.7.0"
testImplementation 'org.opensaml:opensaml-messaging-impl:3.4.5'
implementation 'org.opensaml:opensaml-messaging-api:3.4.5'
runtimeOnly 'org.opensaml:opensaml-profile-api:3.4.5'
runtimeOnly 'org.opensaml:opensaml-soap-api:3.4.5'
runtimeOnly 'org.opensaml:opensaml-soap-impl:3.4.5'
implementation 'org.opensaml:opensaml-storage-api:3.4.5'
implementation 'commons-collections:commons-collections:3.2.2'
implementation 'com.jayway.jsonpath:json-path:2.4.0'
implementation 'net.minidev:json-smart:2.4.10'
Expand Down Expand Up @@ -553,6 +559,7 @@ dependencies {
runtimeOnly 'org.scala-lang.modules:scala-java8-compat_3:1.0.2'


testImplementation "org.opensaml:opensaml-messaging-impl:${open_saml_version}"
implementation 'org.apache.commons:commons-lang3:3.12.0'
testImplementation "org.opensearch:common-utils:${common_utils_version}"
testImplementation "org.opensearch.plugin:reindex-client:${opensearch_version}"
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,7 @@ private AuthTokenProcessorAction.Response handleImpl(

try {

SamlResponse samlResponse = new SamlResponse(saml2Settings, null);
samlResponse.setDestinationUrl(acsEndpoint);
samlResponse.loadXmlFromBase64(samlResponseBase64);
final SamlResponse samlResponse = new SamlResponse(saml2Settings, acsEndpoint, samlResponseBase64);

if (!samlResponse.isValid(samlRequestId)) {
log.warn("Error while validating SAML response in /_opendistro/_security/api/authtoken");
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/amazon/dlic/auth/http/saml/Saml2SettingsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.security.AccessController;
import java.security.PrivateKey;
import java.security.PrivilegedAction;
import java.time.Instant;
import java.util.AbstractMap;
import java.util.Collection;
import java.util.HashMap;
Expand All @@ -28,7 +29,6 @@
import net.shibboleth.utilities.java.support.resolver.ResolverException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.joda.time.DateTime;
import org.opensaml.core.criterion.EntityIdCriterion;
import org.opensaml.saml.metadata.resolver.MetadataResolver;
import org.opensaml.saml.metadata.resolver.RefreshableMetadataResolver;
Expand All @@ -54,7 +54,7 @@ public class Saml2SettingsProvider {
private final String idpEntityId;
private final PrivateKey spSignaturePrivateKey;
private Saml2Settings cachedSaml2Settings;
private DateTime metadataUpdateTime;
private Instant metadataUpdateTime;

Saml2SettingsProvider(Settings opensearchSettings, MetadataResolver metadataResolver, PrivateKey spSignaturePrivateKey) {
this.opensearchSettings = opensearchSettings;
Expand Down Expand Up @@ -107,7 +107,7 @@ Saml2Settings get() throws SamlConfigException {
}

Saml2Settings getCached() throws SamlConfigException {
DateTime tempLastUpdate = null;
Instant tempLastUpdate = null;

if (this.metadataResolver instanceof RefreshableMetadataResolver && this.isUpdateRequired()) {
this.cachedSaml2Settings = null;
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/com/amazon/dlic/auth/http/saml/SamlHTTPMetadataResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.security.AccessController;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back when I migrated the main branch from Apache HttpClient 4 -> Apache HttpClient 5, I needed to retain the dependency on HttpClient 4 because of this class. Is there any version of OpenSAML available with Apache HttpClient 5 so that we can remove this last remaining dependeny on HttpClient 4?

This PR looks good to me.

Copy link
Collaborator Author

@willyborankin willyborankin Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh I will take a look for sure. Beside found out that Shibboleth has its own repository for OpenSAML and the latest version is 4.3.0. I will push changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So 4.3.0 uses HTTP commons version 4. to exclude it completely I could try to prepare PR to the Shibboleth git repo (not sure how it works) or just implement our own soultion with HTTP commons 5 instead of 4. AFAU we need to extend org.opensaml.saml.metadata.resolver.impl.AbstractReloadingMetadataResolver class for that. Wdyt?

Copy link
Collaborator Author

@willyborankin willyborankin Jul 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed 4.3.0 version

Copy link
Collaborator Author

@willyborankin willyborankin Jul 4, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwperks added issue #2932

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for filing an issue to track this @willyborankin! From the Shibboleth git, it looks like the HttpMetadataResolver class is using HttpClient 5 on their main branch: https://git.shibboleth.net/view/?p=java-opensaml.git;a=blob;f=opensaml-saml-impl/src/main/java/org/opensaml/saml/metadata/resolver/impl/HTTPMetadataResolver.java;hb=0d8f395fcca7923cb4d4cc5a98730b5f3fca3aa9

I'm looking to see if there is a release schedule to see when its expected that a published version of the jar will have the update.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4.3.0 is the way to go. 4.0.1 was published February 11, 2021 and 4.3.0 on January 17, 2023. It looks like they are only publishing new artifacts to the shibboleth repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwperks OK found https://shibboleth.atlassian.net/browse/JSSH-16 they are going to release it in version 9.0.0 of net.shibboleth.utilities:java-support. AFAIU this is part of IDP v5 release

import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.time.Duration;

import net.shibboleth.utilities.java.support.resolver.ResolverException;
import org.apache.http.client.HttpClient;
Expand All @@ -31,8 +32,8 @@ public class SamlHTTPMetadataResolver extends HTTPMetadataResolver {

SamlHTTPMetadataResolver(String idpMetadataUrl, Settings opensearchSettings, Path configPath) throws Exception {
super(createHttpClient(opensearchSettings, configPath), idpMetadataUrl);
setMinRefreshDelay(opensearchSettings.getAsLong("idp.min_refresh_delay", 60L * 1000L));
setMaxRefreshDelay(opensearchSettings.getAsLong("idp.max_refresh_delay", 14400000L));
setMinRefreshDelay(Duration.ofMillis(opensearchSettings.getAsLong("idp.min_refresh_delay", 60L * 1000L)));
setMaxRefreshDelay(Duration.ofMillis(opensearchSettings.getAsLong("idp.max_refresh_delay", 14400000L)));
setRefreshDelayFactor(opensearchSettings.getAsFloat("idp.refresh_delay_factor", 0.75f));
}

Expand Down
41 changes: 21 additions & 20 deletions src/test/java/com/amazon/dlic/auth/http/saml/MockSamlIdpServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.net.URISyntaxException;
import java.nio.charset.CharsetDecoder;
import java.nio.charset.CharsetEncoder;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.security.KeyStore;
import java.security.KeyStoreException;
Expand All @@ -32,6 +33,8 @@
import java.security.UnrecoverableKeyException;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Arrays;
import java.util.Collections;
import java.util.Enumeration;
Expand Down Expand Up @@ -60,6 +63,7 @@
import javax.xml.transform.stream.StreamResult;

import net.shibboleth.utilities.java.support.codec.Base64Support;
import net.shibboleth.utilities.java.support.codec.EncodingException;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
import org.apache.hc.core5.function.Callback;
import org.apache.hc.core5.http.ClassicHttpRequest;
Expand All @@ -82,7 +86,6 @@
import org.apache.hc.core5.http.message.BasicHttpRequest;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.net.URIBuilder;
import org.joda.time.DateTime;
import org.opensaml.core.xml.XMLObject;
import org.opensaml.core.xml.XMLObjectBuilderFactory;
import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport;
Expand All @@ -92,7 +95,6 @@
import org.opensaml.messaging.context.MessageContext;
import org.opensaml.messaging.decoder.MessageDecodingException;
import org.opensaml.messaging.handler.MessageHandlerException;
import org.opensaml.saml.common.SAMLObject;
import org.opensaml.saml.common.SAMLVersion;
import org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext;
import org.opensaml.saml.common.messaging.context.SAMLProtocolContext;
Expand Down Expand Up @@ -331,11 +333,11 @@ public String handleSsoGetRequestBase(HttpRequest request) {

HTTPRedirectDeflateDecoder decoder = new HTTPRedirectDeflateDecoder();
decoder.setParserPool(XMLObjectProviderRegistrySupport.getParserPool());
decoder.setHttpServletRequest(httpServletRequest);
decoder.setHttpServletRequestSupplier(() -> httpServletRequest);
decoder.initialize();
decoder.decode();

MessageContext<SAMLObject> messageContext = decoder.getMessageContext();
MessageContext messageContext = decoder.getMessageContext();

if (!(messageContext.getMessage() instanceof AuthnRequest)) {
throw new RuntimeException("Expected AuthnRequest; received: " + messageContext.getMessage());
Expand All @@ -357,19 +359,18 @@ public void handleSloGetRequestURI(String samlRequestURI) {
handleSloGetRequestBase(new BasicHttpRequest("GET", samlRequestURI));
}

@SuppressWarnings("unchecked")
public void handleSloGetRequestBase(HttpRequest request) {
try {

HttpServletRequest httpServletRequest = new FakeHttpServletRequest(request);

HTTPRedirectDeflateDecoder decoder = new HTTPRedirectDeflateDecoder();
decoder.setParserPool(XMLObjectProviderRegistrySupport.getParserPool());
decoder.setHttpServletRequest(httpServletRequest);
decoder.setHttpServletRequestSupplier(() -> httpServletRequest);
decoder.initialize();
decoder.decode();

MessageContext<SAMLObject> messageContext = decoder.getMessageContext();
MessageContext messageContext = decoder.getMessageContext();

if (!(messageContext.getMessage() instanceof LogoutRequest)) {
throw new RuntimeException("Expected LogoutRequest; received: " + messageContext.getMessage());
Expand All @@ -391,7 +392,7 @@ public void handleSloGetRequestBase(HttpRequest request) {

validationParams.setSignatureTrustEngine(buildSignatureTrustEngine(this.spSignatureCertificate));
securityParametersContext.setSignatureValidationParameters(validationParams);
signatureSecurityHandler.setHttpServletRequest(httpServletRequest);
signatureSecurityHandler.setHttpServletRequestSupplier(() -> httpServletRequest);
signatureSecurityHandler.initialize();
signatureSecurityHandler.invoke(messageContext);

Expand All @@ -415,18 +416,18 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {

response.setVersion(SAMLVersion.VERSION_20);
response.setStatus(createStatus(StatusCode.SUCCESS));
response.setIssueInstant(new DateTime());
response.setIssueInstant(Instant.now());

Assertion assertion = createSamlElement(Assertion.class);

assertion.setID(nextId());
assertion.setIssueInstant(new DateTime());
assertion.setIssueInstant(Instant.now());
assertion.setIssuer(createIssuer());

AuthnStatement authnStatement = createSamlElement(AuthnStatement.class);
assertion.getAuthnStatements().add(authnStatement);

authnStatement.setAuthnInstant(new DateTime());
authnStatement.setAuthnInstant(Instant.now());
authnStatement.setSessionIndex(nextId());
authnStatement.setAuthnContext(createAuthnCotext());

Expand All @@ -440,7 +441,7 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {
.add(
createSubjectConfirmation(
"urn:oasis:names:tc:SAML:2.0:cm:bearer",
new DateTime().plusMinutes(1),
Instant.now().plus(1, ChronoUnit.MINUTES),
authnRequest.getID(),
authnRequest.getAssertionConsumerServiceURL()
)
Expand All @@ -450,7 +451,7 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {
.add(
createSubjectConfirmation(
"urn:oasis:names:tc:SAML:2.0:cm:bearer",
new DateTime().plusMinutes(1),
Instant.now().plus(1, ChronoUnit.MINUTES),
null,
defaultAssertionConsumerService
)
Expand All @@ -460,8 +461,8 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {
Conditions conditions = createSamlElement(Conditions.class);
assertion.setConditions(conditions);

conditions.setNotBefore(new DateTime());
conditions.setNotOnOrAfter(new DateTime().plusMinutes(1));
conditions.setNotBefore(Instant.now());
conditions.setNotOnOrAfter(Instant.now().plus(1, ChronoUnit.MINUTES));

if (authenticateUserRoles != null) {
AttributeStatement attributeStatement = createSamlElement(AttributeStatement.class);
Expand Down Expand Up @@ -501,9 +502,9 @@ private String createSamlAuthResponse(AuthnRequest authnRequest) {

String marshalledXml = marshallSamlXml(response);

return Base64Support.encode(marshalledXml.getBytes("UTF-8"), Base64Support.UNCHUNKED);
return Base64Support.encode(marshalledXml.getBytes(StandardCharsets.UTF_8), Base64Support.UNCHUNKED);

} catch (MarshallingException | SignatureException | UnsupportedEncodingException | EncryptionException e) {
} catch (MarshallingException | SignatureException | EncryptionException | EncodingException e) {
throw new RuntimeException(e);
}
}
Expand Down Expand Up @@ -547,7 +548,7 @@ private NameIDFormat createNameIDFormat(String format) {

NameIDFormat nameIdFormat = createSamlElement(NameIDFormat.class);

nameIdFormat.setFormat("urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified");
nameIdFormat.setURI("urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified");

return nameIdFormat;
}
Expand All @@ -567,7 +568,7 @@ private NameID createNameID(String format, String value) {
return nameID;
}

private SubjectConfirmation createSubjectConfirmation(String method, DateTime notOnOrAfter, String inResponseTo, String recipient) {
private SubjectConfirmation createSubjectConfirmation(String method, Instant notOnOrAfter, String inResponseTo, String recipient) {
SubjectConfirmation result = createSamlElement(SubjectConfirmation.class);
result.setMethod(method);

Expand All @@ -591,7 +592,7 @@ private Issuer createIssuer() {
private AuthnContext createAuthnCotext() {
AuthnContext authnContext = createSamlElement(AuthnContext.class);
AuthnContextClassRef authnContextClassRef = createSamlElement(AuthnContextClassRef.class);
authnContextClassRef.setAuthnContextClassRef(AuthnContext.UNSPECIFIED_AUTHN_CTX);
authnContextClassRef.setURI(AuthnContext.UNSPECIFIED_AUTHN_CTX);
authnContext.setAuthnContextClassRef(authnContextClassRef);
return authnContext;
}
Expand Down