informalsystems / hermes

IBC Relayer in Rust
https://hermes.informal.systems
Apache License 2.0
449 stars 329 forks source link

RPC Authorization #1029

Open ebuchman opened 3 years ago

ebuchman commented 3 years ago

Crate

relayer-cli

Summary of Bug

RPC endpoints do not support some means of authentication.

The first is http based user/password in the url, for instance https://<user>:<password>@url:port. This is a pretty convenient way to secure access to nodes (we have some nodes set up like this) so it would be good if the *_addr fields in the hermes config were able to support this. Currently if I set the rpc_addr to this it fails with:

Jun 01 13:43:31.113  INFO ibc_relayer::event::monitor: starting event monitor chain.id=informal-testnet-1
Error: chain runtime error: RPC error to endpoint https://<user>:<password>@rpc.interchain.io/: Parse error. Invalid JSON: expected value at line 1 column 1 (code: -32700)

A related point (since this kind of auth requires https), already noted in https://github.com/informalsystems/ibc-rs/issues/877, is that the grpc_addr does not support https.

Another kind of authentication is that used by eg. https://datahub.figment.io/ where you are given an API key. In that case, the API key can be easily added to the rpc_addr and the websocket_addr URLs, but there is no way to add it to the grpc_addr. For instance this works fine in the hermes config:

rpc_addr = 'https://cosmoshub-4--rpc--full.datahub.figment.io/apikey/<api key>'
websocket_addr = 'ws://cosmoshub-4--rpc--full.datahub.figment.io/apikey/<api key>'

but there is no way to make it work with the grpc_addr. According to the figment datahub docs the way to make this work is by setting an HTTP header field, eg. grpcurl -H "Authorization:<api key>" cosmoshub-4--grpc--archive.grpc.datahub.figment.io:443 list. So perhaps there could be some way to either set this specific Authorization header or to more generally set header fields for these urls ?

Note that the rpc/websocket addrs can also work with the -H "Authorization:<api key>" instead of including it in the url so persumably the API key could be included in the hermes config once for each chain and used for all the URLs for that chain.

That said it occurs to me I couldn't even test if the API key works in the grpc_addr because https doesn't even work there so we should fix that first since it's needed for everything.

Version

Latest

Steps to Reproduce

For the first, add a user/password to the any of the addr fields. For the second, try to connect to a figment data hub node

Acceptance Criteria

Hermes can talk to nodes protected by https along with a user/password or an Authorization API key


For Admin Use

faddat commented 1 year ago

Hi, I think that we are very happy to contribute this one :).

romac commented 1 year ago

Cool! We'd gladly accept a PR for this :)

On order to prioritize this properly, may I ask if you currently have a use case for this at Notional or have heard others mention a need for this recently?

faddat commented 1 year ago

@romac we want to integrate it with out API product, https://notionalapi.com

romac commented 1 year ago

Good to know, thanks! Let me put this back in a milestone then

faddat commented 1 year ago

@romac if I were going to break this down into tasks, you figure it might look like:

I am not super familiar with the hermes codebase, I have only made a few contributions, possibly just bumping the supported sdk version a few times.

@ebuchman -- afaik, we've got https on our grpc. I think that @baabeetaa can explain how we did that, but here is a rough map:

both notionalapi and cosmosia use cosmosia and its configs

(bunch of nodes that can both be scaled, and auto scale) -> caddy LP + https <-> hermes and other consumers

We want to "dogfood" this because relaying is our most latency sensitive operation. If our shared infra can equal the perf of the dedicated machines we can use for relaying now then:

Also in the present state we do have keys that can be appended to the ends of URLS for grpc, too. I think that in the end our approach just changes the path though.


Acceptance criteria

I was hoping that we could change the acceptence criteria a little to get this out the door faster. I think we just need the authorization api key.

antony-everstake commented 1 year ago

Authorization is a very important for grpc. I wanted to inquire whether the process is moving for add Authorization for GRPC? Are there plans to add GRPC authorization to the config file in new releases?

gilbahat commented 1 week ago

Hi,

hermes documentation asserts that http basic auth is supported. is this bug stale or is documentation incorrect? https://hermes.informal.systems/documentation/configuration/configure-hermes.html#connecting-to-a-full-node-protected-by-http-basic-authentication also, are we sure this applies to grpc as well? are there any unit tests for it?

gilbahat commented 1 week ago

as per diagnostic session in interchain discord, tonic does not support basic authentication out of the box. either documentation should be updated or support added for gRPC basic auth