Conversation
csarven
force-pushed
the
feature/authorization-condition
branch
from
f470bc8 to
6e5a2ec
Compare
uvdsl
left a comment
uvdsl
left a comment
There was a problem hiding this comment.
Thank you @csarven for putting this together!
I left some inline comments. And here are more general questions:
- Do we need to mention the new
linkheader relation in #http-definitions? - Do you consider the section #access-conditions to be appendable? Could we add additional conditions (time, attested attribute, ...) in that section going forward (when consensus is reached in the CG, of course)? Or how do you envision such specification evolution, if at all?
| <p id="consider-acl-resource-activities">Implementations are encouraged to use mechanisms to record activities about ACL resources for the purpose of accountability and integrity, e.g., by having audit trails, notification of changes, reasons for change, preserving provenance information.</p> | ||
| <p id="consider-provenance-accountability">Implementations that want to allow a class of write or control operations on resources are encouraged to require agents to be authenticated, e.g., for purposes of provenance or accountability.</p> | ||
| <p about="" id="consider-client-agnostic-control" rel="spec:advisement" resource="#consider-client-agnostic-control"><span property="spec:statement">Implementations are <span rel="spec:advisementLevel" resource="spec:Encouraged">encouraged</span> to consider scenarios in which Authorizations granting <code>acl:Control</code> are client-agnostic and issuer-agnostic, avoiding inadvertent <a href="#loss-of-control-mitigation" rel="rdfs:seeAlso">loss of control</a> if a client or issuer becomes unavailable or untrustworthy, as well as scenarios in which restricting control access to specific clients or issuers could expose the controller to manipulation through a malicious client or issuer.</span></p> | ||
| <p about="" id="consider-condition-legacy" rel="spec:advisement" resource="#consider-condition-legacy"><span property="spec:statement">Implementations are <span rel="spec:advisementLevel" resource="spec:StronglyEncouraged">strongly encouraged</span> to be aware that Authorizations including <code>acl:condition</code> values evaluated by a server without condition support, such as following data migration from a conditions-aware server, could result in elevated access rights. Implementations are strongly encouraged to verify Authorizations with conditions against the condition capabilities of the server prior to deployment.</span></p> |
There was a problem hiding this comment.
Implementations are strongly encouraged to verify Authorizations with conditions against the condition capabilities of the server prior to deployment.
Would you consider making this statement normative in the discovery section, to be explicit about the expectation that a client may have about a server's support of authorization conditions? This relates to my above comment on the discovery section.
There was a problem hiding this comment.
I think this statement in particular stands as advisory because it is something deployments should be aware of and not something that's testable as a requirement (at least within the scope of this specification).
I've updated the wording on #client-link-condition to signal the purpose better. #acl-condition also mentions discovery of supported condition types, and as a related matter for servers in #server-condition-evaluation that unrecognised is not part of evaluation.
There is always the possibility that a deployment will mess things up, and that's not limited to downgrading from a WAC 1.1 to 1.0 server meanwhile using the same ACL resources. For instance, even while at WAC 1.1. the server may switch authentication mechanisms or even not provide the necessary information to WAC about issuer/client for instance. Of course, these are all possible and some may be (un)likely. But I think the current text in the specification is adequate.
TallTed
left a comment
TallTed
left a comment
There was a problem hiding this comment.
Generally looks OK to me. A few small tweaks to be made.
Note: I have not reviewed with a fine tooth comb. If that is desired, please request another review, preferably with pointer to specific sections about which you're concerned.
| <section id="acl-resource-condition-discovery" inlist="" rel="schema:hasPart" resource="#acl-resource-condition-discovery"> | ||
| <h3 property="schema:name">ACL Resource Condition Discovery</h3> | ||
| <div datatype="rdf:HTML" property="schema:description"> | ||
| <p about="" id="server-link-condition" rel="spec:requirement" resource="#server-link-condition"><span property="spec:statement">When a server wants to enable applications to discover supported condition types (<cite><a href="#access-conditions" rel="rdfs:seeAlso">Access Conditions</a></cite>) for a given <a href="#effective-acl-resource">effective ACL resource</a>, the <span rel="spec:requirementSubject" resource="spec:Server">server</span> <span rel="spec:requirementLevel" resource="spec:MUST">MUST</span> advertise each supported condition type by responding to an HTTP request on the effective ACL resource including a <code>Link</code> header with the <code>rel</code> value of <code>http://www.w3.org/ns/auth/acl#condition</code> and the condition type as link target [<cite><a class="bibref" href="#bib-rfc8288">RFC8288</a></cite>].</span></p> |
There was a problem hiding this comment.
My first thought would be to discover it in Storage Description, LWS advertises capabilities this way https://w3c.github.io/lws-protocol/lws10-core/#storage-description-capabilities
Does WAC expect that available conditions may differ across resources in the same storage. In that case it seems that mechanism to manage that variability is left out of scope.
There was a problem hiding this comment.
Placing condition signalling in the HTTP response header of the effective ACL resource means clients cannot alter what is supported or enforced for a given resource. Variability across resources is accommodated by the per effective ACL resource discovery and ultimately left to the URI owner to manage as appropriate. How a resource server manages that variability internally is deliberately out of scope, keeping the specification simpler and orthogonal to resource server implementation choices ( #http-interactions ).
Co-authored-by: Jesse Wright <63333554+jeswr@users.noreply.github.com>
Co-authored-by: Christoph Braun <braun@kit.edu>
csarven
force-pushed
the
feature/authorization-condition
branch
from
eb2f500 to
cafa174
Compare
Co-authored-by: Christoph Braun <braun@kit.edu>
|
@uvdsl , thanks for co-authoring and reviewing.
#http-definitions was intended to indicate parts that can be defined or registered elsewhere. Extension relation types (as with
Yes, other condition types can be added to this section given need, consensus, and interest to implement. It should stay consistent with what's intended by #authorization-extensions. (Though I hope that we don't end up extending indefinitely because that'd make WAC more complicated than it was ever intended. Simple > Complex. Easier to add, harder to remove.) |
|
Thank you @csarven, this looks good now. I think the option to add more condition types is valuable, and I agree that proposals must be carefully considered before adding to not end up with an "open registry". |
|
We at dokieli Enterprises commit to implementing the client features as described in this PR. |
|
I plan to implement checking the client and issuer conditions as well as providing the URLs of supported conditions for servers to advertise in my Java implementation https://github.com/uvdsl/solid-wac-java |
|
Plans for updating https://github.com/solid-contrib/specification-tests/tree/main/web-access-control |
|
@Potherca, would you be interested in supporting client and/or issuer restrictions with WAC in the Nextcloud plugin and the PHP Solid Server? This PR might be of interest to you and your team. |
|
@woutermont, given that you implemented such checks for clients in the past, or maybe also @joachimvh, would you be interested in supporting client and/or issuer restrictions with WAC in CSS? |
|
Expanding on my NACK from the public-solid mailing list Core issue: fail-open authorizationThis proposal introduces a fail-open evaluation path. Authorization evaluation should be monotonic with respect to constraints: adding a constraint must not expand access. In the current design, an As a result, an ACL can grant broader access on a server with less capability. Concrete example: an ACL author adds a condition (e.g., 2FA, issuer restriction) to protect a sensitive resource. A server that does not support that condition ignores it and grants access based only on agent/mode/accessTo. The intended restriction is silently bypassed. The specification acknowledges this risk in Security Considerations, but does not prevent it. Documenting the risk does not mitigate it. This is a structural issue in the evaluation model. Interoperability and migration riskThis also creates a downgrade hazard:
This is particularly problematic for file-based workflows, migrations, backups, or mixed deployments where condition support is inconsistent. Versioning implicationsThis appears to change the effective security model of WAC. A minor version implies backwards compatibility. Here, the same ACL can produce strictly weaker protection under a conforming implementation. That is difficult to reconcile with a minor revision. Direction for a safe designA condition mechanism can be made sound with the following properties:
I'm willing to help specify and implement this model. Given the change in authorization semantics, this seems more appropriate as either:
rather than a minor update to WAC 1.0. |
Changes from PR solid#134: - Fail-closed evaluation: unsupported condition types make the Authorization non-applicable (instead of being silently ignored) - Write-time validation: server SHOULD respond 422 when ACL contains unsupported condition types - Monotonicity: adding a condition can only restrict access, never expand - Migration safety: less-capable servers deny rather than expand access All other aspects of PR solid#134 are preserved: condition syntax, capability discovery, Client/Issuer conditions, conjunctive evaluation.
|
I have published a fail-closed variant building on PR #134: https://melvincarvalho.github.io/web-access-control-spec/ This is a work in progress. It incorporates The diff from PR #134 is ~10 lines. Contributions and review are welcome. |
|
Expanding on my clarification from the public-solid mailing list ( https://lists.w3.org/Archives/Public/public-solid/2026Mar/0034.html ): Thank you for the exhaustively outlined review and the offer to help, both are appreciated. To clarify the two cases:
The RDF graph remains monotonic throughout: triples are added, not retracted. The normative requirements above address mitigation at the specification level, beyond advisory. An ACL moved to a WAC 1.0 server is in the same position as any ACL containing predicates that server does not evaluate, which has always been the case. |
|
|
|
There is indeed a risk - as I had pointed out in a comment to my PR myself. But this risk exists in WAC 1.0 already : The described issue boils down to managing client expectations: Only if a client expects that a condition is honored, but the server does not support it, then there is "elevation of access" as perceived by the client. This is not perceived by the server because the server has no such external expectation. But this is already possible in WAC 1.0. Take our work from 2021 for example, see data model section. WAC 1.0 allows for such extensions, see here. So, if I use my client from the 2021 work on a regular WAC 1.0 server, then the thus client-supplied rule with the time constraint will also not "fail-close" in the sense as desired by @melvincarvalho. At the core of this risk is the client's expectation "how much" access a server is allowed to grant based on the client-supplied rule. And if the server does not fulfill this expectation, then it is an elevation of access rights in the sense that the "excess access right" was not intended to be granted by the client. But indeed, a conforming 1.1 client does mitigate this risk because a conforming 1.1 MUST discover the supported conditions and security considerations outline secure behavior. The argument that a WAC 1.0 server will grant more access than intended if a condition is present is simply based on the false expectation that the condition MUST apply. This assumption is false because WAC 1.0 does not require supporting custom conditions (neither does 1.1). So, I argue that this then is not the server's fault that it grants more access than desired by a client. It is the client's fault for supplying a rule to a server that lacks the functionality to enforce it. And this is already present in WAC 1.0 (as exemplified by our 2021 work). I am not saying that this risk does not exist, but rather that this risk is already present in WAC 1.0 and we are addressing this risk, the missing client expectation guidance, with this PR for WAC 1.1. On your draft for WAC 2.0, @melvincarvalho, which could and maybe should have been suggested changes to this PR: Your changes do not address the exact issue you outline but considering backwards compatability. That is, that a WAC 1.0 server would ignore unsupported conditions and grant "too much" access. Your proposal exhibits the exact same problem. If you really wanted to mitigate that problem once and for all, WAC-next MUST use a different namespace than WAC 1.0. I have outlined in this comment a proposal for CWAC with sufficient formal grounding. However, in the discussion we reached the rough consensus that is reflected in this PR at hand. |
|
@uvdsl Thank you for the detailed response. I've restored your author credit, apologies for the oversight. On the technical point: you're right that neither proposal can fix WAC 1.0 servers retroactively. The difference is what happens on servers that implement the new spec:
The migration risk from 1.0 is identical for both, agreed. But 2.0 ensures that every server implementing the spec is fail-closed by default. 1.1 does not. |
|
@melvincarvalho, thank you for acknowledging the above. In particular, thank you for acknowledging that the behavior of the proposed WAC 1.1 aligns with its predecessor WAC 1.0 where unsupported extensions/conditions might also be ignored. For the changes proposed in this PR at hand, version number 1.1 is thus appropriate. The above does not take away anything from your concern about the applicability of Authorizations linking to non-supported conditions. I think this comment provides a good summary of what would be required to address this issue. However! Addressing this particular issue of applicability of Authorizations linking to non-supported conditions is out-of-scope for this particular PR. This PR keeps the existing model of WAC 1.0 untouched. Therefore, I agree that if that model was changed, e.g. with modifications re applicability of Authorizations, then this would constitute a major update to WAC, where a discussion would need to happen (or maybe it already happened in the past?) if that is still WAC or a separate extension / a new mechanism that builds in parts on WAC. However, such significant changes are not proposed in the PR at hand. I do acknowledge your concerns. At the same time, I believe they are not substantive to the contents of this PR for the reasons outlined above. That said, absolutely feel free to check whether there already exists an open an issue already voicing your concerns about the applicability of Authorizations linking to non-supported conditions or extension in general in WAC. If non found, feel free to open a corresponding issue. If you do, please link to the discussion in this PR and in #133. Thank you very much for your contribution. Going forward, there is but one thing I would like to kindly ask you to consider: |
|
@uvdsl Thank you. I appreciate the engagement and take your point about process, I’ll open an issue for the broader discussion about authorization applicability as suggested. To be clear: the fork is not a rejection of this work. It's a variant that differs by a few lines. The condition types and syntax are something I value. My point is simply that authorization correctness must be enforced at the server, not depend on client expectations, and that requires a breaking change and a new major version. I'll continue implementing fail-closed semantics in JSS. If the community moves toward that model in future, the spec and implementation are ready. |
6 participants
This PR supersedes #133 and closes #81 .
This PR introduces
acl:conditionas an additional requirement on anacl:Authorization, building on the notion of extensibility previously referenced in the Authorization Extensions section of the specification. The feature rests on a capability detection mechanism: servers that support specific condition types, e.g.,acl:IssuerConditionandacl:ClientCondition(specified in this version of the specification), advertise them viaLinkheaders on the effective ACL resource. Clients discover supported condition types from those headers and deploy condition-bearing authorizations accordingly. Condition types not signalled by the server are not used in Authorization Evaluation. When multiple conditions are present, they are conjunctive, i.e., all must be satisfied for an Authorization to be applicable. This mechanism paves the way for additional condition types to be incorporated in the future based on needs and implementations in the ecosystem, e.g., time-based conditions or ODRL policies, as anticipated in Authorization Extensions.The PR includes the following changes (also included in the
#changelogof the specification) with correction classes:acl:conditionas part of an applicable Authorization.Related TODOs (as separate PRs):
Other TODOs:
Preview