Support RequestedAuthnContext #31238

jkakavas · 2018-06-10T21:20:08Z

This implements limited support for RequestedAuthnContext by :

Allowing SP administrators to define a list of authnContextClassRef
to be included in the RequestedAuthnContext of a SAML Authn Request
Veirifying that the authnContext in the incoming SAML Asertion's
AuthnStatement contains one of the requested authnContextClassRef
Only EXACT comparison is supported as the semantics of validating
the incoming authnContextClassRef are deployment dependant and
require pre-established rules for MINIMUM, MAXIMUM and BETTER

Also adds necessary AuthnStatement validation according to the rules
indicated by [1] and [2]

Resolves #29995

[1] https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf
3.4.1.4, line 2250-2253
[2] https://kantarainitiative.github.io/SAMLprofiles/saml2int.html
[SDP-IDP10]

This implements limited support for RequestedAuthnContext by : - Allowing SP administrators to define a list of authnContextClassRef to be included in the RequestedAuthnContext of a SAML Authn Request - Veirifying that the authnContext in the incoming SAML Asertion's AuthnStatement contains one of the requested authnContextClassRef - Only EXACT comparison is supported as the semantics of validating the incoming authnContextClassRef are deployment dependant and require pre-established rules for MINIMUM, MAXIMUM and BETTER Also adds necessary AuthnStatement validation as indicated by [1] and [2] [1] https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf 3.4.1.4, line 2250-2253 [2] https://kantarainitiative.github.io/SAMLprofiles/saml2int.html [SDP-IDP10]

elasticmachine · 2018-06-10T21:20:10Z

Pinging @elastic/es-security

jkakavas · 2018-06-10T21:47:30Z

run sample packaging tests

jkakavas · 2018-06-11T06:33:01Z

@elasticmachine test this please

…text

tvernum · 2018-06-12T05:44:24Z

...n/core/src/main/java/org/elasticsearch/xpack/core/security/authc/saml/SamlRealmSettings.java

@@ -62,7 +62,8 @@
            Setting.simpleString("signing.keystore.alias", Setting.Property.NodeScope);
    public static final Setting<List<String>> SIGNING_MESSAGE_TYPES = Setting.listSetting("signing.saml_messages",
            Collections.singletonList("*"), Function.identity(), Setting.Property.NodeScope);
