OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

security-services message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: RE: [security-services] Additional comments on core-02


Comments inline...

> 1.	*** Lines 613-622 re: subject confirmation: First, 
> since this is really dealing with how to treat confirmations, 
> I recommend moving it into the section on 
> <SubjectConfirmation>.  Next, these are two very long run-on 
> sentences and the phrasing is a bit confusing.  I suggest 
> this alternate text:

I actually prefer it where it is just because it clarifies what the contents
of the element are for, plus the confirmation element is the next section
anyway, so it flows well.

I'm fine with your proposed changes, as long as its understood that we're
not going back to the "confirming entity *is* the subject" meaning it used
to have. This thing is specifically able to support impersonation, and we
need to be clear on that.

> 2.	*** Section 2.4.1.2 re: KeyInfoConfirmationDataType: 
> How does this type get used?  There is no schema-defined 
> element that uses this type.  I expected to see a 
> <KeyInfoConfirmationData> element (or should it be 
> <KeyInfoSubjectConfirmationData>?) defined that was of this 
> type that could be present in the SubjectConfirmation element 
> instead of a more general <SubjectConfirmationData> element.

We could have done this, I don't know that I had any strong reason for not
defining an element. Right now, the data type is used like any other
extension, as an xsi:type. I think the spec is pretty clear on this, I don't
know what else I should say.

I guess we need examples, but I really don't have time to provide them in
addition to everything else I'm doing.

> 3.	Lines 652-654 describes an example using 
> <SubjectConfirmationData> to hold  a <ds:KeyInfo> element.  
> It is a confusing example once you read section 2.4.1.2 on 
> KeyInfoConfirmationDataType.

I added a cross reference. The text is still correct. When the extension
type is used, the element stays the same, so it's content would be a
<ds:KeyInfo>.

> 4.	Lines 682-689: The use of the phrase "while confirming 
> itself" is pretty confusing and could really use some 
> additional explanatory text.  Does this mean the attributes 
> are only used when the entity presenting the assertion is the 
> "subject" of the assertion?

No, it means they're used when evaluating subject confirmation. Core doesn't
define when that is precisely. At least it never did before. I reworded a
little bit to clarify that the attributes apply to the characteristics of an
action performed by a "confirming entity", not the subject. We should make
sure that term is in the glossary. I also added a sentence at the beginning
of the section:

"Subject confirmation takes place when a relying party seeks to verify the
relationship between an entity presenting the assertion (i.e. the confirming
entity) and the subject of the assertion's claims."

I think the problem here is that we're finally fleshing out this whole
concept and making more extensive use of it, and people are noticing that
it's a bit vague and hard to understand. I don't think we can solve this by
adding a lot of text here. We need somebody to write up a good overview of
the concept for the guidelines document.

I still the think the best way to understand them is to read the profiles
document, so we point there.

> 5.	*** Section 2.5.1.5 re: <ProxyRestriction> uses 
> statements like "a relying party MUST NOT issue an 
> assertion.".  This violates the definition of a relying party 
> since only asserting parties issue assertions.

Will add more language, though I think it was pretty clear what was meant.
Adding more tortuous clauses seems to make it less clear to me...

> 6.	Lines 978-979 re: the note that AuthorityBinding was 
> removed.  Should we clutter up the core spec with these 
> notes?  I think they should just go in the 1.1/2.0 
> differences section of the tech overview. If we want to 
> include them, we should go through the spec to mention them 
> all.  For example, we don't mention that the 
> AttributeNamespace attribute was removed in section 2.7.3.1. 
> Lines 1383-1384 do describe removal of <RespondWith>.

Well, we mentioned all the things we removed that have no new equivalent.
Eg. AttributeNamespace was replaced with NameFormat and more liberal
wildcarding.

> 7.	Line 1046: While I know what the "or both" clause is 
> referring to once you read the schema, it's not a very clear 
> description.

But it's precise. And the schema is part of the spec. You aren't expected to
be able to read the spec without it. I think we've been less than firm on
that point. Syntactically, the schema is the spec.

> 8.	Line 1057: The "in addition to the assertion issuer" 
> clause is confusing. What does it mean?

Agreed. It was a poor way to say "the assertion issuer is implicitly
involved, and this element is for naming anybody else that was involved".

> 9.	The intro section of section 3 (Protocols) should 
> mention that in some cases, responses can be created without 
> a previous request being received.

Done.

