Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve Endpoint Error Response #136

Open
zachmann opened this issue Nov 8, 2024 · 10 comments · May be fixed by #145
Open

Resolve Endpoint Error Response #136

zachmann opened this issue Nov 8, 2024 · 10 comments · May be fixed by #145
Assignees

Comments

@zachmann
Copy link
Collaborator

zachmann commented Nov 8, 2024

The specification does not say much about the error response from the resolve endpoint, only that it follows section 8.9 "Responses from Federation Endpoints", which lists a few different error strings that SHOULD be used.

A resolver might limit its resolving service to only a (sub-)federation and not resolve every trust chain in the world.
In a hierarchical federation, entities therefore might want to query multiple resolvers.
In such a scenario it could make sense to have some error conditions standardized to ease the error handling.
Especially, it would be helpful to differentiate between the error cases where the resolver does not accept the request (e.g. because it does not support the specified trust anchor) and the case where it cannot find any valid trust chain.

Do you think it is in scope for the specification to specify the error responses in more detail (I would say it's enough to follow the principle of 8.9), or it's entirely up to the implementations?

@selfissued
Copy link
Member

If there are error conditions that you believe would be useful to signal to callers, please suggest their definitions in this issue.

@zachmann
Copy link
Collaborator Author

I can think of the following errors:

  • invalid_trust_anchor / invalid_anchor (should be in line with Inconsistent Trust Anchor parameter names: anchor, trust_anchor_id #131): The endpoint cannot service the requested trust anchor. The HTTP response status code SHOULD be 404 (Not Found).
  • invalid_sub: The endpoint cannot service the requested subject. The HTTP response status code SHOULD be 404 (Not Found).
  • invalid_trust_chain: The resolver cannot construct a valid trust chain between sub and anchor. The HTTP response status code SHOULD be 404 (Not Found).
  • invalid_metadata / metadata_policy_error: The resolver cannot verify the metadata [Probably need a better description here]. The HTTP response status SHOULD be 400 (Bad Request).

@peppelinux
Copy link
Member

invalid sub and invalid trust anchor might be used also in the fetch endpoint

while invalid trust chain and invalid metadata along with metadata policy error would be used in the resolve response

I can do a PR for this, thank you @zachmann

@peppelinux peppelinux self-assigned this Nov 15, 2024
@zachmann
Copy link
Collaborator Author

Yes the first two are more general, while the later two are more specific to the resolve endpoint.

But I would say invalid_trust_anchor is not applicable for fetch, since the request does not contain a parameter for ta, but only issuer. (invalid_issuer already exists btw.).

I welcome a PR.

@selfissued selfissued self-assigned this Nov 19, 2024
@selfissued
Copy link
Member

selfissued commented Nov 19, 2024

Note that we already have defined these error codes:

  • missing_trust_anchor - No trusted Trust Anchor could be found.
  • trust_chain_validation_failed - Trust chain validation failed.

They could be construed as overlapping with invalid_trust_anchor and invalid_trust_chain. Arguably, the new proposed error codes are more general. Should we replace the existing error codes with the new ones?

@selfissued
Copy link
Member

It seems that we should harmonize the error codes defined at 8.9. Error Responses from Federation Endpoints and those defined at 12.1.3. Authentication Error Response.

@selfissued selfissued linked a pull request Nov 19, 2024 that will close this issue
@zachmann
Copy link
Collaborator Author

Note that we already have defined these error codes:

* `missing_trust_anchor` - No trusted Trust Anchor could be found.

* `trust_chain_validation_failed` - Trust chain validation failed.

They could be construed as overlapping with invalid_trust_anchor and invalid_trust_chain. Arguably, the new proposed error codes are more general. Should we replace the existing error codes with the new ones?

Indeed, I missed those. I agree that harmonizing makes sense.

I think trust_chain_validation_failed and invalid_trust_chain are indeed similar, it's enough to have one of those only, I don't have a strong opinion about which one to keep; keeping trust_chain_validation_failed obviously has the upside that existing implementation do not need to adapt.

My understanding of the existing missing_trust_anchor is that when resolving the trust chain following the authority hints does not end up in any trust anchor (request server defines the trust anchors). This is differently from my proposed invalid_trust_anchor which means that the client sends trust anchor(s) and the server does not accept does.
I would say it's not ideal to have such similar error codes where the difference might not be obvious. However, I don't know if the missing_trust_anchor is needed, I would say this is part of trust_chain_validation_failed / invalid_trust_chain. I don't know if it needed to have a separate error code for this.

@peppelinux
Copy link
Member

Very good points @zachmann

I'd go for

trust_chain_validation_failed
Inconsistency in statements, signature errors, missing entity id bindings, authorities endpoints not reachable or timedout

metadata_resolution_failed
Something went wrong in the resolution of the requested metadata

metadata_not_found
The requested metadata is not available within the statements

trust_anchor_unknown
The requested trust anchor Is not trusted by the resolvers

subject_not_found
The leaf Is not available in the cache or not discoverable through it's entity configuration

@zachmann
Copy link
Collaborator Author

I'm fine with the first three;
for the later two, personally, I prefer the definitions of invalid_trust_anchor and invalid_subject proposed in the PR #145
I find those more general useful.

@selfissued
Copy link
Member

I've updated the PR to reconcile the two different sets of error codes. Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants