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

[Feature request]: Network Key and Pan ID / Extended Pan ID auto generated during install to minimize insecure installs #16188

Closed
bharvey88 opened this issue Jan 15, 2023 · 34 comments
Labels
feature request Feature request

Comments

@bharvey88
Copy link

bharvey88 commented Jan 15, 2023

Is your feature request related to a problem? Please describe

While learning more about zigbee and z2m in the home assistant discord server I have been made aware that my install is far from secure. Per: https://www.zigbee2mqtt.io/advanced/zigbee/03_secure_network.html#change-zigbee-network-encryption-key it says the install uses a default encryption key and pan ID and extended pan ID. It also states we are unable to generate the key ourselves when using the addon inside home assistant OS. I find this default install behavior to be very worrying and I request that there be a way to automatically generate both a random network key and pan ID to make this a noob-friendly installation experience.

Describe the solution you'd like

Automatically generate randomized network key and pan ID and extended pan ID during installation process.

Describe alternatives you've considered

Alternatively, a required set of options for both network key and pan ID and extended pan ID in the configuration tab with accompanying documentation in the documentation tab of the addon.

Additional context

No additional context to give, let me know if you want any added information.

Stale bot closed this issue and I was unable to reopen so I made a new one: #14868

@bharvey88 bharvey88 added the feature request Feature request label Jan 15, 2023
@bharvey88 bharvey88 changed the title [Feature request]: Network Key and Pan ID auto generated during install to minimize insecure installs [Feature request]: Network Key and Pan ID / Extended Pan ID auto generated during install to minimize insecure installs Jan 15, 2023
@eikowagenknecht
Copy link

