RE: [security-services] comments re draft-sstc-metadata-iop-02

["Scott Cantor" <cantor.2@osu.edu>]

Finally getting back to this, comments inline, draft-03 will be uploaded
shortly.

> General comments:
> 
> - The SAML V2.0 metadata specification [SAML2Meta] requires that
> signatures be verified.  What if the consumer can not verify the
> signature?

That's out of scope, it's part of exchange/verification. I added a sentence
to that effect.

> - A signature may appear on a RoleDescriptor or a
> AffiliationDescriptor element within an EntitiesDescriptor (or
> EntityDescriptor) element.  What if the consumer can not verify the
> signature on a RoleDescriptor or a AffiliationDescriptor element?

Same thing, that's part of the exchange/import process in which the set of
trusted metadata is determined for a given point in time. This profile only
deals with the content of whatever metadata is ultimately accepted.
Obviously, any sane exchange/verification procedure will do the obvious
thing in this particular case.

> - In section 2.5.1, are you suggesting that keys bound to certificates
> be called out in metadata with the <ds:KeyValue> element?  Why not use
> <ds:X509Data> exclusively in this case?

I'm saying they can be, and it doesn't matter if they are or not. There's no
reason to mandate it either way. The win would be if I disallowed the other
altogether and didn't have to support it. I would have dumped
X509Certificate if I felt KeyValue was practical enough to use.

Once my proposed XMLSig extension for an EncodedKeyValue that's
ASN.1-encoded is approved, I suspect that I'll revise this and add that
option, and it will probably be the favored one, but thanks to SSL, there're
always going to be certificates around anyway.

> - In section 2.5.1, you don't mention <ds:X509SKI> but as you know,
> this element is similar to <ds:X509Certificate> in that it
> unequivocally defines a key.

I think we've reached consensus on that one after subsequent discussions.

> - As noted in section 2.7, the validUntil and cacheDuration attributes
> are particularly important.  However, [SAML2Meta] does not adequately
> treat the case when they appear on a RoleDescriptor or a
> AffiliationDescriptor element within an EntitiesDescriptor (or
> EntityDescriptor) element

I created an errata, now approved, to improve that.

> Specific comments:
> 
> - [lines 163--165] Why is this reference normative?

Agreed, though there's some indirect dependence through XMLSig, anyway, but
I moved it to non-normative.

> - [lines 189--191] I think this should reference the Second Edition of
> [XMLSig].

Fixed.

> - [line 245] Is this subsection really needed?

The profile is insufficient to meet its requirements unless the assumption
in this section holds. There may be other assumptions needed, in which case
I'll add them to this section once they're identified.

> - [lines 257--258] I don't understand the normative requirement you're
> trying convey in this sentence.  Shouldn't this be a MUST?

No, a MUST would be for the consumer. Metadata is already required to be
rooted with one of those elements, by definition. The point here is that you
can conform producing either one, but I think using the normative MAY is
confusing/inappropriate, so I changed that.

> - [lines 258--260] I don't understand this sentence.  In what sense
> does this profile apply to the <md:RoleDescriptor> element?

Most of the profile is governing content that's inside a role. I reworded
the sentence to explain that the profile is meant to apply to all roles that
are included.

> - [lines 278--279] This normative requirement is redundant since the
> previous section contains an identical requirement.

I moved some of the normative language from earlier to this spot.

> - [lines 284--285] I don't see the point of allowing both forms in a
> single <ds:KeyInfo> element.  This could be a source of problems in
> fact.  For instance, how does a consumer process such a construct?

I can't think of a strong reason to include both, but no problem I can think
of is created by doing it. The rules for consumers are down below, and
handle that case. Is there a problem you're thinking of? Obviously they
could just be mismatched, but that's not going to break anything other than
being wrong.

> - [lines 294--296] This requirement contradicts (in spirit, at least)
> section 3.1.5 of [SAML2Meta].

I don't see any contradiction at all, but I don't recall why I included that
sentence to begin with. It seems off-topic, so I pulled it.

> - [line 307] The parenthetical remark on this line is unclear.  What
> does the pronoun "their" refer to?

The EntityDescriptors inside the group, but I removed the clause.
 
> Suggested edits:

Applied.

Thanks, 
-- Scott