[security-services] Comments on core-21

["Eve L. Maler" <eve.maler@sun.com>]

Following are substantive comments on core-21.  I suspect there are many 
other things that I blazed past, but I'm running short on time...  The line 
numbers match both the .doc and the .pdf version.  I'm hoping we can 
discuss at least some of these today, so that I can attempt to incorporate 
changes before I leave on vacation (December 13-18).

	Eve

---------------------------------
Examples:
Section 2.3, line 218; Section 2.4, line 443; Section 3.3, line 780; 
Section 3.4, line 839; and Section 3.5, line 948:

This spec desperately needs examples.  It would be nice if we could put a 
juicy example in each top-level section; they currently don't have much 
introductory text in them anyway.

---------------------------------
Namespaces:
Section 1.2, line 137:

I think we need to decide on final namespaces for these things.  E.g., the 
internal TC version number shouldn't appear in there.  (And I think we 
agreed at F2F #4 that the version number wouldn't either -- correct?)  I'm 
not thrilled with using "real file names" for namespace URIs, but I suppose 
I've lost this battle in the schema-development community.  Here are 
suggestions, but I'm not married to them:

   http://www.oasis-open.org/committees/security/schema/assertion
   http://www.oasis-open.org/committees/security/schema/protocol

---------------------------------
Concepts:
Section 1.3, line 149:

I really think we need some material here.  I know that Darren is working 
on a white paper as an outreach tool; maybe we can steal a bit of 
that.  But I think we also need the following topics to be covered:

- Enough of the domain model to introduce the basic authority/asserting
   party vs. client/requester/relying party structure and terminology (and,
   of course, making a cross-reference to the Glossary).  We should have
   a simplified producer-consumer model figure here.

- An explanation of the SAML concept of "namespaces" (not XML ones) that
   is used for actions, attributes, etc.

- A slightly deeper explanation of our theory on SAML extensibility.

---------------------------------
Identifer references:
Section 2.2.1, line 182; Section 2.3.2, line 242; and Section 3.5.1, line 955:

The semantic described here clearly pertains to identifiers for things -- 
*not* to identifier *references*.  Thus, it feels wrong to me to use IDType 
in declaring the <AssertionID> element and the InResponseTo attribute on 
<Response>.  Either the section that defines IDType has to explain the 
dichotomy of semantics, or we need to have a parallel simple type called 
IDTypeRef.

I don't terribly mind having a split in the semantics overlaying the same 
type, but I think it's cleaner from the perspective of data binding to 
define two types.  Also, (something I've suggested before) I would prefer 
that the <AssertionID> element be called <AssertionIDRef> instead.  I think 
it's confusing to have an AssertionID attribute that means one thing and an 
<AssertionID> element that means another.

Here's how things would look if my suggestions were followed:

<!--currently lines 198-200; unchanged-->
<simpleType name="IDType">
   <restriction base="string"/>
</simpleType>

<!--new; described separately as a reference to an IDType-->
<simpleType name="IDTypeRef">
   <restriction base="string"/>
</simpleType>

<!--currently line 245-->
<element name="AssertionIDRef" type="saml:IDTypeRef"/>

<!--currently lines 965-970-->
<complexType name="ResponseAbstractType">
   ...
   <attribute name="InResponseTo" type="saml:IDTypeRef" .../>
   ...
</complexType>

---------------------------------
<AssertionSpecifier>:
Section 2.3, line 218:

As I've commented before, I think we don't really need the 
<AssertionSpecifier> element (particularly if we get rid of the 
single/multiple distinction and all that's left is AssertionID[Ref] and 
Assertion).  It's basically functioning as a concrete schema model group 
and doesn't add any juicy semantics.

I'm running out of time to write these comments, but if there's interest, 
I'll work up the set of schema changes as soon as I can (probably after my 
vacation).

---------------------------------
Conditions complexity:
Section 2.3.3.1, line 308:

This is a minor point, but I think the content model for Conditions should 
be rationalized a bit to put the SAML-native stuff first.  It's more 
complicated to allow a mixture of AudienceRestrictionCondition and 
Condition than to require any audience information to go first, with any 
extension stuff trailing behind.  Here's how the schema would look with 
this change:

<!--currently lines 309-316-->
<complexType name="ConditionsType">
   <sequence>
     <element ref="saml:AudienceRestrictionCondition"
       minOccurs="0" maxOccurs="unbounded"/>
     <element ref="saml:Condition"
       minOccurs="0" maxOccurs="unbounded"/>
   </sequence>
   ...
</complexType>

---------------------------------
Terminology:
Section 2.3.3.2, line 371 as an example (but really a global issue):

There are several terms bandied about in this spec that I'm concerned are 
underdefined or inappropriately used: [SAML] application, [SAML] client, 
[SAML] service, principal.  And there are terms that I'm surprised are 
*not* used: authority, requester, responder.  I'd like to suggest that:

- we stick to "subject" instead of "principal" because that's what the 
markup is focused on;

- we use "requester" instead of "client", because a requester could be a 
service itself;

- and that we use "[SAML] authority" instead of "[SAML] service" because 
we've carefully defined this term.

---------------------------------
<AdviceElement>:
Section 2.3.3.1, line 284; and Section 2.3.3.2, line 369:

The <Conditions> structure uses the <Condition> element as the only obvious 
extension point, whereas the <Advice> structure not only has 
<AdviceElement>, it also allows ##other namespaces without explicit 
extension of the SAML schema.  I just want to check that we have a 
rationale for this -- with conditions, we want to force them to extend the 
schema, right?  And with advice, it's okay to use the SAML schema as is 
because it won't change the native semantics?  But then in the case of 
advice, I'm not sure why we need <AdviceElement> at all, because they can 
always extend <Advice> to require whatever they want, and either way you 
still need an extension schema because <AdviceElement> has an abstract type.

