Skip to content

Add maxDuration to limit DoS attacks. - #136

Open
msporny wants to merge 2 commits into
mainfrom
tm-fix-m05
Open

msporny wants to merge 2 commits into
mainfrom
tm-fix-m05

Conversation

@msporny

@msporny msporny commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

This is an attempt at a fix for issue M-05 from the Security Review by SING: w3c/security-request#141 (comment)

/cc @simoneonofri


Preview | Diff

@dlongley dlongley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but note that I think an exception/error is probably better for the timeout case. I think it might be confusing to return empty chains -- which seems to be an implicit signal of a timeout error (which might be confused with other conditions?) rather than just being explicit.

at any depth gains the ability to assert recognition claims that propagate
upward through the entire chain to [=E4|verifiers=] that have configured
only a top-level trust anchor.
only a top-level trust anchor. Depth limits bound one chain. They do not

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting another "trust anchor" use.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, need to do a blanket fix of these across the spec, they've gotten out of control. Will change to "recognition anchor" once all these security/privacy PRs have landed.

Comment thread index.html Outdated
<var>credential</var> followed by the elements of the returned chain to
<var>chains</var>.
<var>chains</var>. If the elapsed time since this algorithm started exceeds
<var>maxDuration</var>, return an empty list.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to throw a TimeoutError instead of returning an empty list.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted fix in c678521.

@msporny

msporny commented Sep 28, 2026

Copy link
Copy Markdown
Member Author

I think an exception/error is probably better for the timeout case.

Attempted fix in c678521.

This branch has not been deployed

No deployments
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 this pull request may close these issues.

2 participants