nerves-hub / nerves_hub_link

Connect devices to NervesHub via a Phoenix channel
https://hex.pm/packages/nerves_hub_link
Apache License 2.0
36 stars 18 forks source link

fwup_public_keys which is referred to by atom name doesn't work on 0.10.1 #78

Closed pojiro closed 3 years ago

pojiro commented 3 years ago

Describe the bug

If we use nerves_hub_link 0.10.1 with fwup_public_keys which is reffered to by atom name, the nerves cannot update its firmware.

The log is below,

22:42:59.835 [error] FWUP ERROR: Error decoding public key

22:42:59.843 [error] GenServer Fwup.Stream terminating
** (stop) {:exit, 1}
Last message: {#Port<0.226>, {:exit_status, 1}}
State: %{cm: #PID<0.1478.0>, port: nil}

To Reproduce

  1. create nerves device
$ git clone https://github.com/pojiro/nerves_hub_link_test
$ git checkout v0.1.0-nerves_hub_link_0.10.1
$ cd nerves_hub_link_test
# change ssh key config and fwup_public_keys(use atom name) in config/target.exs
$ export MIX_TARGET=[you have]
$ mix deps.get && mix deps.compile
$ mix nerves_hub.device create
$ mix nerves_hub.device cert create [device id]
$ mkdir rootfs_overlay/certs && cp nerves_hub/*.pem rootfs_overlay/certs
# change nerves_hub_link ssl config in config/target.exs, ssl: [certfile: /certs/[***].pem, keyfile: /certs/[***].pem]
$ mix firmware
$ mix nerves_hub.device burn [device id]
  1. publish firmware, create deployment and activate it
$ git checkout v0.1.1
$ mix firmware
$ mix nerves_hub.firmware publish --key [yourkey]
# then create deployment and activate it on nerves-hub or use api
  1. attach the nerves RingLogger

Expected behavior

It should also works with atom name.

Environment

$ mix nerves.env --info
==> nerves
==> hello_nerves_hub
|nerves_bootstrap| Environment Package List

  Pkg:         nerves_toolchain_armv7_nerves_linux_gnueabihf
  Vsn:         1.4.1
  Type:        toolchain
  BuildRunner: {Nerves.Artifact.BuildRunners.Local, []}

  Pkg:         nerves_system_br
  Vsn:         1.14.4
  Type:        system_platform
  BuildRunner: {nil, []}

  Pkg:         nerves_system_bbb
  Vsn:         2.9.0
  Type:        system
  BuildRunner: {Nerves.Artifact.BuildRunners.Local, []}

  Pkg:         nerves_toolchain_ctng
  Vsn:         1.8.2
  Type:        toolchain_platform
  BuildRunner: {nil, []}

|nerves_bootstrap| Loadpaths Start

Nerves environment
  MIX_TARGET:   bbb
  MIX_ENV:      dev

|nerves_bootstrap| Environment Variable List
  target:     bbb
  toolchain:  /home/pojiro/.nerves/artifacts/nerves_toolchain_armv7_nerves_linux_gnueabihf-linux_x86_64-1.4.1
  system:     /home/pojiro/.nerves/artifacts/nerves_system_bbb-portable-2.9.0
  app:        /home/pojiro/Sandbox/hello_nerves_hub

|nerves_bootstrap| Loadpaths End

I confirm this issue on rpi3 and bbb.

Additional context

fhunleth commented 3 years ago

@pojiro Could you debug the issue or figure out what line of code breaks this?

pojiro commented 3 years ago

Yes, I will try to dig this out.

pojiro commented 3 years ago

@fhunleth I figured out, this code breaks.

If fwup_public_keys includes atom name of key, the code makes fwup_public_keys become like [:devkey, "publick key binary"]. The mixed fwup_public_keys are used in here. So fwup_args include atom key, then it causes this issue.

I think,

  1. we don't need config.fwup_public_keys in that line, because NervesHubLink.Certificate.fwup_public_keys() resolves them.
  2. we need validation here.
  3. and need some tests, 'cause as we know, updating fw is very very important feature. I would not like to imagine this happens in production.

Thank you.

fhunleth commented 3 years ago

@pojiro Thanks for looking into this. See Jon's PR for a fix. He's planning on making a release today to avoid breaking any more projects.

pojiro commented 3 years ago

@fhunleth @jjcarstens I appreciate for your quick fix. I will polish my skills and would like to contribute and pay back to Nerves Project and you :)