-
Notifications
You must be signed in to change notification settings - Fork 47
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
Security improvements #454
Open
Gee-64
wants to merge
10
commits into
GideonZ:u64ii
Choose a base branch
from
Gee-64:feature/security-improvements-u64ii
base: u64ii
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Configuration settings of this type are not shown while browsing the configuration (unless being edited). The password fields are redacted when reading configuration through the API, but can be written just like before.
- Rename LAN config settings from "Network settings" to "Ethernet Settings". - Create a new "Network Settings" menu item for stuff that is not really related to the interfaces (Ethernet, WiFi). - Move the hostname setting from the Ethernet / Wifi settings menu to the new menu item. - Increase max hostname length to 31 chars to make room for a default unique hostname consisting of a product name plus a partial mac, like for example "Ultimate-64-Elite-II-12D3E1" (27 chars).
The "ident" server running on UDP port 64 is used to discover Ultimate devices on the local network. If a JSON response is sent to the client, a new boolean field "requires_authentication" is included to indicate if future communication on the "dma" TCP port 64 should start with a SOCKET_CMD_AUTHENTICATE (0xFF1F) command. The argument to this new command is simply a password, and the response to the client is single byte (0x00 for password incorrect, 0x01 for password correct). An incorrect password will cause the Ultimate to close the connection after a brief delay (to slow down password brute forcing).
The web API under http://<ultimate-ip>/v1/ is now optionally password protected. The password should be supplied using a 'X-Password' HTTP header. Example use: curl --header "X-Password: my-secret-password" http://192.168.2.64/v1/version To determine if a password is needed the client should do a GET on /v1/version (without a password) and if the HTTP response status code is 401 or 403 then a password is required. The HTML page served by the Ultimate web server does not yet support entering a password.
- Rename make_get_request() to make_binary_get_request() (response is binary data) - Create make_get_request() (response is JSON data) - On startup make a request to /v1/info to determine if a password is required - Add a login page and a logout entry in the left side menu - The top banner now shows the correct product type (uses /v1/info response) - Fully backwards compatible with older firmware versions without password support
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is intended to allow somewhat secure usage of Ultimate products in a "party setting" with lots of random people on the same LAN/WIFI. It adds (optionally configured) password protection to all network services, as well as the possibility to disable services individually. By default everything is enabled and no password is set (i.e fully backwards compatible). Some code refactoring was done to try and keep things clean.
The PR is a pretty big, but split into separate commits to allow reviewing each one on its own. Happy to change how things are done or break the PR up into separate PRs if that helps, just let me know what you want. Please see it as a starting point for discussion.
I know work for the new U64E2 is top priority so I would fully understand if this particular PR is deferred until U64II is merged / released.
Note: This PR is toward the u64ii branch. The reason for targeting this branch is that the needed refactoring touches the same code as the u64ii branch does and would cause headaches/conflicts when u64ii is merged. I do have a branch which is mergable toward master if that is really preferred, but I doubt it.
The first commit is a "cleanup" commit that centralizes all "product"-related stuff, avoiding code duplication.
The remaining commits can be summarized as this:
Issues that this PR relates to:
#441
#429 (though it looks like WiFi passwords are moved to the WiFi module I think?)
#406 (added separate info endpoint in order to not pollute the API version endpoint)
Edit: If anyone wants to try this out as it looks right now there are "softbootable" versions of the firmware to try over at
https://github.com/Gee-64/1541ultimate/releases