biemster / FindMy

Query Apple's Find My network
346 stars 53 forks source link

Code Improvement and Refactor #38

Closed Chapoly1305 closed 11 months ago

Chapoly1305 commented 11 months ago

Dear author of project and maintainers, I have successfully run this project with docker version of anisette and used raspberry pi to send the broadcast. To make the project easier for new comers, improved the flexibility for other projects or service, and more secure, I have made the following changes. Please help to review, thanks.

  1. Secure method of private key generation. The use of random to generate private key is dangerous. The better practice is to use secure library for generating cryptographic secure random number (key).
  2. The original SQL insertion query is dangerous, mitigated by using precompiled statement.
  3. Clarify variable naming on generate_keys.py
  4. The adv seems to be the public key numbers, and hash is the sha256 then base64 of the public key. Refactored the names improve clarity and matching the concepts (Public Key, Private Key, AdvertisementKey) used in openhaystack on generate_keys.py.
  5. Add notice for authentication for GSA service.
  6. Catch the password error message instead of just crashing.
  7. Add details into README document. Removed the SQL table creation step and make it automatically create table if it's not existed in database.
  8. A few formatting improvement suggested by PyCharm.
  9. Modify the report database to save lat and lon so that can be used by other application or web services.

The modification have been validated on Linux VPS of mine without any issue.

image image
Chapoly1305 commented 11 months ago

Following comment https://github.com/biemster/FindMy/pull/36#issuecomment-1838412001 @biemster thanks for the feedback, I fully understand your project goal. For security consideration, please review and consider to mitigate the security issues below,

  1. Insecure method of private key generation. https://github.com/biemster/FindMy/blob/4f124266017c1187f47bf4cd4a6a8e7a990c3614/generate_keys.py#L25
  2. Insecure method of SQL query insertion. https://github.com/biemster/FindMy/blob/4f124266017c1187f47bf4cd4a6a8e7a990c3614/request_reports.py#L86

Much appreciated.

biemster commented 11 months ago

Thanks for this, I'm going to think about it.