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

Add option to disable random MAC address in client mode #403

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

yenon
Copy link

@yenon yenon commented Apr 7, 2024

To allow for easier access via WIFI, the changing mac address of the goggle will result in different IP's every boot, thanks to the dhcp on the network not recognizing the device by it's MAC.
This PR introduces a (semi) permanent MAC that can be used as a fixed address, if so desired.
The first address of the device is generated randomly from system time and saved to the settings_ini.
Changing of the address is also supported by placing a file named macaddr.txt in the SD-card root.
Validation is in place, and only valid mac's will be accepted.

Please validate this a bit more cautiously, my C skills are rusty.

Cleaned up error in comment, we want to avoid broadcast and multicast addresses.
@SumolX
Copy link
Contributor

SumolX commented Apr 22, 2024

Nice job! I would recommend not keeping both clientid and the option for random MAC address. Remove the clientid setting attribute and associated references, use the persistent Mac attribute instead. Simplify the code to always use the generate MAC address instead. I don't see a benefit for having the user switch between random and static MAC address and having extra UI options. While the file on the sd card is good for debugging and manually setting the Mac once you have verified this works is it necessary to keep the logic for loading a Mac from sd card? I would imagine most people won't know about this and probably just rely on the auto generation since you have solved the underlying dhcp ip reuse issue.

@yenon
Copy link
Author

yenon commented Apr 23, 2024

Before removing features, I'd like to know nobody actually uses them.
Secondly, what does clientid actually do? I cannot find a manpage for udhcpc that has the x flag, no idea what it does.
Removing random MAC completely might be bad for privacy in open networks, thanks to fingerprinting. (We are talking about a video goggle though.)

I see the point of not needing the MAC to be changed via SD card. Mostly just me wanting to set the default to what it used to be on my first try. Maybe in case of a settings file update?
I foresee no real changes to the way MAC addresses work in the wild, so the code would essentially remain untouched for a long time, so why not have it? How it works is actually described in the note at the bottom of the settings page.

@SumolX
Copy link
Contributor

SumolX commented Apr 23, 2024

clientid - https://techhub.hpe.com/eginfolib/networking/docs/switches/5130ei/5200-3942_l3-ip-svcs_cg/content/483572383.htm

So if you are going to keep random Mac for privacy reasons.... then the client id should be made random as well or it should be updated to use the MAC address as well regardless what mode the privacy is set to.

My argument for simplifying the MAC address handling was to minimize confusion and clutter within the UI and having an external text file used for overriding. The approach should be too keep it simple since at the end of the day this is not a full blown PC envirnoment. I know I don't use the googles to access the web for surfing.... ;)

Anyway those are my recommendations.... you can choose how to move forward.

@yenon
Copy link
Author

yenon commented Apr 26, 2024

Keeping the MAC static seems sane for me, especially with our usecase. The clientid can be the static mac address, just as a fallback in case of weird behavioured dhcp's. I'll drop the menu entry for changing it, that will already simplify a lot. I'd like to keep the mac changing by file, since we do not adhere to any reasonable standard for it's assignment and duplicates are unlikely, but not impossible. Sound good?

@yenon
Copy link
Author

yenon commented Apr 26, 2024

Should be alright now.

@yenon yenon marked this pull request as draft April 26, 2024 17:05
@nerdCopter
Copy link
Contributor

nerdCopter commented Aug 30, 2024

April 26 comment makes 100% sense to me as someone in the IT industry. Maybe rebase on main and mark ready for review.

@yenon
Copy link
Author

yenon commented Aug 30, 2024

It would make sense, if the used wifi driver actually supported it properly. The current driver seems to offer better performance, but lacks any means of changing the mac address, if I remember correctly. Feel free to pull and build this, but it will not work.

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

Successfully merging this pull request may close these issues.

3 participants