ucan-wg / delegation

1 stars 1 forks source link

v1.0.0-rc.1 #2

Open expede opened 11 months ago

expede commented 11 months ago

I may get pilloried for this version. WIP, obviously

Preview :books:

Okay, this version switches to IPLD. This makes it much easier to not need a IPLD version off to the side, and many teams that have adopted UCAN already use IPLD somewhere in their stack. There is a contingent of people that feel strongly in favour of JWT for a variety of reasons. I also defended the JWT strategy for a long time, beacuse I had many first-hand converstaions of "I need to sell this to management, please tell me it's a JWT and not some inscrutable binary format".

A few things have changed:

I believe that this proposal makes writing both UCAN Delegations and libraries much easier and more comprehensible. It also lowers our maintenance burden between multiple formats.

Changelog

Metadata

Structure

Time

Prose

Gozala commented 7 months ago

I have recently been dealing with a case where account re-delegates anything ever delegated to it to some agent. In previous ucan spec this could be expressed as { with: "ucan:*", can: "*" }. I am realizing that this draft does not handle this case or at least is not specified in the text.

@expede what is the current take on this ? We really would like to support this somehow to handle an account recovery. I am not a big fan of ucan:*, but nothing is even worse.

Perhaps we could do something similar to what we did with exp: null to imply never expiring delegation and have sub: null to mean everything I could possibly delegate ?

Part of me also wishes I could constraint those in cond along with other things.

As an alternative I'd be in favor of making sub a SQLite like glob pattern (where * means any number on any chars and ? means any single character) as can fields are kind of already like it. That way we'd be able to delegate everything as { sub: "*", can: "*" }. I suppose only supporting * in addition to DIDs may be more pragmatic approach similar to sub: null and allow to be extended in the future.

smoyer64 commented 2 months ago

Selector analysis/implementation

I'm doing preliminary work on a Go implementation of UCAN v1.0.0-rc1 and have a few recommentations based on attempting to implement policy validation. This comment in particular will focus on the selector element as this seems to have the most discontinuity within the policy portion of the v1_ipld delegation` specification.

Methodology

The following methodology was employed to develop the policy validation (not all these steps are fully complete pending this discussion):

  1. Copy and slightly modify the policy ABNF from the delegation specification. I've submitted a PR with just enough changes to make the ABNF valid.

  2. Copy and normalize the examples in the [Selectors]() section of the delegation specification for use as interop testcases.

  3. Generate Go Examples (tests with output) for each testcase. For each testcase, a jq library is used to evaluate the example args against the selector.

  4. Execute each testcase with the same input using jq 1.6 and verify the the output is expected. For each testcase, a jq library is used to evaluate the example args against the selector.

Result

  1. There are many inputs which are valid selectors per the ABNF but that are invalid or non-working filters in jq.

  2. There are examples in the specification that don't behave per the specification.

