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

Switched from mac address to machine id #1081

Merged
merged 1 commit into from
Mar 20, 2022
Merged

Switched from mac address to machine id #1081

merged 1 commit into from
Mar 20, 2022

Conversation

thomasjacquin
Copy link
Collaborator

The PR switches the unique ID from Mac Address to Machine ID.

This is to address the fact that in some cases, the RPi can have multiple interfaces enabled and there's a need to parse the output and select only one mac address. Furthermore, if the user switches from wlan0 to eth0, they would have a different mac address.

The Machine ID documentation can be found here: https://man7.org/linux/man-pages/man5/machine-id.5.html

Copy link
Collaborator

@AndreasLMeg AndreasLMeg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this should work - but not tested

pi@allsky:~ $ MACHINE_ID="$(cat /etc/machine-id)"
pi@allsky:~ $ echo $MACHINE_ID
6b5d3aff9e7742a0923061180d7be273
pi@allsky:~ $ digit="${MACHINE_ID: -1}"
pi@allsky:~ $ echo $digit
3

Does your php do security checks?
I fear there is a possibility of making your server unusable here - with a targeted call (curl) it is easy to make the database/map unusable.
In fact, I exploited this vulnerability.

@AndreasLMeg
Copy link
Collaborator

@thomasjacquin: I know I'm paranoid, but the urls should also be checked to see if there really is an allsky page behind them.
I hope only honest people will hang around here, but you can never be sure...

@thomasjacquin
Copy link
Collaborator Author

Good call @AndreasLMeg. I'm an eternal optimist so I'm always hoping things will go just fine :)
I can make sure to only allow one request per IP address per day to limit curl requests in a loop.
I can also check that the targeted website contains a certain file/variable. That should filter most of the requests.

@thomasjacquin thomasjacquin merged commit 35e0a70 into master Mar 20, 2022
@thomasjacquin thomasjacquin deleted the allsky-map branch March 20, 2022 20:52
@EricClaeys
Copy link
Collaborator

@thomasjacquin, @AndreasLMeg ,
The server could look for <websiteurl>/config.js and to be even safer, check that it contains imageName. And if you really want to be cool, check if camera: "some camera" is the same as the camera variable passed to the server.

@thomasjacquin
Copy link
Collaborator Author

@EricClaeys I was thinking about config.js but then I realized that some people post images to their website but don't always have the allsky-website code installed on it. Some users just post to their Wordpress page.

@EricClaeys
Copy link
Collaborator

@thomasjacquin Good point. Funny - I'm currently doing that so it would have failed with me!
I guess that means we should allow website_url to be null but image_url to contain something.

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.

None yet

3 participants