New flags to listen to all interfaces #495

chevdor · 2018-08-03T15:21:56Z

The current version listens only on 127.0.0.1.

This change is required for containerised versions to run properly.

Adds:

--ws-external
--rpc-external

USAGE:
    polkadot [FLAGS] [OPTIONS] [SUBCOMMAND]

FLAGS:
        --dev             Run in development mode; implies --chain=dev --validator --key Alice
    -h, --help            Prints help information
        --light           Run in light client mode
        --no-telemetry    Should not connect to the Polkadot telemetry server (telemetry is on by default on global
                          chains)
⥤    --rpc-external    Listen to all rpc interfaces (Default is local)
    -t, --telemetry       Should connect to the Polkadot telemetry server (telemetry is off by default on local chains)
        --validator       Enable validator mode
    -V, --version         Prints version information
⥤    --ws-external     Listen to all web socket interfaces (Default is local)

<...>

gavofyork · 2018-08-03T17:37:19Z

opening the RPC to external hosts by default is a little too promiscuous for my liking. i think this will have to be fixed properly.

chevdor · 2018-08-03T17:44:20Z

Yes, I can understand.
If we go safety first, we will have less nodes (missing out all those docker nodes).
If we take this fix without #494 we increase the risks. I think this is however acceptable for a PoC stage with only a testnet. I would gladly take a look at #494 but I have no idea how long it would take me, I have no clue how this was done in parity.

chevdor · 2018-08-03T17:46:40Z

--dev already carries a meaning. Would a generic --unsafe mode be an acceptable way to pack tweaks like this one?

So secure by default, only the local interface but open with --unsafe.

gavofyork · 2018-08-04T05:44:14Z

Should just be:

add a couple of value-taking options in cli.yml (--ws-interface, --rpc-interface);
in cli/src/lib.rs, use .unwrap_or("127.0.0.1:9933") to give them the current default, then pass on into the config as before.

chevdor · 2018-08-04T16:54:22Z

Ok I will take care of that.

tomusdrw · 2018-08-05T10:57:52Z

Maybe we can try to avoid too many configuration flags for RPC servers for now? Can live with --expose or --external-rpc flag that sets the interfaces to 0.0.0.0? I'd be in favor of something more clever for the future (either the proxy or configuration files). I don't think that selecting a specific interface to listen on is super useful anyway - usually it's just localhost or "external/all"

gavofyork · 2018-08-05T12:05:04Z

fair enough - should make it even easier :)

chevdor · 2018-08-07T07:23:51Z

I went with the naming suggested by @tomusdrw but flipped it. That allows having all the rpc related option as --rpc-foobar

tomusdrw · 2018-08-07T07:34:27Z

substrate/cli/src/cli.yml

+      long: rpc-external
+      help: Listen to all rpc interfaces (Default is local)
+      takes_value: false
+  - ws-external:


I believe there is no point in running RPC externally but not WS, would merge the two flags to avoid clutter in CLI help.
In the future we will most likely run HTTP & WS transport on the same port anyway, so maybe it will be a good opportunity to merge the two options as well.

So I guees it's all right :)

I did not want to be too pushy here and let for now the users decide. That allows at the moment a user to 'open up' WS while keeping RPC only local only. It may be better security-wise for now.

tomusdrw · 2018-08-07T07:35:42Z

substrate/cli/src/lib.rs

@@ -280,7 +280,7 @@ where
 			None => 30333,
 		};

-		config.network.listen_address = Some(SocketAddr::new("0.0.0.0".parse().unwrap(), port));
+		config.network.listen_address = Some(SocketAddr::new("127.0.0.1".parse().unwrap(), port));


Why this was changed? I think network should always listen externally.

Wuuhooo, sorry, this is a big mistake on my hand, good catch and sorry about that. Will fix.

Fix paritytech#439 Fix paritytech#494

