Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[enhancement]: Add universe-server configuration option to enable client access without authentication #390

Closed
dstadulis opened this issue Jul 10, 2023 · 4 comments · Fixed by #393 or #398
Assignees
Labels

Comments

@dstadulis
Copy link
Collaborator

dstadulis commented Jul 10, 2023

Background


#367 requests to serve Universe stats with minimum prerequisites. E.g. Enable a universe server to respond to queries without requiring the client / user authenticate to the universe server:

add a config flag which allows anyone to access [(QueryAssetStats and UniverseStats)] RPC methods without needing to supply a macaroon

As a universe operator I would like to respond to requests without requiring authentication from querying clients in order to make access to my service easily available without the need to identification / user friction

As a universe operator I would like to respond to requests without requiring authentication from querying clients in order to not require Terminal users to be logged in to query Universe stats

Deliverables


  1. Add universe config flag to reduce the server's required client authentication for (QueryAssetStats and UniverseStats) gRPC calls
    Relevant additions to list of RPCs will be made here:

    // macaroonWhitelist defines methods that we don't require macaroons to
    // access. We also allow these methods to be called even if not all
    // mandatory middlewares are registered yet. If the wallet is locked
    // then a middleware cannot register itself, creating an impossible
    // situation. Also, a middleware might want to check the state of tapd
    // by calling the State service before it registers itself. So we also
    // need to exclude those calls from the mandatory middleware check.
    macaroonWhitelist map[string]struct{}

    which was delivered in commit 1660aa7

  2. Consider a holistic design / more comprehensive set, of the configuration flags if other, unspecified-in-this-issue, calls or services might also benefit from not needing authentication.

Alternative solutions


bake a custom macaroon with limited access to these RPCs only and hard-code that into the Terminal app

Advantages:

  • Universe server could deny simple/naive resource exhaustion / DoS attempts

Disadvantages:

  • Changes necessary to Terminal
  • maintaining the hard-coded macaroons
  • less private asset-retrieval requests
@ffranr
Copy link
Contributor

ffranr commented Jul 10, 2023

I've created this PR which should achieve the desired behaviour. It doesn't yet include any new config flags.

@ffranr
Copy link
Contributor

ffranr commented Jul 10, 2023

I feel like the right solution here is to add a macaroon-whitelist command line argument.
And then the user could just add:

tapd ... --macaroon-whitelist=QueryAssetStats --macaroon-whitelist=UniverseStats

Or a config file. Do we have precedent for this in LND? @guggero ?

@guggero
Copy link
Member

guggero commented Jul 11, 2023

I don't think we should do that, as users tend to shoot themselves in the foot with security related features like this. Macaroons are a bit tricky to handle, especially for new users. So if they see a way to disable authentication on all endpoints, they will do it, not being aware of the security implications. So I think we should only have a config option that is reasonably safe, for example --allow-public-stats that then white lists just the "safe" endpoints (that only return statistics/proofs but no wallet balances and similar data).

@ffranr
Copy link
Contributor

ffranr commented Jul 12, 2023

Re-opening this whilst I'm working on a PR for adding --allow-public-stats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ✅ Done
3 participants