Recommendations

  1. The specification includes the following line:

    Any selection MAY begin and/or end with a single dot. Multiple dots (e.g. .., ...) MUST NOT be used anywhere in a selector.

    Any jq filter that doesn't begin with a . is effectively a constant and applying that filter to ANY input value will result in the filter constant being sent to the output. This behavior can be simply demonstrated by the following shell command:

    $ echo -n '{"field":"value"}' | jq --raw-output '"filter"'
    filter

    Since the third element of an equality or inequality is also effectively a constant, statements composed this way will yield a value that can be computed without the invocation's args field and should therefore be excluded from the predicate tree evaluation. This

    Therefore, I'd recommend that the word "MAY" in the above quoted text be replaced with the word "MUST".

  2. The delegation specification includes .to[99]? as an example of using a try operator. The results of a jq command with an unknown object or array index is null, so stating that the ? operator turns what would otherwise be an error into a null isn't demonstrated by this example. This behavior can be demonstrated by the following shell commands:

    $ echo -n '{"field":"value"}' | jq --raw-output '.["nope"]'
    null
    $ echo -n '{"field":"value"}' | jq --raw-output '.["nope"]?'
    null

    We should choose a try operator example that effectively demonstrates the intended utility. Note that the examples in the jq manual don't show the utility of the Optional operator ? either.

  3. The delegation specification makes the following statements:

    UCAN Delegation uses predicate logic statements extended with jq-style selectors as a policy language.

    and

    Selector syntax is closely based on jq's "filters".

    but then further explains the differences between UCAN selectors and jq filters in the "Differences from jq" section. After reading through this section, it seems like we should be able to say that the selector grammar is a proper subset of the jq filter grammar.

    I have mixed feelings about the name selector as we're actually filtering the invocation's args and then applying the predicate. I can however see some utility in "naming" the subset. Using the word try to describe the ? operator is a bit harder to justify - that word has connotations in many other languages (how do I catch the result of a selector with a try?) In jq, this operator is described as an Optional and it's very similar to optional chaining in both Typescript and Groovy. If Optional isn't the correct word, perhaps we can pick another without "baggage"?

  4. The delegation specification states that the ABNF included in the Policy language section is the "formal syntax". In my opinion, this ABNF should be suitable for use when generating a parser in your language of choice. In its current state, the selector syntax is too permissive and it should be strengthened to limit selectors to those that are intended throughout the rest of the document. This is obviously impacted by the decisions about recommendations #1 and #3 above.

    Since I'm already generating a Go parser from this grammar, I have a vested interest in the ABNF being syntactically strict and would be happy to complete this work. If recommendation #3 is adopted, I'd also fuzz the selector by generating a corpus from the ABNF and executing it in jq. The output of jq should then provide the test results expected from the generated Go code.

Notes

  1. It's entirely possible that I'm reading the specification incorrectly - if so, please don't take the above input personally as I'm simply trying to adopt the awesome ideas presented in the UCAN specifications.

  2. I was going to compare my prototype policy code the the Rust implementation that was linked at the end of Brooklyn's IPFS Camp presentation (https://github.com/expede/rs-ucan.) Can someone provide the link to this code? Is it suitable to think of the Rust implementation as the RI?

Gozala commented 1 month ago

@smoyer64 thanks for taking a stab at it and sharing the feedback. It may be worth pointing out that selector syntax while was mostly inspired by jq, compatibility with it was not a goal. Bunch of jq features aren't supported and there were set of cases where diverging from jq seemed like a better choice in our context, you can view some of the details here https://github.com/ucan-wg/delegation/issues/5

I'm not sure about the rust implementation but I had a draft of JS implementation that can be viewed here https://github.com/storacha-network/ucanto/pull/344/files

expede commented 1 month ago

Thanks for the feedback @smoyer64 ! Responses inline:

Quickly jumping to the end first

  1. It's entirely possible that I'm reading the specification incorrectly - if so, please don't take the above input personally as I'm simply trying to adopt the awesome ideas presented in the UCAN specifications.

Thank you for the deep engagement with the material! I think other than the question about how strictly we adhere to jq, I'd like to implement the rest of your comments! πŸ™

[...]

  1. Copy and slightly modify the policy ABNF from the delegation specification. I've submitted a PR with just enough changes to make the ABNF valid.

PR merged! Thanks :)

[...]

Result

  1. There are many inputs which are valid selectors per the ABNF but that are invalid or non-working filters in jq.

Yes, as Irakli responded earlier, we do have some divergences from jq. We think that they're well motivated ( #5 ) , but they have already bitten both you and me, so perhaps we should rethink that.

Recommendations

[...]

Therefore, I'd recommend that the word "MAY" in the above quoted text be replaced with the word "MUST".

Agreed βœ… Will fix (DONE)

  1. The delegation specification includes .to[99]? as an example of using a try operator. The results of a jq command with an unknown object or array index is null, so stating that the ? operator turns what would otherwise be an error into a null isn't demonstrated by this example. This behavior can be demonstrated by the following shell commands:

This is one of the examples that we chose to diverge on. In short: jq is (largely) a parser that processes streams, and UCAN policies are a policy language that operate on static data. The security implications of permissive behaviour may be significant, and we can always get the null behaviour by using a try (?).

There's also a few cases of expressivity, e.g. distinguishing between a null value and a missing key.

Note that the examples in the jq manual don't show the utility of the Optional operator ? either.

