https://issues.shibboleth.net/jira/browse/SSPCPP-380

diff --git a/adfs/adfs.cpp b/adfs/adfs.cpp
index 5d89ecb..c49ec99 100644 (file)
--- a/adfs/adfs.cpp
+++ b/adfs/adfs.cpp
@@ -658,9 +658,18 @@ void ADFSConsumer::implementProtocol(
         pair<bool,unsigned int> authnskew = sessionProps ? sessionProps->getUnsignedInt("maxTimeSinceAuthn") : pair<bool,unsigned int>(false,0);
 
         const saml1::AuthenticationStatement* ssoStatement=saml1token->getAuthenticationStatements().front();
-        if (authnskew.first && authnskew.second &&
-                ssoStatement->getAuthenticationInstant() && (now - ssoStatement->getAuthenticationInstantEpoch() > authnskew.second))
-            throw FatalProfileException("The gap between now and the time you logged into your identity provider exceeds the limit.");
+        if (ssoStatement->getAuthenticationInstant()) {
+            if (ssoStatement->getAuthenticationInstantEpoch() - XMLToolingConfig::getConfig().clock_skew_secs > now) {
+                throw FatalProfileException("The login time at your identity provider was future-dated.");
+            }
+            else if (authnskew.first && authnskew.second && ssoStatement->getAuthenticationInstantEpoch() <= now &&
+                    (now - ssoStatement->getAuthenticationInstantEpoch() > authnskew.second)) {
+                throw FatalProfileException("The gap between now and the time you logged into your identity provider exceeds the allowed limit.");
+            }
+        }
+        else if (authnskew.first && authnskew.second) {
+            throw FatalProfileException("Your identity provider did not supply a time of login, violating local policy.");
+        }
 
         // Address checking.
         saml1::SubjectLocality* locality = ssoStatement->getSubjectLocality();

diff --git a/shibsp/handler/impl/SAML1Consumer.cpp b/shibsp/handler/impl/SAML1Consumer.cpp
index 0a82306..46ef55c 100644 (file)
--- a/shibsp/handler/impl/SAML1Consumer.cpp
+++ b/shibsp/handler/impl/SAML1Consumer.cpp
@@ -39,6 +39,7 @@
 # include <saml/saml1/core/Assertions.h>
 # include <saml/saml1/core/Protocols.h>
 # include <saml/saml2/metadata/Metadata.h>
+# include <xmltooling/XMLToolingConfig.h>
 # include <xmltooling/io/HTTPRequest.h>
 # include <xmltooling/util/DateTime.h>
 using namespace opensaml::saml1;
@@ -211,7 +212,8 @@ void SAML1Consumer::implementProtocol(
 
             // Extract message bits and re-verify Issuer information.
             extractMessageDetails(
-                *(*a), (minor.first && minor.second==0) ? samlconstants::SAML10_PROTOCOL_ENUM : samlconstants::SAML11_PROTOCOL_ENUM, policy
+                *(*a),
+                (minor.first && minor.second==0) ? samlconstants::SAML10_PROTOCOL_ENUM : samlconstants::SAML11_PROTOCOL_ENUM, policy
                 );
 
             // Run the policy over the assertion. Handles replay, freshness, and
@@ -227,11 +229,20 @@ void SAML1Consumer::implementProtocol(
             tokens.push_back(*a);
 
             // Save off the first valid SSO statement.
-            const vector<AuthenticationStatement*>& statements = const_cast<const saml1::Assertion*>(*a)->getAuthenticationStatements();
+            const vector<AuthenticationStatement*>& statements =
+                    const_cast<const saml1::Assertion*>(*a)->getAuthenticationStatements();
             for (vector<AuthenticationStatement*>::const_iterator s = statements.begin(); s!=statements.end(); ++s) {
-                if (authnskew.first && authnskew.second &&
-                    (*s)->getAuthenticationInstant() && (now - (*s)->getAuthenticationInstantEpoch() > authnskew.second))
-                    contextualError = "The gap between now and the time you logged into your identity provider exceeds the limit.";
+                if ((*s)->getAuthenticationInstant() &&
+                        (*s)->getAuthenticationInstantEpoch() - XMLToolingConfig::getConfig().clock_skew_secs > now) {
+                    contextualError = "The login time at your identity provider was future-dated.";
+                }
+                else if (authnskew.first && authnskew.second && (*s)->getAuthenticationInstant() &&
+                        (*s)->getAuthenticationInstantEpoch() <= now && (now - (*s)->getAuthenticationInstantEpoch() > authnskew.second)) {
+                    contextualError = "The gap between now and the time you logged into your identity provider exceeds the allowed limit.";
+                }
+                else if (authnskew.first && authnskew.second && (*s)->getAuthenticationInstant() == nullptr) {
+                    contextualError = "Your identity provider did not supply a time of login, violating local policy.";
+                }
                 else if (!ssoStatement) {
                     ssoStatement = *s;
                     break;

diff --git a/shibsp/handler/impl/SAML2Consumer.cpp b/shibsp/handler/impl/SAML2Consumer.cpp
index 9e939dc..6af77e7 100644 (file)
--- a/shibsp/handler/impl/SAML2Consumer.cpp
+++ b/shibsp/handler/impl/SAML2Consumer.cpp
@@ -238,10 +238,19 @@ void SAML2Consumer::implementProtocol(
             // Save off the first valid SSO statement, but favor the "soonest" session expiration.
             const vector<AuthnStatement*>& statements = const_cast<const saml2::Assertion*>(*a)->getAuthnStatements();
             for (vector<AuthnStatement*>::const_iterator s = statements.begin(); s!=statements.end(); ++s) {
-                if (authnskew.first && authnskew.second && (*s)->getAuthnInstant() && (now - (*s)->getAuthnInstantEpoch() > authnskew.second))
-                    contextualError = "The gap between now and the time you logged into your identity provider exceeds the limit.";
-                else if (!ssoStatement || (*s)->getSessionNotOnOrAfterEpoch() < ssoStatement->getSessionNotOnOrAfterEpoch())
+                if ((*s)->getAuthnInstant() && (*s)->getAuthnInstantEpoch() - XMLToolingConfig::getConfig().clock_skew_secs > now) {
+                    contextualError = "The login time at your identity provider was future-dated.";
+                }
+                else if (authnskew.first && authnskew.second && (*s)->getAuthnInstant() &&
+                        (*s)->getAuthnInstantEpoch() <= now && (now - (*s)->getAuthnInstantEpoch() > authnskew.second)) {
+                    contextualError = "The gap between now and the time you logged into your identity provider exceeds the allowed limit.";
+                }
+                else if (authnskew.first && authnskew.second && (*s)->getAuthnInstant() == nullptr) {
+                    contextualError = "Your identity provider did not supply a time of login, violating local policy.";
+                }
+                else if (!ssoStatement || (*s)->getSessionNotOnOrAfterEpoch() < ssoStatement->getSessionNotOnOrAfterEpoch()) {
                     ssoStatement = *s;
+                }
             }
 
             // Save off the first valid Subject, but favor an unencrypted NameID over anything else.