Re: [security-services] comments re sstc-saml-holder-of-key-browser-sso-draft-07

["Tom Scavo" <trscavo@gmail.com>]

On Sun, Nov 2, 2008 at 11:14 PM, Nate Klingenstein <ndk@internet2.edu> wrote:
>
> Between that and my delays getting this forward
> thus far, I don't think we can push it towards CD just yet

I think this spec is progressing at a typical rate, but yes, I agree
it's probably a mistake to send it to Public Review now since you'd
likely end up having to respond to significant comments from the
community.

>> - As it stands, this profile permits an arbitrary combination of HoK
>> and non-HoK assertions in a given response.  Is this advisable?  It
>> leads to a more difficult implementation, does it not?
>
> I don't think it makes the implementation more difficult, necessarily.  It
> could make the deployment more difficult, but that's deployers' prerogative.

Suppose, for example, an IdP issues an authn assertion and an
attribute assertion (which is not unlikely).  What is your advice
regarding the subject confirmation of both assertions?

>> - Did you consider including language such as that in the following
>> message?
>>
>> http://lists.oasis-open.org/archives/security-services/200809/msg00009.html
>
> I've added as much as I can without defining the binding of <ds:KeyInfo> to
> SubjectConfirmations in an AuthnRequest, and left the language such that
> when this is done, it should compose easily with this spec.

I'll look at this in the new revision, thanks.

>> - This profile needs a version history.
>
> Isn't this handled better already by the OASIS document repository?

Kavi is difficult to navigate and so it would be great for a scavenger
hunt but that's about all! ;-)

> I've added a reference to it.

The SAML wiki provides a fairly good version history (or at least it
contains the information required to construct one easily).

>> - [lines 394--395] This sentence seems to say that if a <saml:Subject>
>> element is included in the request, the <samlp:AuthnRequest> element
>> MUST be signed.  Is this the intent?
>
> Yes.  I can't think of a meaningful use case for a non-trusted
> <saml:Subject> in an <samlp:AuthnRequest> under this profile.

Sorry, I must be missing something.  A <saml:Subject> in the request
is simply asking for a strongly matching <saml:Subject> in the
response.  Why does that require the SP to authenticate to the IdP?

>> - [lines 413--414] The phrase "if the user agent cannot satisfy the
>> <saml:SubjectConfirmation> present in the <samlp:AuthnRequest>"
>> doesn't make sense.  I thought that the <saml:SubjectConfirmation>
>> element in the request indicated the SP's desire to obtain an
>> assertion containing such a <saml:SubjectConfirmation> element.
>
> There can still be a failure to satisfy it, e.g. if other confirmation
> methods were included or the format requested were not supported by the IdP.

I'm including the text here for reference:

"If the user agent cannot satisfy the <saml:SubjectConfirmation>
present in the <samlp:AuthnRequest>, or it fails to obtain a key from
the user agent, the identity provider MUST respond with a
<samlp:Response> message containing an error status and no
assertions."

I don't know what it means for the user agent to "satisfy the
<saml:SubjectConfirmation> present in the <samlp:AuthnRequest>."  All
I know is that the IdP must issue an assertion containing a
<saml:Subject> element that strongly matches the <saml:Subject>
element in the request.  Now the meaning of "strongly matches" is
sufficiently vague that I wouldn't dare to characterize it, but saying
that the user agent must "satisfy the <saml:SubjectConfirmation>
present in the <samlp:AuthnRequest>" is just adding fuel to the fire.

>> - [lines 445--446] What is a "uniquely identifying <saml:NameID>"?
>
> A uniquely identifying <saml:NameID>, e.g. one that refers to a specific
> principal at the IdP.  From my perspective, I don't think I can pick any
> clearer words.

Sorry, this paragraph makes no sense.  First, a <saml:AuthnStatement>
element doesn't contain a <saml:NameID> element, and second, why would
the IdP *not* assert a NameID "that refers to a specific principal"?

>> - [lines 462--464] This requirement is apparently contradictory.
>
> I don't think so.  I just don't want any further requirements, inclusive or
> exclusive, on the certificate itself from this profile's point of view.
>  It's just a vector for keying information.  Deployments are still free to
> use them for anything else they want.  I tried refining the language to make
> this more clear.

Okay, I'll review what you wrote in the new version.

>> - [lines 470--473] The first sentence normatively specifies three
>> security properties while the second sentence only mentions two.
>> Moreover, the second sentence has no normative language.  Perhaps this
>> is an oversight?
>
> I've tightened the second sentence to a SHOULD, avoiding a MUST in case
> someone wants to do something weird.  The third sentence has been merged
> with the prior paragraph, and I think it covers confidentiality.

Same here.

>> - [lines 516--523]  Are these two paragraphs redundant?  Isn't this
>> simply reiterating what is already written in the metadata spec?
>
> This was added in response to your comments on 2 September.  The language in
> the metadata spec is pretty tight, now that I've reread it, so I'm
> comfortable pulling this completely.
>
> http://lists.oasis-open.org/archives/security-services/200809/msg00001.html

Sorry, I should be more consistent with my comments.  In my defense,
we did not have a Metadata Interoperability Profile on 2 Sep (or at
least one whose scope I understood).

There is no guidance in the metadata spec regarding the
WantAssertionsSigned attribute, for example.  This is perhaps as it
should be.  Does this belong in the Metadata Interoperability Profile?
 If not, I don't see any alternative but to profile it here.

>> - [line 297] s/spoofing/initiating/
>>
>> - [line 300] s/spoofed/initiated/
>
> Not proper spec language, huh?  Can I use "forgery"? :D

The word "spoof" has connotations.  Are you trying to convey some
hidden meaning with these words?  If so, it's probably better to be
explicit.  If not, I think you should choose more benign wording.

>> - [line 560] s/validation of key possession/holder-of-key subject
>> confirmation/
>
> This list keeps growing semi-exponentially.  I hope this is the peak of the
> nit bubble, because while these are great changes, I can't imagine you'll
> find even more next time.

In my review of draft-06, suggested edits (aka nits) appeared for the
first time.  Previous versions of the spec weren't ready for
"suggested edits," I'm afraid.  In other words, I have not been able
to do a technical edit until now (since the document contained too
many errors).  Sorry, there's only so much I can do given this formal
review process.

> I think you've done more for this spec than I have

If you'd like me to do a guest edit for technical correctness, I'd be
happy to do that.

Tom