I found that while the docs say it can't be done when using the HA addon, it's not that hard (or I'm missing something).

Warning: You need to repair all devices after you do that!!

  • Shutdown zigbee2mqtt addon
  • Open \<homeassistant_ip>\config\zigbee2mqtt\configuration.yaml
  • set network_key and ext_pan_id like so:
    advanced:
        network_key:
        - 1
        - 2
        - 3
        - 4
        - 5
        - 6
        - 7
        - 8
        - 9
        - 10
        - 11
        - 12
        - 13
        - 14
        - 15
        - 220
      ext_pan_id:
        - 1
        - 2
        - 3
        - 4
        - 5
        - 6
        - 7
        - 221
    
    Obviously use other values for each entry (betweed 1 and 221 should be safe).
  • Remove coordinator_backup.json
  • Restart and repair all your devices

@bharvey88
Copy link
Author

I found that while the docs say it can't be done when using the HA addon, it's not that hard (or I'm missing something).

Warning: You need to repair all devices after you do that!!

* Shutdown zigbee2mqtt addon

* Open \<homeassistant_ip>\config\zigbee2mqtt\configuration.yaml

* set network_key and ext_pan_id like so:
  ```
  advanced:
      network_key:
      - 1
      - 2
      - 3
      - 4
      - 5
      - 6
      - 7
      - 8
      - 9
      - 10
      - 11
      - 12
      - 13
      - 14
      - 15
      - 220
    ext_pan_id:
      - 1
      - 2
      - 3
      - 4
      - 5
      - 6
      - 7
      - 221
  ```
  
  
      
        
      
  
        
      
  
      
    
  Obviously use other values for each entry (betweed 1 and 221 should be safe).

* Remove coordinator_backup.json

* Restart and repair all your devices

This is useful but I created this issue with the hopes of changing the default install behavior of z2m to be a secure install not using known keys, not just the ability to change settings after install.

@eikowagenknecht
Copy link

Sure, that would be even better 👍

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale issues label Mar 2, 2023
@bharvey88
Copy link
Author

bharvey88 commented Mar 2, 2023 via email

@junosuarez
Copy link

I agree this is an important issue. Insecure defaults are one of the most common category of security vulnerabilities in systems design. Most people setting up a zigbee network are looking to get started as quickly as possible and have little knowledge at the beginning. However, by the time they learn about this problem, they may have invested time and effort in setting up a large network with many devices - having to re-pair each of them creates a lot of friction which may lead to continuing with the insecure default configuration.

The proposed solution of random key generation at installation time is a good one. Let's please keep in mind the various ways that people first install or run zigbee2mqtt. In particular, Docker or other declarative, minimal configuration setups.

Further, I think this warrants showing a warning in the zigbee2mqtt web UI with a link to instructions on how to improve network security.

Are there any constraints, considerations, or concerns from the project maintainers?

@HelpITru
Copy link

HelpITru commented Mar 2, 2023

This is a huge security hole. And a lot of time spent looking for problems. I have three neighbors with default settings and devices walk between networks. I'm the only smart one who will change the panid soon. And they don't know and will suffer until I tell them what to do.

@HelpITru
Copy link

HelpITru commented Mar 2, 2023

after all , after changing the PanID , you still need to repaired all devices !!! and at the first initial installation, there are no linked devices yet. Thanks!

@github-actions github-actions bot removed the stale Stale issues label Mar 3, 2023
@bharvey88
Copy link
Author

bump before the bot gets here.. still hoping to have this looked at by @Koenkk

@xelemorf
Copy link

bump

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale issues label May 19, 2023
@bharvey88
Copy link
Author

bharvey88 commented May 19, 2023 via email

@github-actions github-actions bot removed the stale Stale issues label May 20, 2023
@bharvey88
Copy link
Author

dont even think about it bot. still open!

@sjorge
Copy link
Contributor

sjorge commented Jun 6, 2023

There is actually already support to generate a random network key by setting it to GENERATE. This was requested a few times over the years and I believe the main hurdle is that by setting it to in data/configuration.yaml in the repository it would break most (all?) existing installs.

I vaguely remember there being talk about moving data/configuration.yaml to data/configuration.yaml.example and copying it to data/configuration.yaml if it's missing... but I don't remember what happened to it as I can't find the ticketissue. It was at least a few years ago.

Might have been a mix of different issues and PRs:

@lyricnz
Copy link

lyricnz commented Jun 21, 2023

Yes, please implement random default; using a preconfigured static value is dumb.

@spudwebb
Copy link

spudwebb commented Jun 21, 2023

There is actually already support to generate a random network key by setting it to GENERATE. This was requested a few times over the years and I believe the main hurdle is that by setting it to in data/configuration.yaml in the repository it would break most (all?) existing installs.

Looks like you can set pan_id and network_key to GENERATE but not ext_pan_id. Is there a reason for that?

@bharvey88
Copy link
Author

continuing to hope for a reply from the maintainer here

@Koenkk
Copy link
Owner

Koenkk commented Jul 1, 2023

I'm fine with doing this, but the thing holding me back is that the people who run a git clone installation will get a merge conflict on their configuration.yaml.

@bharvey88
Copy link
Author

I'm fine with doing this, but the thing holding me back is that the people who run a git clone installation will get a merge conflict on their configuration.yaml.

I understand your hesitation but if this is increasing security for all new installs I believe it warrants a breaking change for the minority of people using git clone.

@Koenkk
Copy link
Owner

Koenkk commented Jul 5, 2023

I will check if I can implement something non-breaking.

@attila-lendvai
Copy link

attila-lendvai commented Jul 15, 2023

IIRC, it's possible to add a file to .gitignore, and also have it committed to the repo using git add -f -- if you want to accommodate a usecase where the read-write data storage file is also added to the git repo, that is. i wouldn't. this is just one example of the many ways it can lead to headaches.

also, people who run it from a git checkout (me included) are expected to be savvy enough to deal with breaking changes.

but now i'll need to disassemble a couple of wall sockets to properly secure my network, which is certainly more headache than a dirty git diff in a checkout.

@Koenkk
Copy link
Owner

Koenkk commented Jul 18, 2023

I've made a PR to fix this issue (without introducing a breaking change). Please let me know if this is what you expect: #18357

@spudwebb
Copy link

your PR generates a random network_key, but what about pan_id and ext_pan_id don't we need those to be randomly generated as well?

@MattWestb
Copy link

MattWestb commented Jul 19, 2023

Not needed if only having one 15.4 network in radio range but if not they must being unique or you is getting conflicts and strange problems in some cases.
I think the best is also randomizing this 2 for getting it working without problems for all user cases.

@bharvey88
Copy link
Author

your PR generates a random network_key, but what about pan_id and ext_pan_id don't we need those to be randomly generated as well?

Yes I agree, I opened this issue to solve the problems of an insecure install due to all three of these (network_key, pan_id, ext_pan_id) not just network_key. This came to light after realizing zha does randomize all of these things during first install. I will also note that zha does an initial energy scan and chooses the best channel for you, which frankly I also think is a great QoL feature.

@MattWestb
Copy link

ZHA is also warning if using Z2M standard network setting that is also good made and the energy scan is also good if having problems but its normally no danger if getting one warning then i have my production network over lapping with my WiFi and is normally not having problems but is getting wearing at most start of ZHA.

@Koenkk
Copy link
Owner

Koenkk commented Jul 19, 2023

pan_id/ext_pan_id has nothing to do with security (see #18357 (comment))

@MattWestb
Copy link

pan_id/ext_pan_id has nothing to do with security (see #18357 (comment))

So you can also see it but in in Zigbee and other 802.15.4 network its one part of the security configuration.
Also you is getting mush problems then users is setting up more Z2M systems in radio range and getting strange problems in there systems then all is badly configuration from start.

Koenkk added a commit that referenced this issue Aug 3, 2023
* fix: Add secure default config. #16188

* Enable frontend by default
@Koenkk
Copy link
Owner

Koenkk commented Aug 12, 2023

Closing this now as network_key, ext_pan_id an pan_id are now generated for new installations.

@Koenkk Koenkk closed this as completed Aug 12, 2023
@louis-lau
Copy link

@Koenkk docker install instructions told me to download https://raw.githubusercontent.com/Koenkk/zigbee2mqtt/master/data/configuration.yaml, which does not have the values set to GENERATE. Does that mean no secure defaults for new docker installations?

@Impact123
Copy link
Contributor

@louis-lau
Copy link

Perhaps the instructions need updating in that case

@Koenkk
Copy link
Owner

Koenkk commented Oct 4, 2024

@louis-lau would you mind making a PR to update the instructions?

@louis-lau
Copy link

louis-lau commented Dec 15, 2024

Took me much longer than it should have (life), but see Koenkk/zigbee2mqtt.io#3329 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

No branches or pull requests