> 10.	Lines 1429-1431: Might want to add an explanation of 
> why a responder might choose NOT to respond to a request.

I added a sentence about not responding if a DOS attack was suspected. I
can't actually think of any other reasons?

> 11.	Section 3.2.2 re: StatusResponseType.  Why isn't it 
> defined as abstract?

Because some of the response elements use it directly. Why define new types
for no reason?

> 12.	Lines 1549-1551: We should also mention why a responder 
> might NOT use second-level status codes (e.g. to protect 
> against probing attacks).

Added a MAY around omitting them for this reason.

> 13.	Line 1554: ErrorAttrNameOrValue - This is the only 
> error status code with "error" in the name.  Shouldn't this 
> be "InvalidAttrNameOrValue" for consistency?

No opinion.

> 14.	Line 1558: InvalidNameIDPolicy" - he description 
> implies it's just for an unsupported name ID "format". Is 
> "format" synonymous with "policy"?

It refers to the name of the element that is "invalid". Happens to be called
NameIDPolicy, mostly for historical reasons and because no better name was
ever suggested.

> 15.	*** Section 3.4: Why is the response to an 
> <AuthnRequest> not an <AuthnResponse> instead of <Response>?  
> All other protocols have symmetry in their names. This was a 
> major complaint by developers trying to understand the new specs.

A <Response> is used anytime the response message consists of a status plus
zero or more assertions, which matches this case. Should we also have
<AuthnResponse>, <AttributeResponse>, etc.? Isn't it more confusing when the
response might contain information other than just an AuthnStatement? I
think what we have is the best alternative.

> 16.	Section 3.4: The list of actors does not include 
> "Identity Provider".

Added.

> 17.	*** Lines 1880-1884 re: "Presenter": What if the 
> AuthnRequest is sent to the IDP via the Artifact binding?  In 
> this case, the presenter is the entity that presents the 
> <ArtifactResolve> request.

I disagree. I have always been clear that the artifact binding is just
sending a message by reference. It stands in for the actual message, and
from the standpoint of the "outer" protocol exchange, it's no different than
if the message were passed by value. The presenter of the artifact is the
presenter of the message. Anything else is just a mess.

> Throughout the section, the 
> "Presenter" term is, I think, presumed to be carrying the 
> <AuthnRequest> message. For example, in lines 1894-1896, a 
> "presenter" does not really send an IDP an <AuthnRequest> 
> message.  It's the SP that sends it.

See above, I would dispute that characterization. That's why the section is
written the way it is, it doesn't need to even know about the binding. I
think this is better.

> The SP may use the user 
> agent to transfer the message, but the SP may also send it 
> via the Artifact binding.  You might be able to get by with 
> just redefining the term "presenter", but if you are using 
> the Artifact binding, the test isn't really accurate.

I think it's totally accurate.

> 18.	Line 1903; "authenticate or authorize the requested 
> subject to provide a service"? Makes it sound like the 
> subject is providing a service.  I assume you mean that the 
> relying party mentioned earlier in the sentence is what is 
> providing the service.  If so, need to clean up the phrasing.

Done.

> 19.	Line 1922: "expects to govern" - not sure what you mean 
> by this.  Can it be clarified?

Is "limit" better? It makes sense to me, so I need help...what do conditions
do? A simple example is just controlling the lifetime.

> 20.	*** Lines 2011-2015 re: the "AllowCreate" attribute: 
> The definition says that False is the default, which means 
> you should not create new identifiers.  This attribute is not 
> described in Profiles or in Bindings.  According to the 
> current definition, this means that an IDP could never create 
> identifiers (persistent, transient, or any other kind) for 
> the principal.

Not in the context of responding to an AuthnRequest, no. No other context is
part of SAML, so outside SAML, you can do whatever you want, right?

> 21.	*** Line 2071: <GetComplete> seems like an odd element 
> name for this.  Why not something like <IDPListRef>?  There 
> is also no explanation of what dereferencing of this URI 
> would return.  Is it an XML fragment containing <IDPEntry> 
> elements?  It's included in an example in Profiles, but is 
> never explained.

It's not something I came up with, so I left it (and the name) specified
pretty much how it was in ID-FF. I added a sentence explaining what it
represents.

> 22.	Lines 2970-2971: "the signature MUST first be 
> calculated and in place, and then the element encrypted" 
> doesn't parse - "and in place"?

The Signature content needs to be part of the XML instance being encrypted.
I changed the wording.

-- Scott



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]