Bug 5710 - add raw SAML assertion to security context
: add raw SAML assertion to security context
Status: RESOLVED FIXED
: GridShib
SAML/Binding Tools
: 0.3
: All All
: P3 enhancement
: beta
Assigned To:
:
:
:
: 5748
  Show dependency treegraph
 
Reported: 2007-12-07 17:05 by
Modified: 2008-04-25 21:12 (History)


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2007-12-07 17:05:31
Add all raw, unparsed SAML assertions to the SAML security context.  Even if
the SAML content does not survive attribute acceptance policy, leave the raw
SAML assertions in the security context.  Thus the SAML assertions will always
be logged regardless of policy.
------- Comment #1 From 2007-12-12 10:18:21 -------
Isn't it simply a matter of not including the AttributeAcceptancePIP in the
chain?
------- Comment #2 From 2007-12-12 12:19:17 -------
For discussion, suppose we had the following authz chain:

<authz value="myscope:org.globus.gridshib.SAMLAssertionPushPIP 
              myscope:org.globus.gridshib.AttributeAcceptancePIP"/>

Currently, the SAMLAssertionPushPIP parses the SAML assertion and adds *all*
SAML content to the security context.  The PIP makes no distinction between
trusted or untrusted attributes, that is, it does not consider the overall
trustworthiness of the issuer or the specific claims asserted by that issuer.

Subsequently, the AttributeAcceptancePIP makes a pass over the security context
and removes all security information that was not obtained from a trusted
issuer.  If you remove the AttributeAcceptancePIP from the chain, both trusted
and untrusted security information will be found in the security context, and
hence both trusted and untrusted attributes will be logged.  I don't think it's
a good idea to log attributes without distinguishing between trusted attributes
and untrusted attributes.

In any event, we did not architect for arbitrary authz chains (such as the one
above).  We decided early on that there would be just one authz chain:

<authz value="myscope:org.globus.gridshib.GridShibPDP"/>

The GridShibPDP is a superset of the following chain:

<authz value="myscope:org.globus.gridshib.SAMLAssertionPushPIP 
              myscope:org.globus.gridshib.AttributeAcceptancePIP 
              myscope:org.globus.gridshib.SAMLBlacklistPDP 
              myscope:org.globus.gridshib.SAMLMapPIP 
              myscope:org.globus.gridshib.SAMLAttributePDP"/>