* master: README: fixed typo in docker run command (#518) Merge *_at methods. (#515) New flags to listen to all interfaces (#495) If contract reaches max depth, return Err (#503) Some networking cleanups (#504) Derivable Encode & Decode (#509) substrate: return Option in all storage related RPC methods (#510) Build with locked Cargo.lock on CI (#514) Place call data into a newly allocated pages (#502)

* Add new flag to allow listening to all interfaces Fix #439 Fix #494 * Fix capitalisation

* examples: Use tokio instead of std async Signed-off-by: Alexandru Vasile <[email protected]> * test-runtime: Use tokio instead of std async Signed-off-by: Alexandru Vasile <[email protected]> * subxt: Use tokio instead of std async Signed-off-by: Alexandru Vasile <[email protected]> * examples: Use only necessary tokio features Signed-off-by: Alexandru Vasile <[email protected]>

chevdor added the A0-please_review Pull request needs code review. label Aug 3, 2018

chevdor force-pushed the will-439-network-interface branch from c991fe0 to cfeaccf Compare August 3, 2018 15:27

chevdor mentioned this pull request Aug 3, 2018

Json RCP access fails through Docker container #439

Closed

chevdor requested a review from tomaka August 3, 2018 16:55

gavofyork added A4-gotissues and removed A0-please_review Pull request needs code review. labels Aug 3, 2018

chevdor force-pushed the will-439-network-interface branch from cfeaccf to 0f8dd16 Compare August 6, 2018 16:48

chevdor changed the title ~~Listen to all interfaces by default~~ New flags to listen to all interfaces Aug 6, 2018

chevdor force-pushed the will-439-network-interface branch from 0f8dd16 to b562955 Compare August 6, 2018 16:50

tomusdrw approved these changes Aug 7, 2018

View reviewed changes

tomusdrw suggested changes Aug 7, 2018

View reviewed changes

Add new flag to allow listening to all interfaces

2aabe80

Fix paritytech#439 Fix paritytech#494

chevdor force-pushed the will-439-network-interface branch from b562955 to 2aabe80 Compare August 7, 2018 08:32

chevdor added the A0-please_review Pull request needs code review. label Aug 7, 2018

Fix capitalisation

f1fca90

gavofyork approved these changes Aug 8, 2018

View reviewed changes

gavofyork removed the A4-gotissues label Aug 9, 2018

gavofyork merged commit 620adb9 into paritytech:master Aug 9, 2018

chevdor deleted the will-439-network-interface branch August 9, 2018 07:39

gavofyork pushed a commit that referenced this pull request Aug 10, 2018

New flags to listen to all interfaces (#495)

2355463

* Add new flag to allow listening to all interfaces Fix #439 Fix #494 * Fix capitalisation

lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020

service: cleanup task spawning (paritytech#495)

ebf7ee2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New flags to listen to all interfaces #495

New flags to listen to all interfaces #495

chevdor commented Aug 3, 2018 •

edited

Loading

gavofyork commented Aug 3, 2018

chevdor commented Aug 3, 2018

chevdor commented Aug 3, 2018

gavofyork commented Aug 4, 2018

chevdor commented Aug 4, 2018

tomusdrw commented Aug 5, 2018

gavofyork commented Aug 5, 2018

chevdor commented Aug 7, 2018

tomusdrw Aug 7, 2018

chevdor Aug 7, 2018

tomusdrw Aug 7, 2018

chevdor Aug 7, 2018

New flags to listen to all interfaces #495

New flags to listen to all interfaces #495

Conversation

chevdor commented Aug 3, 2018 • edited Loading

gavofyork commented Aug 3, 2018

chevdor commented Aug 3, 2018

chevdor commented Aug 3, 2018

gavofyork commented Aug 4, 2018

chevdor commented Aug 4, 2018

tomusdrw commented Aug 5, 2018

gavofyork commented Aug 5, 2018

chevdor commented Aug 7, 2018

tomusdrw Aug 7, 2018

Choose a reason for hiding this comment

chevdor Aug 7, 2018

Choose a reason for hiding this comment

tomusdrw Aug 7, 2018

Choose a reason for hiding this comment

chevdor Aug 7, 2018

Choose a reason for hiding this comment

chevdor commented Aug 3, 2018 •

edited

Loading