[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Review comments on sstc-saml-core-2.0-draft-14-diff.pdf
General: I'd like to offer my compliments on a solid and substantial piece of work. Following Scott's comments on the 1 June focus call, suggesting that the core, profiles, and bindings documents were sufficiently complete and stable to review substantively, I decided to take a fresh look. Many of the points I'm citing here concern possible clarifications, which could (I hope) be helpful to readers who haven't been involved in SSTC discussions. I'll follow this with a separate message with a few comments on the bindings draft. Specific: Line 269: This may be pedantic, but I'll suggest changing "MUST consist of at least one" to "MUST contain at least one". As written, someone might interpret the sentence as meaning that SAML strings cannot contain any whitespace characters. Lines 282-283: This is an attractive goal, but I'm not sure how one entity can act to fully ensure a separate entity's behavior regarding identifier assignment and usage. Consider, e.g., the possibility that a peer might have an implementation bug causing a received assertion's ID to be copied into another assertion that it generates. If we wanted to introduce stronger boundaries in this area, we might need to consider a structured ID format, where one component identified the ID's issuer and it would be clearly apparent if a particular ID were emitted by some other entity. Within the scope of the pseudorandom approach as intended, it might be useful to underscore the need for unique seeding of the generator within each system. Lines 529-530: I think it would be helpful to provide some further commentary about the case without statements, since this may be surprising to readers when first encountered. Perhaps, "... such an assertion identifies a named principal in a manner which can be referenced or confirmed using SAML methods, but asserts no further aspects associated with that principal", or some such? Lines 776-777: I'm not wholly clear about the semantics or use of <OneTimeUse> from the perspective of a relying party receiving an assertion that contains it, and believe it might be helpful to provide some more clarification here. In normal operation, aren't all assertions that an RP receives likely to be processed immediately on receipt in any case? Further, just what does it mean for an RP to reuse an assertion? If it's presenting that assertion to a third party, this seems like a different sort of interaction, outside the role of acting as an RP. Does the continuing use of any state or session established as a result of processing an assertion constitute reuse of that assertion, for example, so would this be precluded for assertions indicating <OneTimeUse>? Lines 909-911: I think some more explanation would be helpful here, particularly as the text suggests a preference for data items with an unusual characteristic: small integer values which (for privacy reasons) are hoped both to repeat and be reused by providers for different principals. Is there a strong requirement for the string alternative, and (if so) would it be clear to implementers just what they must do in their construction to avoid violating the MUST NOT in the last sentence? Lines 956-957: Since this contains a domain name, not an address, why is this called "DNSAddress" rather than "DNSName"? Line 1423, also elsewhere: I think a definition of the phrase "security context" is needed as used for SAML purposes, and could not find it within this document or in glossary-2.0-draft-01 as linked from the SSTC web page. Lines 1980, 2135, 2221, 2319, 2444: There are several sets of processing rules that state, reasonably, that recipients must validate any signature present on a message. There aren't, however, associated statements about what the recipients are to do if this validation fails. (Similarly at 2332 and 2345, for "must authenticate".) Can we agree and state that recipients are not to act on the contents of messages with failed signatures or following failed authentications, except for purposes of reporting errors? Lines 1988-1990: This text suggests that UnknownPrincipal is the appropriate status code to return not only for an unrecognized subject but also for a recognized subject when an authentication failure occurs. Would it be appropriate to recommend a different status code to represent the second case? Lines 2337-2343: At 2337, suggest changing "to any assertion" -> "to any assertion or session established based on that assertion". The structure of the bulleted list is somewhat odd, in that the last bullet doesn't appear to be a condition on the assertion. Some (non-exhaustive) editorial points: Generally, section cross-references should be revisited and adjusted. Line 40: "protocols that conveys" -> "protocols that convey". Line 520: "statement elements" -> "statement elements may occur within an assertion" ? Line 1281: "in the sending this" -> "in the sending of this" Line 1550: "<RequestAuthnContex>" -> "<RequestedAuthnContext>"? Line 1722: "Reqest" -> "Request". Lines 1917-1918: move clause "and associated information" to end of sentence? Line 2226: "identifier provider" -> "identity provider"? Lines 2384-2387: This sentence does parse, but it's challenging to do so. It might be worth trying to break the content down into a few simpler statements. Line 2397: From the schema fragment, it looks like <NameIDPolicy> is required - add a [Required] tag to the description for consistency? --jl
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]