So I guess the net is, I suggest we get rid of <AdviceElement>, whose name 
is awkward anyway.  And we should probably explain the theory behind the 
two different extension approaches in Section 5 (which I'm planning to do 
anyway).

---------------------------------
Model group:
Section 2.3.4, lines 412-416; and Section 2.3.5, lines 434-437:

I expect we're going to throw out the single/multiple distinction today, 
but *if* we don't, I suggest that the schema use a model group to ensure 
that these two choice lists stay in sync.

---------------------------------
Single subject:
Section 2.4.2.1, line 471:

I feel strongly that we should disallow information for multiple subjects 
in the <Subject> element.  Not only is the practice deprecated in this 
section already, but the rest of the spec seems to assume that "a single 
subject" will be supplied wherever the <Subject> element is used (which is 
everywhere!).  This would change the schema as follows:

<!--currently lines 488-494-->
<complexType name="SubjectType">
   <choice>
     ...
   </choice>
</complexType>

---------------------------------
<SubjectConfirmationData>:
Section 2.4.2.3, line 528; and Section 2.4.2.3.2, line 536:

This element is local; it should be global, like the rest of them.

---------------------------------
AuthenticationMethod namespace:
Section 2.4.3, line 544:

We currently say nothing about the "namespace" from which the 
authentication method values come.  Who determines whether it's "password" 
or "Password"?  Is this a function merely of the Bindings spec, or should 
we have an AuthenticationMethodNamespace attribute here, with some 
canonical URIs supplied in Section 6?  Currently this is too underdefined.

---------------------------------
AuthenticationLocality weirdness:
Section 2.4.3.1, line 566:

The rationale provided in this section is too informal and perhaps too 
voluminous.  It would be ideal to turn it into something that can be 
described formally as "OPTIONAL".

Also, the text says that DNSAddress is required, while the schema says it's 
optional.  Which is correct?

---------------------------------
<AuthorizationStatement> weirdness:
Section 2.4.4, line 583:

There are a couple of things potentially amiss here.

- How can the Resource attribute value be optional?  If it can be absent, 
how should the resource be interpreted?   I would prefer to make it required.

- How can the Decision be optional?  (I think someone has brought this up 
before.)

---------------------------------
URI references:
Section 2.4.4, line 590 (but really a global issue):

We keep talking about "URIs" throughout, but do we really mean to exclude 
URI references (with # fragment identifiers on the end)?  I don't think 
so.  We should say "URI reference" throughout.  I've contributed some 
definitions to the Glossary that I hope will help our readers understand 
the difference.


---------------------------------
<AttributeValue> contents:
Section 2.4.5.1.1, line 704:

Do we really want ##any here, or ##other?  I wouldn't want any SAML 
elements appearing here, since it would be unspecified what their semantics 
would be in this context.

Where we use ##other, BTW, we should probably be careful to explicitly 
exclude both saml: and samlp: elements.

---------------------------------
Status codes:
Section 3.2.2, line 760:

This is just a reminder that we need to solve this open issue.
---------------------------------
Queries for multiples:
Section 3.4, line 839 (but really global to this section):

Some of the language in the sections describing the queries talks about 
responses returning *an* assertion, *a* statement.  We need to clarify the 
circumstances under which multiple ones can be returned as a part of the 
definition of the protocol.

---------------------------------
CompletenessSpecifier occurrence:
Section 3.4.4, lines 897 and 917:

Given that it's essentially a response parameterizer/filter, why is this 
attribute required?  Have we considered making a default?  I would think 
that Partial is a good default, but folks may already have discussed 
reasons to force users to supply the filter they want.

---------------------------------
Abstract responses:
Section 3.5.1, line 950:

Why does <Response> have an abstract type?  It's not like we have 
specialized responses for the different kinds of assertions returned; you 
have to use the actual <Response> element.  I think the type should be made 
concrete (but of course it should still be possible to extend it, like all 
the other types).

---------------------------------
Versioning testability:
Section 4, line 1000:

How testable are all the MUSTs, MUST NOTs, and MAYs in this section?  Also, 
when it says (e.g. on line 1029) that a version number is "not supported", 
by whom is it not supported?  The basic confusion here is that "SAML 
application" is being used instead of something more specific, like 
requester vs. responder.

---------------------------------
SAML semantics:
Section 5, line 1054:

At some point a couple of months ago, I proposed wording that we should use 
to say that the semantics of SAML constructs cannot be changed by extension 
schemas.  I don't have time to look it up now, but a blanket statement 
should suffice even for the <Conditions> case, because the wording that I 
added to describe the evaluation of conditions makes it very clear what the 
SAML semantics are even in that case.

---------------------------------
Type derivation vs. substitution:
Section 5.3, line 1094:

This information is useful, but feels really non-normative.  Should it be 
in an appendix, separate from Section 5, or is that a little too off the 
beaten track?

---------------------------------
Identifier URIs:
Section 6, line 1129:

As a start, I propose that we use the following base URIs for the 
identifiers that we invent:

   http://www.oasis-open.org/committees/security/ids/confmethods/...
   http://www.oasis-open.org/committees/security/ids/actions/...

--
Eve Maler                                    +1 781 442 3190
Sun Microsystems XML Technology Center   eve.maler @ sun.com