Interesting! Can you expand on this?

  1. The delegation specification makes the following statements:

    UCAN Delegation uses predicate logic statements extended with jq-style selectors as a policy language.

    and

    Selector syntax is closely based on jq's "filters".

After reading through this section, it seems like we should be able to say that the selector grammar is a proper subset of the jq filter grammar.

πŸ€” Perhaps. jq operates on streams, and is more permissive in how it treats things like assertion violation (e.g. when a field is not present), which may not be appropriate for an access control context.

I have mixed feelings about the name selector as we're actually filtering the invocation's args and then applying the predicate.

Could you expand? I'm not certain that ours is filtering in the same sense as jq filters on streams (or in the sense that you filter on arrays or collections). We have things lilke nested quantification to handle these cases instead. Arguably ours are "lenses" or "coarse parsers", but in these relatively simple cases that's often used interchangeably with "selector". I'm happy to call them something like IPFS Lenses if that's helpful, but it may get confused with the more common bidirectional lenses or profunctor lenses (with setters).

If Optional isn't the correct word, perhaps we can pick another without "baggage"?

Sure, let's switch it to "Optional" βœ… (DONE)

  1. The delegation specification states that the ABNF included in the Policy language section is the "formal syntax". In my opinion, this ABNF should be suitable for use when generating a parser in your language of choice.

If helpful, we could also switch to IPLD Schema if that's better for this purpose than ANBF. I was trying to avoid IPFS Schema since it has its own set of design issues.

In its current state, the selector syntax is too permissive and it should be strengthened to limit selectors to those that are intended throughout the rest of the document. This is obviously impacted by the decisions about recommendations #1 and #3 above.

Yes, it does depend on the other design decisions for certain. I'm not as confident in the strategy of generating a parser directly since you'll need to reserialise the CBOR to JSON for this to work, right? That could be expensive on every request.

  1. I was going to compare my prototype policy code the the Rust implementation that was linked at the end of Brooklyn's IPFS Camp presentation (https://github.com/expede/rs-ucan.) Can someone provide the link to this code?

I'll flip the switch on the Rust implementation momentarily (DONE). It needs a few tweaks to come in line with your and Irakli's comments, but a version of it was being used right at the end of Fission.

Is it suitable to think of the Rust implementation as the RI?

We have two parallel implementations: JS and Rust.

expede commented 1 month ago

Any jq filter that doesn't begin with a . is effectively a constant and applying that filter to ANY input value will result in the filter constant being sent to the output.

Indeed, that's how the Rust code works.

#[test_log::test]
fn test_fail_missing_leading_dot() -> TestResult {
    let got = Selector::from_str("[22]");
    assert!(got.is_err());
    Ok(())
}

Will update the spec βœ…

expede commented 1 month ago

@smoyer64 I pushed the branch directly to the ucan-wg fork, which is world-visible: https://github.com/ucan-wg/rs-ucan/tree/v1.0-rc.1 Wasm doesn't work yet, and some tweaks are needed to bring up to the above changes in the spec

smoyer64 commented 1 month ago

I'm happy to concede that the deviations from strict compatibility with jq perhaps led the discussion in the wrong direction ... I hadn't seen #5 but that's exceedingly helpful in understanding the spec. There are examples of selectors that both pass the ABNF and that are invalid. One example that I can remember without consulting my notes is that, while the text states that .. is not allowed, code generated from the ABNF doesn't reject it. I'm still happy to do some fuzzing while considering #5.

Interesting! Can you expand on this?

This example from the jq manual works with and without the optional operator: https://jqplay.org/?q=.foo%3F&j=%7B%22notfoo%22%3A+true%2C+%22alsonotfoo%22%3A+false%7D

I'm not as confident in the strategy of generating a parser directly since you'll need to reserialise the CBOR to JSON for this to work.

Yes and I'm happy to see that @alanshaw has written a tokenizer/parser/matcher that works on the IPLD nodes.

I'll flip the switch on the Rust implementation momentarily (DONE).

Awesome! This will make it easier to decipher the spec - should I feed anything I think are ambiguities back into PRs?