Skip to content
Snippets Groups Projects

Miscellaneous RPC connect spec tweaks

Merged Nick Mathewson requested to merge nickm/arti:rpc-spec-updates into main
3 unresolved threads

These came up while I was implementing. Some of these reflect discussions that we've had.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
260 267 but we cannot read it due to an error other than `EACCES`, `ENOENT`,
261 268 etc.
262 269 - The connect file explicitly tells us to abort.
263
264 TODO RPC These are still TBD; are they "decline" or "abort"?
265
266 - A filename within a connect point
267 contains a `${VARIABLE}` that cannot be expanded.
270 - The specified address for connect file,
271 or a file that the connect point mentions,
272 contains a misformed shell expansion string, like `unix:${foo`.
273 - A `socket` IP address in a connect point is not localhost.
268 274 - A filename within a connect point is not absolute.
  • Nick Mathewson
    Nick Mathewson @nickm started a thread on an outdated change in commit ab4b21e6
  • 260 267 but we cannot read it due to an error other than `EACCES`, `ENOENT`,
    261 268 etc.
    262 269 - The connect file explicitly tells us to abort.
    263
    264 TODO RPC These are still TBD; are they "decline" or "abort"?
    265
    266 - A filename within a connect point
    267 contains a `${VARIABLE}` that cannot be expanded.
    270 - The specified address for connect file,
    271 or a file that the connect point mentions,
    272 contains a misformed shell expansion string, like `unix:${foo`.
    273 - A `socket` IP address in a connect point is not localhost.
  • added 1 commit

    • 3cdec908 - rpc-connect-sketc: Add question on TOML tolerance.

    Compare with previous version

  • added 1 commit

    • f9a7dbce - rpc-connect-sketch: Per discussion, change non-localhost to "decline"

    Compare with previous version

  • Nick Mathewson
    Nick Mathewson @nickm started a thread on commit 3cdec908
  • 287 287 Unrecognized TOML tables and members in a connect point
    288 288 must be ignored.
    289 289
    290 <!-- TODO:
    291 Should RPC servers insist on understanding all tables and members?
    292 It seems that for them the consequences of not understanding the options
    293 are more serious.
    294 -->
    • Comment on lines +290 to +294

      @Diziet Any thoughts on this?

      It seems to me that if we allow an RPC server to ignore unrecognized tables and members, it might ignore some new member that is supposed to create an access restriction.

    • If we have the server reject them, we're committing to more limited compatibility: old clients connecting to new servers is always fine, but new clients connecting to old servers only works right if the connect configuration is changed along with the server.

      An example of what we would be losing is the ability to have the following scenario:

      • The global system configuration is set up to request some new feature X
      • But the system-installed arti does not understand X.
      • The intent is that either that system arti, or a possible user-installed arti which does understand X, might be started.

      More broadly, we'd be saying that the configuration must be provided by the same people as providing the server implementation. (Not necessarily the same Unix user, but the same humans or the same ansible config or whatever.)

      I think that's OK, but I'm not 100% sure.

      More generally, I'm uncomfortable with this rule. It goes against usual protocol design principles. Possibly further discussion of the implications could reassure me.

      The alternative (let's call it option 2) is either (i) the default access control settings must be minimal or (ii) access control settings are in the main RPC server config, not the connect strings.

      Of these I think option 2(ii) is my current favourite.

    • Okay. I am then going to add a note to this section saying that we cannot ever add any new "critical" members to an existing connect-point type.

    • (I'll do that in a separate MR so I can merge this one.)

    • Please register or sign in to reply
  • Nick Mathewson requested review from @Diziet

    requested review from @Diziet

  • Nick Mathewson mentioned in commit 5f19e6be

    mentioned in commit 5f19e6be

  • Nick Mathewson mentioned in merge request !2628 (merged)

    mentioned in merge request !2628 (merged)

  • Please register or sign in to reply
    Loading