evcc-io / evcc

Solar Charging ☀️🚘
https://evcc.io
MIT License
3.67k stars 687 forks source link

Masking off secrets on failed startup page #4978

Closed tarelda closed 2 years ago

tarelda commented 2 years ago

Is your feature request related to a problem? Please describe. When EVCC fails to starts it shows the running config file. Currently only ip addresses are masked with stars (*).

Describe the solution you'd like I would like my vehicle tokens and idtag to be masked off too.

Describe alternatives you've considered Access to EVCC is restricted to local network, but still I don't think these should be in cleartext display.

Additional context

premultiply commented 2 years ago

Please try recent nightly build. There should be no real private information.

tarelda commented 2 years ago

Please try recent nightly build. There should be no real private information.

Version: 0.105.2 25ec8591 2022-10-29_11:56:44 go build -v -tags=release -ldflags='-X github.com/evcc-io/evcc/server.Version=0.105.2 -X github.com/evcc-io/evcc/server.Commit=25ec8591 -s -w'

Zrzut ekranu z 2022-10-29 14-13-43

25ec8591 is based on 59e5ad767caafda9b21e84c3406df656f8637e65

goebelmeier commented 2 years ago

RFC 1918 IP addresses are not worth to be masked, they can't reveal anything and can't be misused. And combined with logs they help debugging typos.

premultiply commented 2 years ago

Ok, the tokens may be discussed to be hidden.

The others shall not be hidden.

andig commented 2 years ago

Happy to take. Pr against root.go/helper.go for the tokens. Good catch! We should also check if there are different notations for those tokens?

tarelda commented 2 years ago

@andig while testing #4985 I noticed that something in c83e10996dd65c6a3bf0ffdbf9c1bc8b2123e2c8 broken present restart behaviour.

log.FATAL.Println(httpd.ListenAndServe()) never gets executed and process ignores SIGTERM and SIGINT.

andig commented 2 years ago

Think I‘ve fixed that yesterday already, please rebase.

tarelda commented 2 years ago

Think I‘ve fixed that yesterday already, please rebase.

I've already used c83e109 which is your "fix" commit. Today I repeated the test:

root@pingwin:/evcc# make && ./evcc --log ocpp:TRACE,DEBUG Version: 0.105.2 f2f2edfc 2022-10-30_16:08:26 go build -v -tags=release -ldflags='-X github.com/evcc-io/evcc/server.Version=0.105.2 -X github.com/evcc-io/evcc/server.Commit=f2f2edfc -s -w' github.com/evcc-io/evcc/util/modbus github.com/evcc-io/evcc/provider github.com/evcc-io/evcc/detect/tasks github.com/evcc-io/evcc/detect github.com/evcc-io/evcc/vehicle/ford github.com/evcc-io/evcc/meter/lgpcs github.com/evcc-io/evcc/vehicle/fiat github.com/evcc-io/evcc/vehicle/bluelink github.com/evcc-io/evcc/vehicle/jlr github.com/evcc-io/evcc/vehicle/bmw github.com/evcc-io/evcc/charger github.com/evcc-io/evcc/vehicle/mercedes github.com/evcc-io/evcc/meter github.com/evcc-io/evcc/vehicle/nissan github.com/evcc-io/evcc/vehicle/porsche github.com/evcc-io/evcc/vehicle/psa github.com/evcc-io/evcc/vehicle/renault github.com/evcc-io/evcc/vehicle/seat/cupra github.com/evcc-io/evcc/vehicle/skoda github.com/evcc-io/evcc/vehicle/smart github.com/evcc-io/evcc/vehicle/vw github.com/evcc-io/evcc/vehicle/vw/id github.com/evcc-io/evcc/core github.com/evcc-io/evcc/vehicle github.com/evcc-io/evcc/hems github.com/evcc-io/evcc/cmd/configure github.com/evcc-io/evcc/cmd github.com/evcc-io/evcc [main ] INFO 2022/10/30 17:08:32 evcc 0.105.2 (f2f2edfc) [main ] INFO 2022/10/30 17:08:32 using config file: /etc/evcc.yaml [main ] INFO 2022/10/30 17:08:32 listening at :7070 [ocpp ] DEBUG 2022/10/30 17:08:33 waiting for chargepoint: 30s [main ] FATAL 2022/10/30 17:09:03 cannot create charger 'carport-wallbox': cannot create charger 'template': cannot create charger 'ocpp': timeout [main ] FATAL 2022/10/30 17:09:03 will attempt restart in: 30s ^C^C^C^C^C^C^C^C^C^C^C^C root@pingwin:/evcc#

As you can see, I pressed Ctrl+C (SIGTERM) multiple times and process didn't stop. And there was no error page.

Diff between my tree and current master:

$ git diff diff --git a/cmd/root.go b/cmd/root.go index 64ca03b6..52470522 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -25,7 +25,7 @@ import ( "github.com/spf13/viper" )

-const rebootDelay = 5 time.Second // delayed reboot on error +const rebootDelay = 30 time.Second // delayed reboot on error

var ( log = util.NewLogger("main")

I increased rebootDelay to have time to press Ctrl+C and refresh browser to see if there is error page.

andig commented 2 years ago

@tarelda could you give https://github.com/evcc-io/evcc/pull/4992 a try? 3 changes:

Concurrency is hard- only way really is to have test cases which is again hard for this part.

tarelda commented 2 years ago

@tarelda could you give #4992 a try? 3 changes:

  • revert the delay change, that was a mistake
  • start the shutdown listener (<-stopC) before the error handler or it will not catch channel closed
  • move waiting for rebootDelay outside the sync.Once or the shutdown handler or Ctrl-C won't be aber to trigger the sync.Once

Concurrency is hard- only way really is to have test cases which is again hard for this part.

I know, but #4946 was my first functional golang code :) That's why at least I try to help with testing.

With #4992 everything works except from http server, so I pushed my fix into my fork and referenced it in #4992.