-
+    public static final Setting<List<String>> REQUESTED_AUTHN_CONTEXT_CLASS_REF = Setting.listSetting("sp.req_authn_context_class_ref",


I don't think we want the sp. prefix here.
My setting names haven't been entirely consistent, but the sp. settings are really "tell us about your Kibana instance", and the other settings like force_authn are separate.

tvernum · 2018-06-12T05:45:48Z

...in/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java

+    private void checkAuthnStatement(List<AuthnStatement> authnStatements) {
+        if (authnStatements.size() != 1) {
+            throw samlException("SAML Assertion subject contains {} Authn Statements while exactly one was expected.", authnStatements
+                .size());


Nit: can we wrap at the comma rather than the dot?

... expected.", authnStatements.size());

tvernum · 2018-06-12T05:47:21Z

...in/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java

+            throw samlException("SAML Assertion subject contains {} Authn Statements while exactly one was expected.", authnStatements
+                .size());
+        }
+        for (AuthnStatement authnStatement : authnStatements) {


Since the list must have exactly 1 element, it would be neater to have

final AuthnStatement authnStatement = authnStatement.get(0);

instead of a single-iteration loop

tvernum · 2018-06-12T05:51:06Z

...in/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java

+                    authnCtxClassRefValue = authnStatement.getAuthnContext().getAuthnContextClassRef().getAuthnContextClassRef();
+                }
+                if (Strings.isNullOrEmpty(authnCtxClassRefValue) || reqAuthnCtxClassRef.contains(authnCtxClassRefValue) == false) {
+                    throw samlException("Rejecting SAML assertion as the AuthnContextClassRef {} is not one of the ({}) that were " +


Wrap the first message parameter ({}) as [{}] so that any spaces (or the empty string) are clearly delimited.

tvernum · 2018-06-12T05:52:51Z

...urity/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthnRequestBuilder.java


    SamlAuthnRequestBuilder(SpConfiguration spConfig, String spBinding, EntityDescriptor idpDescriptor, String idBinding, Clock clock) {
        super(idpDescriptor, spConfig, clock);
        this.spBinding = spBinding;
        this.idpBinding = idBinding;
        this.nameIdSettings = new NameIDPolicySettings(NameID.TRANSIENT, false, null);
+        this.reqAuthnCtxClassRef = spConfig.getReqAuthnCtxClassRef();


You don't need to store this in a field, it's available as super.serviceProvider.getReqAuthnCtxClassRef()

jkakavas · 2018-06-12T06:26:24Z

Thanks for the feedback @tvernum , I believe I addressed all points

tvernum · 2018-06-12T06:32:57Z

...urity/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthnRequestBuilder.java

 import org.opensaml.saml.saml2.metadata.EntityDescriptor;
 import org.opensaml.saml.saml2.metadata.IDPSSODescriptor;

 import java.time.Clock;
+import java.util.List;


Nit: I think this import is unused now.

This implements limited support for RequestedAuthnContext by : - Allowing SP administrators to define a list of authnContextClassRef to be included in the RequestedAuthnContext of a SAML Authn Request - Veirifying that the authnContext in the incoming SAML Asertion's AuthnStatement contains one of the requested authnContextClassRef - Only EXACT comparison is supported as the semantics of validating the incoming authnContextClassRef are deployment dependant and require pre-established rules for MINIMUM, MAXIMUM and BETTER Also adds necessary AuthnStatement validation as indicated by [1] and [2] [1] https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf 3.4.1.4, line 2250-2253 [2] https://kantarainitiative.github.io/SAMLprofiles/saml2int.html [SDP-IDP10]

* master: Remove RestGetAllAliasesAction (#31308) Temporary fix for broken build Reenable Checkstyle's unused import rule (#31270) Remove remaining unused imports before merging #31270 Fix non-REST doc snippet [DOC] Extend SQL docs Immediately flush channel after writing to buffer (#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) CCS: don't proxy requests for already connected node (#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (#31298) [Rollup] Metric config parser must use builder so validation runs (#31159) [ML] Check licence when datafeeds use cross cluster search (#31247) Add notion of internal index settings (#31286) Test: Remove broken yml test feature (#31255) REST hl client: cluster health to default to cluster level (#31268) [ML] Update test thresholds to account for changes to memory control (#31289) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) Ignore numeric shard count if waiting for ALL (#31265) [ML] Implement new rules design (#31110) index_prefixes back-compat should test 6.3 (#30951) Core: Remove plain execute method on TransportAction (#30998) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Modify pipelining handlers to require full requests (#31280) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) Fix Netty 4 Server Transport tests. Again. REST hl client: adjust wait_for_active_shards param in cluster health (#31266) REST high-level Client: remove deprecated API methods (#31200) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) Delete typos in SAML docs (#31199) REST high-level client: add Cluster Health API (#29331) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) Remove some line length supressions (#31209) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) LLClient: Support host selection (#30523) Upgrade to Netty 4.1.25.Final (#31232) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) HLRest: Add get index templates API (#31161) Remove all unused imports and fix CRLF (#31207) [Tests] Fix self-referencing tests [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Limit the number of concurrent requests per node (#31206) Call ensureNoSelfReferences() on _agg state variable after scripted metric agg script executions (#31044) Move java version checker back to its own jar (#30708) [test] add fix for rare virtualbox error (#31212)

* 6.x: SQL: Fix build on Java 10 [Tests] Mutualize fixtures code in BaseHttpFixture (#31210) [TEST] Fix RemoteClusterClientTests#testEnsureWeReconnect [ML] Update test thresholds to account for changes to memory control (#31289) Reenable Checkstyle's unused import rule (#31270) [ML] Check licence when datafeeds use cross cluster search (#31247) Fix non-REST doc snippet [DOC] Extend SQL docs [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) SQL: Whitelist SQL utility class for better scripting (#30681) Add 5.6.11 version constant. Fix version detection. [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) Add missing release notes. Security: fix token bwc with pre 6.0.0-beta2 (#31254) Fix compilation error in UpdateSettingsIT (#31304) Test: Remove broken yml test feature (#31255) Add unreleased version 6.3.1 [Rollup] Metric config parser must use builder so validation runs (#31159) Removes experimental tag from scripted_metric aggregation (#31298) [DOCS] Removes coming tag from 6.3.0 release notes 6.3 release notes. Add notion of internal index settings (#31286) REST high-level client: add Cluster Health API (#29331) Remove leftover usage of deprecated client API SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) HLRest: Add get index templates API (#31161) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) [TEST] Mute RecoveryIT.testHistoryUUIDIsGenerated Ignore numeric shard count if waiting for ALL (#31265) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160) Fix Netty 4 Server Transport tests. Again. [DOCS] Fixed typo. [DOCS] Added release highlights for 6.3 (#31256) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Upgrade to Netty 4.1.25.Final (#31232) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) [DOCS] Adds machine learning 6.3.0 release notes (#31217) Remove all unused imports and fix CRLF (#31207) [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Compliant SAML Response destination check (#31175) Move java version checker back to its own jar (#30708) TEST: Retry synced-flush if ongoing ops on primary (#30978) [test] add fix for rare virtualbox error (#31212)

Docs for elastic#31238 - Adds documentation for the req_authn_context_class_ref setting - Adds a section in SAML Guide regarding the use of SAML Authentication Context.

Add documentation for #31238 - Add documentation for the req_authn_context_class_ref setting - Add a section in SAML Guide regarding the use of SAML Authentication Context.

jkakavas added >enhancement review v7.0.0 :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v6.3.1 labels Jun 10, 2018

jkakavas requested a review from tvernum June 10, 2018 21:20

Merge remote-tracking branch 'origin/master' into requested-authn-con…

a47fe0e

…text

tvernum reviewed Jun 12, 2018

View reviewed changes

addresses feedback

4d2eafc

tvernum approved these changes Jun 12, 2018

View reviewed changes

Remove unused import

ec3e785

jkakavas added the v6.4.0 label Jun 12, 2018

jkakavas merged commit b2e48c9 into elastic:master Jun 12, 2018

jkakavas added backport pending and removed review v6.3.1 backport pending labels Jun 12, 2018

jkakavas mentioned this pull request Aug 17, 2018

[DOCS] Add RequestedAuthnContext Documentation #32946

Merged

jkakavas deleted the requested-authn-context branch September 14, 2018 06:56

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support RequestedAuthnContext #31238

Support RequestedAuthnContext #31238

jkakavas commented Jun 10, 2018

elasticmachine commented Jun 10, 2018

jkakavas commented Jun 10, 2018 •

edited

Loading

jkakavas commented Jun 11, 2018

tvernum Jun 12, 2018

tvernum Jun 12, 2018

tvernum Jun 12, 2018

tvernum Jun 12, 2018

tvernum Jun 12, 2018

jkakavas commented Jun 12, 2018

tvernum Jun 12, 2018

Support RequestedAuthnContext #31238

Support RequestedAuthnContext #31238

Conversation

jkakavas commented Jun 10, 2018

elasticmachine commented Jun 10, 2018

jkakavas commented Jun 10, 2018 • edited Loading

jkakavas commented Jun 11, 2018

tvernum Jun 12, 2018

Choose a reason for hiding this comment

tvernum Jun 12, 2018

Choose a reason for hiding this comment

tvernum Jun 12, 2018

Choose a reason for hiding this comment

tvernum Jun 12, 2018

Choose a reason for hiding this comment

tvernum Jun 12, 2018

Choose a reason for hiding this comment

jkakavas commented Jun 12, 2018

tvernum Jun 12, 2018

Choose a reason for hiding this comment

jkakavas commented Jun 10, 2018 •

edited

Loading