[Federation] Ambiguities in the definition of constraints in Entity Statements

[Stefan Santesson]

The definition of constraints is ambiguous and should be clarified to avoid interop problems.

Naming constraints

This is said to mimic RFC 5280, but that seems highly unlikely. RFC 5280 is quite complex and does not fit the examples in the draft. For example an RFC 5280 constraint on an URI only include the host part of the UIR (e.g. “.example.com” that may be extended with subdomains and “host.example.com” which cannot.

I doubt that this was the intention of the specification. If it was, then I think it must be clarified in the text (rather than a reference to RFC 5280). I’m trying to figure out what was meant by looking at the examples, and I feel deeply unsure.

• Was it intended as a list of EntityIdentifiers (Exact match)?
• Was it intended as a base URL that can be extended by sub paths (URL prefix)?
• Or was it intended to contain a host name as defined in RFC 5280?

‌

allowed_leaf_entity_types

Here I think the text needs clarification on how to treat presence of metadata for federation entities.

The only way to check this constraints is to compare it with present Entity Type identifiers in the metadata declaration of the leaf entity. But since any entity type may provide common metadata in the federation endpoint entity type metadata, this becomes a bit tricky.

How do you determine that an entity that declares federation endpoint metadata does not use this to act as an Intermediate Entity?

When trying to codify, this appears to be a non trivial check.

I would suggest (as I have before) that all normal federation services declare ALL their metadata under the Entity Type identifier that belongs to their role. That is an RP declare all its metadata under openid_relying_party. And OP declare all its metadata under openid_provider. Consequently the federation_entity entity type identifier is exclusively used to support federation infrastructure roles (Trust Anchor, Intermediates, Resolvers, Trust Mark Issuers) and is never used for regular federation service roles (RP, OP, OAuthClient, AS, Resource servers). This would solve several issues:

• This constraint would be a whole lot clearer and logical
• This would solve the ambiguity in the resolve request that includes a typ identifier which can’t handle metadata for several entity types. This does not work if the OP metadata is divided over several entity type metadata.

If not fixed this way, some clarifications and implementation guidance is needed.

‌

[Stefan Santesson]

I would also propose a simpler approach to the following text

If a Subordinate Statement contains a constraint specification that is more restrictive than the one in effect, then the more restrictive constraint MUST take effect from here on. If a Subordinate Statement contains a constraint specification that is less restrictive than the one in effect, then it MUST be ignored.

This seems to suggest that implementations should attempt to merge different name constraints of the path, and must figure out what is meant by “less restrictive”, which can become an unnecessary complex peace of code.

I would suggest something much simpler (and safer)

Constraints processing MUST check all constraints of the validated chain against their respective subordinate statements in the path.

This does the trick with much less code. If the constraints of the TA is fully checked against the path, no one can make it less restrictive by defining other constraints e.g. in the subordinate IE statement. If the TA constraints is not valid for the whole path, chain validation will fail.

This is also compatible with how constraints is defined to work:

If the claim appears in an Entity Configuration, it MUST be applied to all Subordinates of that Entity. If the claim appears in a Subordinate Statement, then it MUST be applied to the subject of the Subordinate Statement and all the subject's Subordinates.

This certainly seems to support processing each constraint individually against its subordinates. This does not require any comparison of constraints of the path.

[Stefan Santesson]

Here is my core implementation of the constraints check for an individual constraints against its subpath. I iterate this over all present constraints of the path.

This is what I think the standard attempts to say. But I’m not really sure. I would appreciate if you could help me verify if this is a correct interpretation.

Note that the variable “leafEntityTypes” contains all entity types where the leaf statement has present metadata, excluding federation_entity from that list if present.

    // Check max path length = the number of allowed intermediates
    if (maxPathLength != null) {
      int intermediateCount = subordinateStatements.size();
      if (isSelfSigned(leafStatement)) {
        // This implementation allows a chain to end with an Entity Statement.
        // If the last statement is selfsigned it is not counted as an Intermediate Entity statement
        intermediateCount -= 1;
      }
      if (intermediateCount > maxPathLength) {
        throw new ChainValidationException("Max path length constraints check failed");
      }
    }

    // Check naming constraints
    if (excluded != null && !excluded.isEmpty()) {
      // Fail if any subject Entity Identifier starts with any declared excluded name
      if (subjectEntityIdentifiers.stream().anyMatch(subjectId -> excluded.stream().anyMatch(subjectId::startsWith))
      ) {
        throw new ChainValidationException("Excluded name constraints violation");
      }
    }
    if (permitted != null && !permitted.isEmpty()) {
      // Fail if not all subject Entity Identifiers starts with at least one of the permitted names
      if (!subjectEntityIdentifiers.stream().allMatch(subjectId -> permitted.stream().anyMatch(subjectId::startsWith))) {
        throw new ChainValidationException("Permitted name constraints violation");
      }
    }

    // Check leaf entity types
    if (allowedLeafEntityTypes != null && !allowedLeafEntityTypes.isEmpty()) {
      if (!new HashSet<>(allowedLeafEntityTypes).containsAll(leafEntityTypes)){
        throw new ChainValidationException("Leaf entity type constraints violation");
      }
    }

‌

[Vladimir Dzhuvinov]

Tagged as Federation 1.0 issue.

[Michael Jones]

The editors agreed to investigate this for the upcoming Implementer's Draft.

[Michael Jones]

To be addressed by https://bitbucket.org/openid/connect/pull-requests/700

[Giuseppe De Marco]

Closed by https://bitbucket.org/openid/connect/pull-requests/700

[Stefan Santesson]

There are quite some improvements here, but all problems are unfortunately not adressed.

The following problems remains:

• Path length constraints is hard to understand (expect interop problems)
• Last 2 paragraphs in section 6.2 are functionally redundant and only complicates the implementation algorithm
• The examples of permitted and excluded subtrees should NOT include scheme identifier (e.g. https://)
• Allowed leaf entity types does not adress the need to specify under which conditions federation_entity metadata is allowed even if not permitted by this constraint.

‌

Elaborations:

RFC 5280 (in basic constraints) specifies path length constraints as the number of intermediates that are allowed above last certificate (Statement).

In OpenID federation you have one additional statement in the path, the leaf Entity Configuration. Strictly comparing to the logic of Certificate path, you are doing OK, but many will struggle to understand why the last Entity Statement above the Entity Configuration does not count. I fear that many will not read all the details and will use path-length as the maximum number of intermediates above the entity configuration statement. Honestly, I think that would be the right thing to do, because it makes more logical.

‌

The last 2 paragraphs do not add anything to just processing all constraints as declared. If one constraint is more restrictive than another, then both will be enforced. This is equal to enforcing the most restrictive.

It is quite problematic to try to describe a combination logic here. For example name constraint can not just be merged. For example if the TA has a more permissive name constraints, and the lower IE has a more restrictive name constraints, then the TA constraint must be applied to the whole path AND the IE constraint only from the IE and below. There is no natural way to merge these rules. They have to be applied both in their own chain context. I suggest removing these paragraphs stating that all constants must be processed and all of them must be satisfied for the path to be valid. Period.

‌

The last 2 bullets are quire clear and hopefully I don’t need to elaborate.