If we're going to support other authz chains, we need to reconsider the
implementation of the SAML security context so that it's not possible to base
an authz decision on untrusted security information.  That includes logging of
untrusted attributes.
------- Comment #3 From 2007-12-12 14:31:06 -------
In the TG science gateways use case
(http://www.globus.org/mail_archive/gridshib-dev/2007/12/msg00002.html), we
trust attributes in certificates whose subjects are in the grid-mapfile.  Your
notion of trust as being solely determined by the AttributeAcceptancePIP seems
unnecessarily restrictive, in that it requires us to duplicate the trust
information we already have in our grid-mapfile in a second
AttributeAcceptancePIP configuration.
------- Comment #4 From 2007-12-13 07:49:19 -------
(In reply to comment #3)
> In the TG science gateways use case
> (http://www.globus.org/mail_archive/gridshib-dev/2007/12/msg00002.html), we
> trust attributes in certificates whose subjects are in the grid-mapfile. 

Attributes in certificates should not be trusted just because the issuer's DN
is in the gridmap.  I don't trust trscavo to assert groups or roles, but I do
trust trscavo to assert an e-mail address or to provide an identifier
indicating his IdP (for the purposes of attribute query).  Moreover, I don't
trust trscavo to assert an AuthenticationStatement at all, which rules out IP
address and AuthenticationInstant, two attributes required by TG.  Only Science
Gateways are trusted to assert an AuthenticationStatement.

> Your
> notion of trust as being solely determined by the AttributeAcceptancePIP seems
> unnecessarily restrictive, in that it requires us to duplicate the trust
> information we already have in our grid-mapfile in a second
> AttributeAcceptancePIP configuration.

The gridmap was never envisioned as a list of trusted SAML issuers.  The
gridmap is simply a mapping from DN to local user name.  It provides little or
no basis on which to build an attribute-based trust model.

There are three types of X.509-bound SAML assertions:

1. Self-issued assertions where the presenter is the subject
2. Self-issued assertions where the presenter is acting on behalf of the
subject
3. Third-party assertions

Each type of assertion is validated differently (e.g., third-party assertions
must be signed).  Policy is also based on assertion type (see note re
AuthenticationStatement above).  The fact that the presenter is in the gridmap
does not help to determine the type of assertion.  

Currently, the software only accepts self-issued assertions from trusted
entities acting on behalf of the subject.  (The Science Gateway fits into this
category.)  The other two types of assertions are ignored.  Also, attribute
acceptance policy is currently implemented as a boolean, that is, either an
entity is trusted (to assert any and all attributes) or it is not.  That is why
the software restricts the assertion type, because the other two types require
more complicated machinery.
------- Comment #5 From 2007-12-13 12:12:26 -------
If, as you say, TG needs to set an attribute acceptance policy in any case,
then I don't think this bug is something that TG needs.  I don't see how you
can say at
<http://www.globus.org/mail_archive/gridshib-dev/2007/12/msg00005.html> that
this meets the requirement that "no policy file should be required besides the
gridmap."
------- Comment #6 From 2007-12-13 13:02:16 -------
(In reply to comment #5)
> If, as you say, TG needs to set an attribute acceptance policy in any case,
> then I don't think this bug is something that TG needs. 

If a science gateway is NOT on the trusted SAML issuer list, the parsed SAML
will be purged from the security context by the AttributeAcceptancePIP, but the
raw, unparsed SAML assertion will remain and therefore appear in the logs.  If
all you require is to read the SAML in the logs (whether or not it's trusted),
then this should do the trick.  (Btw, this item has been on my TODO list for
quite some time since I need the raw, unparsed SAML assertion in the security
context in any event.)

> I don't see how you can say at
> <http://www.globus.org/mail_archive/gridshib-dev/2007/12/msg00005.html> that
> this meets the requirement that "no policy file should be required besides the
> gridmap."

This is your requirement, not mine :-)  I've been trying to convince you that
this is an unreasonable requirement.  In the very least, it's a new requirement
that was not taken into account when GS4GT was designed and implemented.  I'm
groping for a short-term workaround that satisfies your needs.

In the end, the Grid SP (GS4GT) requires SAML metadata for all SAML issuers
except the first assertion type in Comment #4.  (At that point, the flat
entity-mapping file goes away.)  That's a significant requirement, but I don't
see any way around it.  
------- Comment #7 From 2008-01-03 18:30:27 -------
Change summary:

- removed SAMLUtil.processAssertion
- removed SAMLUtil.processSubject
- added method SAMLSecurityContext.parseSAMLAssertion(SAMLSubjectAssertion)
- implemented trivial class ParsedSAMLAssertion
- added method SAMLSecurityContext.getParsedSAMLAssertions()
- in SAMLSecurityContext, changed visibility of other "add" methods from public
to protected
- added parsed SAML assertions to toString method (and therefore, logged
output)
- added 'trusted' member (with initial value false) to the interface
- added "(untrusted)" prefix to toString method
- removed rawAttributes from SAMLSecurityContext
------- Comment #8 From 2008-01-03 18:43:58 -------
A parsed SAML assertion is a trusted assertion, that is, an assertion from a
known issuer.  Typically, if the relying party possesses trusted metadata for
the issuer, the assertion is trusted by definition.

Although the assertion is trusted, the security information in the assertion
may or may not be trusted.  Hence, the new 'trusted' member, which is
initialized to false in all cases.  Thus, it is up to the implementation to
determine the trustworthiness of individual pieces of security information in
the assertion.

Since the other "add" methods are now protected, this forces all security
information to go through method parseSAMLAssertion before being added to the
security context.  This is for consistency, of course.

A parsed SAML assertion can not be "unparsed," that is, a parsed assertion can
not be removed from the security context.  Thus the raw assertion will appear
in the logs even if every piece of security information is deemed untrustworthy
(and therefore not logged).
------- Comment #9 From 2008-01-03 18:45:19 -------
These changes will appear in GridShib SAML Tools v0.3.0.