[DaleSeo]

Fixes #1298.

Motivation and Context

#935 added a same-origin requirement for the challenge's resource_metadata URL to block SSRF. But that also rejects a valid setup where one central protected resource metadata (PRM) host serves metadata for multiple resource servers, which RFC 9728 and SEP-985 allow. Discovery discarded the pointer before reading authorization_servers.

Now, a resource_metadata URL from a WWW-Authenticate challenge doesn't have to be on the same origin as the MCP resource. Instead, it goes through the same SSRF host check used for authorization server URLs.

How Has This Been Tested?

Add tests

Breaking Changes

None.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added appropriate error handling
• I have added or updated documentation as needed

[alexhancock]

This doesn't appear to me to check if it's http vs https.

Self::is_same_origin(base_url, candidate) used to do this incidentally by comparing the schemes, but now nothing does.

Should consider adding an explicit check for that back to this method.

[DaleSeo]

@alexhancock Good catch, thanks. Added an explicit no-downgrade check.

[ndreno]

Same-origin also provided a second property incidentally, beyond the scheme: it did not depend on DNS. is_allowed_metadata_url still decides from the URL's host string, and for a DNS name that returns "allowed" without constraining the address the client connects to. With same-origin gone, that now applies to resource_metadata as well. I sent the details through the advisory process (GHSA-5p52-q7pq-9fq2) and suggested a fix in #1298: filter resolved addresses with a reqwest::dns::Resolve on both discovery clients. Worth settling before this merges.

[DaleSeo]

Thanks for flagging, @ndreno. Would you be willing to start a temporary private fork to follow the advisory process? I'd like to keep this PR focused on cross-origin metadata support and hold off on merging until the security fix is ready and the maintainers have agreed on sequencing.

[ndreno]

Yes, I'll start the fork from the advisory and put the resolver-level fix there, with the reproduction turned into regression tests. One design question will come with it, on how the loopback exception carries over once the check moves to resolution time. I'll raise it in the fork.

[mzeng-openai]

👋 thanks @ndreno and @DaleSeo. Is the security fix still waiting on review or a decision on loopback handling? What’s left to unblock it? We also have a few issues waiting to be unblocked by this change.

[DaleSeo]

Thanks for checking in, @mzeng-openai. This PR is still waiting on the related security patch in the advisory's private fork. I reviewed the fix and left some feedback, but I haven't heard back yet.

@ndreno, could you take a look when you get a chance? If you don't have time right now, just let me know. The maintainers can take it from here so this doesn't hold anyone up.

[ndreno]

Sorry guys, I'm quite overwhelmed with work those days and won't be able to look at those topics for some time

[mzeng-openai]

no worries, can we have someone else take on the security patch in the meanwhile?

[DaleSeo]

@mzeng-openai I took over the security patch, and it's now merged into main. I've rebased this PR on top of it, so it's ready for review again. Both will be included in the next release, 3.7.0. Thanks for your patience!