Add Wifi Settings #91

dodoradio · 2023-06-01T12:11:34Z

does what it says on the tin, adds wifi settings. Allows scanning networks, logging in with username or password where needed. Has a dialog to show autoconnect, disconnect and remove buttons, along with the IP address for the given network.

Limitations:

doesn't show any 'connection failed' dialog when you fail to log in (i.e. no 'wrong password' or 'bad username' messages)
doesn't have a 'show typed password' button to allow viewing what you've typed in
doesn't allow viewing the password for a given saved network
doesn't allow logging into interactive login wifi (such as the wifi in public transport)
current asteroid virtual keyboard requires some setup and isn't super easy to type on (but that's not directly relevant to this PR)
some small UI tweaks still needed (though not exactly ugly in its current form)

The big blocker is security. It would not be good to merge this right now as our watches don't ship any passwords or authentication at all. A number of solutions, as seen in discussions I've had with @eLtMosen and @beroset , in order roughly reflecting simplicity:

Show a popup the first time the wifi page is shown with text to the effect of 'your watch has no password by default. don't connect to public wifi. you do this at your own risk' (pros: very easy. Cons: easy to ignore, users are lazy and may not heed this warning)
make a default password on all asteroid watches something like asteroid (pros: sets a default password. cons: user is not likely to change password, not much better than zero password)
Generate a random password for ceres on first install. Add a page which shows the ceres password in asteroid-settings (perhaps this would only show the default password, but not a password the user has set themselves). Disable ssh to root and only allow su or sudo from ceres as the way to get to root. (pros: mitigates the 'lazy user' issue by providing a default password that is, at least, unique. cons: makes boot issues harder to debug, as user will not know this password until first boot. makes ssh instructions look less easy for users unfamiliar with shell.)
Add an rsa key generation step to the installation process (pros: key auth is sexy and cool and all the serious admins do it. very secure. cons: different machines and distros will have different ssh setups, so instructions will likely be 'here's how to do it on ubuntu, or go figure it out yourself'. not trivial for new linux users, and somewhat complicated to deal with on windows)

I think that a good first step for security is adding sudo to our base images. This isn't absolutely necessary, but it will make it easier to disable root-ssh. Then disabling root ssh would be the next step in the right direction.

Essentially an updated version of f4a795b from https://github.com/magnefire/asteroid-settings original commit info below: Author: MagneFire <[email protected]> Date: Thu Feb 4 22:52:23 2021 +0100 [WIP] WiFi: Add WiFi page. CUrrently consists of: - Toggle WiFi on/off. - Scan for WiFi networks when adapter powered. - Testing for key input (InputPanel, too small needs tweaking). - Show list of scanned WiFi networks.

Wifi can be turned on or off, and networks show up in a list. There is currently no capability of connecting to wifi networks.

…ility

- tapping on a saved network initiates a connection - opening the settings on a saved or connected network won't show the login fields

- add ip address on current network - add 'disconnect' button - add 'remove' button

…ttings

…tings pages - make all rows same height - use new LabeledActionButton - make IP text fit in column - use dims to be consistent with other settings pages

- align all text to the middle

…efore network has been added

eLtMosen · 2023-06-01T20:53:05Z

On the password issue, i would favor to set a standard Passphrase. One that is not too easy to break within seconds in a sort of drive-by scenario. Like "FreeYourWri$t_24-7".
Then nag users during initial setup to change that to a randomized password generated on the watch and shown to note it down. When skipping that step and keeping the standard password, a "We assume you know the involved riscs" warning is shown with "Please refer to our documentation how to change the standard password or set up key auth.

dodoradio · 2023-06-01T21:03:21Z

there's currently no 'shell' steps in the install process, it's very much get-up-and-go as soon as you've booted up.
Perhaps it would be appropriate to have a shell setup script in /home/ceres/ which could serve as a sort of companion to the initial setup/tutorial we have in the launcher. But it's not clear exactly what kind of things would show up in that sort of installer apart from setting a password.

beroset · 2023-06-01T21:25:04Z

I have been thinking about this and I believe I have a solution that minimizes the security risks.

The problem

The problem with our existing situation is that there are no passwords for either the ceres or root users. This is obviously a security problem, but only if we provide remote connectivity, such as with WiFi. Our current Bluetooth connectivity does not provide a means for shell access, and we do not currently have any watches with supported cellular phone connectivity.

Potential solutions

There are at least four potential solutions. They are listed below with what I see as the most serious drawbacks for each.

don't provide connectivity
This is essentially the current situation, but it limits many things a user might otherwise usefully do with a watch, such as synchronizing a remote calendar or fetching weather or map data.
set default system passwords
This would be an incremental security update, but unless the user changes the passwords, the passwords are easily discoverable from our online documentation. Getting every user to do so is probably not realistic, even with nag screens.
require the user to specify a password on installation
While this is what is currently done with many Linux distributions, it's not at all certain that users would choose good passwords and remember them.
generate secure passwords for the user
Generating secure and unique passwords for each watch instance would be a useful way to enhance security, but relying on the user to memorize or securely record the password is probably more onerous than insisting the user come up with one.

A proposed solution

The problem we're really trying to solve has to do with remote exploits over WiFi much more than preventing physical access to devices. With that insight, a solution might be:

generate secure passwords for both ceres and root on first boot
store both passwords in a way that allows retrieval on the watch (e.g. in plain text somewhere)
provide a "Passwords" screen within asteroid-settings and provide an option for the user to reveal the existing passwords and/or change them
once the user has changed the password, optionally disable the reveal capability

The advantage is that every watch would be usable (relatively) securely even if we provided WiFi connectivity by default, but it avoids all of the drawbacks listed above.

Having WiFi on is still going to be a security risk, since the underlying kernels almost certainly have known security flaws, but this may provide a reasonable way to mitigate that risk.

dodoradio · 2023-06-01T21:38:48Z

I do like your proposed solution, but I don't think we should show the root password. I think it would be much better to use sudo for this, at least because sudo comes with a 'root comes with responsibilities' warning when you first use it.

I would note that it seems the android kernels have some basic CVE patches applied, or something to that effect at least. I was attempting some known exploits for 3.18 kernel on koi while doing that port, and it was, at least, resistant to the most basic attacks I'd found

ManuelBoe14 · 2023-06-02T08:06:22Z

As far as I understand it correctly, not having a password is a problem because attackers could exploit the unprotected access to the Smartwatch.

Couldn't we just minimise the attack surface by disabling SSH and possibly installing a firewall like ufw that blocks all incoming connections by default?

Someone who needs services like SSH will probably know how to disable the firewall and/or enable SSH.

dodoradio · 2023-06-02T10:12:07Z

ssh is the primary way of getting a shell. the main issue here is that there's no way to enable ssh / disable firewall on a watch where you can't get to shell. This wifi patch is mostly useless for anything other than ssh - the only two apps that use the internet are beroset's weatherfetch and my map, which both require manual installation through a command line-based package manager.

it's worth mentioning that adb is a potential way to get shell access as well, but it has a few quirks:

it initially drops you in as root, which isn't great. users should know when they've manually escalated privileges.
there's no guarantee all watches will have adb if some move to a mainline kernel (though that is far off in the future)
on some watches the USB setup is really frustrating. on koi, at the moment, USB requires significant disassembly which was so frustrating that I finally wrote this wifi patch. ssh works over wifi, adb only works over USB
the adb shell is quite basic and a bit austere. It doesn't set up any convenient environment variables like ssh does, it drops you in as root, it doesn't support and terminal standards. it very much feels like the basic debugging tool it was meant to be, built for an android that doesn't encourage hacking.

I don't think that disabling ssh solves any of our ssh security problems.

FlorentRevest · 2023-06-06T22:24:01Z

src/qml/main.qml

+                    iconName: "ios-wifi-outline"
+                    onClicked: layerStack.push(wifiLayer)
+                }
+                ListItem {


I think you didn't mean to add the following entries

FlorentRevest · 2023-06-06T22:26:29Z

src/qml/WiFiPage.qml

+        id: wifiModel
+        name: "wifi"
+        onCountChanged: {
+            console.log("COUNT CHANGE " + count)


I know this is a draft but since I'm at it, let me think out loud. I believe we should try to keep the console.logs that make it to production ~minimal and "to the point". There's a sweet spot between not providing any debug info in our logs (basically the current situation) and drowning useful information in tons of logs (what would happen if we start to do this sort of things)

FlorentRevest · 2023-06-06T22:28:01Z

src/qml/WiFiPage.qml

+        anchors.horizontalCenter: parent.horizontalCenter
+        height: parent.height
+        header: Item {
+            //this is literally a statuspage


I haven't looked at the exact diff but any reason why you're not re-using/extending the existing statuspage instead ?
Not having had time to look into it yet is a valid question, I'm just curious

ok, the comment here is old and misleading. Basically, this is/was the visual components of a statuspage, but they're then moved and resized to make way for the rest of the menu items when wifi is enabled. I'll add this to my cleanup todo list.

FlorentRevest · 2023-06-06T22:30:02Z

src/qml/WiFiPage.qml

+                    } else if (modelData.favorite){
+                        qsTrId("id-wifi-saved")
+                    } else {
+                        qsTrId("id-wifi-notsetup")


I believe you'd need to provide the string in a specially formatted comment just above these qsTrId

Also a final PR would need to update the translation files, but you can keep this for when that's undrafted

FlorentRevest · 2023-06-06T22:35:03Z

src/qml/WiFiPage.qml

+                }
+            }
+
+


Spacing between components isn't consistent with sometimes 2 spaces, sometimes 1, sometimes 0.

The QML convention seems to be to leave 1 empty line between two consecutive blocks at the same depth. I believe that @eLtMosen uses a QML linter. Whatever canonical coding style is outputted by a sane QML linter should be our project-wide default.

FlorentRevest · 2023-06-06T22:36:35Z

src/qml/main.qml

+                    title: qsTrId("id-wifi-page")
+                    iconName: "ios-wifi-outline"
+                    onClicked: layerStack.push(wifiLayer)
+                }


I think we should guard this entry with a visible clause similar to the one for "Sound" so that porters can specify whether Wi-Fi is supported by the port and we only show this menu when it's relevant and not a misleading broken UX on watches where it's not supported by the port

yep, that sounds like a good idea

FlorentRevest · 2023-06-06T22:38:48Z

src/qml/WiFiPage.qml

+
+        footer: Item {height: wifiStatus.powered ? root.height*0.15 : 0; width: parent.width}
+
+        delegate: MouseArea {


I wonder if we should use a https://github.com/AsteroidOS/qml-asteroid/blob/master/src/controls/qml/ListItem.qml here instead of re-inventing that wheel.

We can also have the ListItem use a Marquee, I think it's a sane default anyway.

this does show two bits of text instead of one, and doesn't make use of an icon, so ListItem isn't relevant here

qml-asteroid is not set in stone. We can expand ListItem to handle a primary and a secondary text and no icon.

The reason why I think this is relevant is that this sort of code factorization also keeps the design consistent across the OS. For example, ListItem has a HighlightBar that isn't used here. The more we factorize code the better it is to keep the UI sharp and to conduct refactorings/redesigns in the future.

The highlightbar is something I wanted to add, but I wanted to push the code to prompt the security conversation before I invested too much time.
I want to keep this code here without abstracting it away, but I'm open to the conversation. However:

while adding components to org.asteroid.controls can improve consistency, it won't necessarily improve maintainability. Recent changes which added highlightbar immediately broke asteroid-settings, but that was fixed - however, asteroid-map still hasn't, and the new highlightbar changes have broken the UI significantly. I think future changes, apart from miniscule tweaks to font size and the like, will probably bring the same kind of breakage and the same kind of cross-repo maintenance tasks.

qml-asteroid won't account for all the edge cases, and I think this might be an edge case. This chunk of code definitely needs cleaning up and making more consistent, but I'm not sure it will be used anywhere else, and I think 'potential to be used somewhere else' is the bar we should set for adding things to qml-asteroid.

FlorentRevest · 2023-06-06T22:41:27Z

src/qml/WiFiConnectionDialog.qml

+                TextField {
+                    id: passphraseField
+                    text: modelData.passphrase
+                    echoMode: TextInput.Password //smartwatches are hard to type on. it is worth adding a 'show password' button for this field


Delegating all UX decisions to @eLtMosen but I don't think we gain very much from hiding the password here. Imho these password placeholders are for situation where the user is sitting in a crowded space and types a password that they don't want anyone around to see. I don't really see this happening with a user typing their Wi-Fi passphrase once on their watch. I don't feel particularly strongly either way but this echoMode feels a bit overkill to me.

FlorentRevest · 2023-06-06T22:52:23Z

Regarding the SSH password by default debate, a couple of drive-by comments:

Having root SSH enabled seems very useful to me. This lets us install stuff like watchfaces without complications. I very often scp stuff to /. Disabling root SSH would get in the way.
Maybe it could make sense to distinguish dev builds from user builds and keep an empty root password by default on dev builds so this doesn't get in the way. For end-users (whatever gets distributed by asteroidos.org) I like dynamically generating a password and showing it to them in some easy way. I agree with beroset that there's nothing wrong with showing a root password given our threat model (remote attacker only), if someone has physical access to the watch it's already game over anyway so no point doing security theater
I was thinking about how Android does ADB over Wi-Fi. I think they have a nice model where the user is expected to ack an incoming connection once manually on the mobile device. Given our threat model (defending against remote attackers while keeping the life of the devs and users easy) this seems like the best UX we could hope for. I wonder if we could implement something in the style of our current Bluetooth agent where we show a pop-up to the user "accept incoming connection yes/no" and then the trust bond is remembered for the future... I believe that SSH has a fancy "agent" mechanism to do this sort of sophisticated authentication scenario but I've only used it on the client side, I don't know what that looks like on the server side)

dodoradio · 2023-06-06T23:17:02Z

Regarding root SSH - I don't think it's a great idea to leave it enabled, partly just because it can encourage irresponsible root shell usage from new users. It's fairly trivial to enable it for developers who need it (if they're regularly building their own images, they might even want to tweak that conf file at the build system level). Instead. It would be more complex, but I think much better practice to make sure we don't rely on remote root for such simple things as watchface management.
Perhaps we could source watchfaces from the user's home dir?

I hope we get a sport tracker soon - that will be logging GPS data and can reveal a user's home address. Currently we can afford to exchange developer convenience for security, but I don't think that's a good model moving forward. A large userbase will bring a lot of users who are unfamiliar with good security practices (eg. don't use root shell unless you have to) and I think we should cater to those first even if it means you can't ssh [email protected] "rm -rf /* --no-preserve-root" without changing any config files

beroset · 2023-06-17T11:52:55Z

Having root SSH enabled seems very useful to me. This lets us install stuff like watchfaces without complications. I very often scp stuff to /. Disabling root SSH would get in the way.

I think that for developers, the better way to do that is to set up password-less login using ssh-copy-id instead of having an empty password. That way, it only needs to be done once and preserves some level of security without getting in the way of developers.

dodoradio added 16 commits June 1, 2023 10:41

Visually clean up the wifi page.

cc71d99

Wifi can be turned on or off, and networks show up in a list. There is currently no capability of connecting to wifi networks.

Add login dialogue to wifi page

9951258

Split off connection dialog into its own file, to improve code readab…

bc739c9

…ility

Fix spacing

13f6828

Differentiate between saved and available networks

512b10a

Add specific behaviours for saved or connected networks

d6f838c

- tapping on a saved network initiates a connection - opening the settings on a saved or connected network won't show the login fields

Add status info and settings to connection page

859dd99

- add ip address on current network - add 'disconnect' button - add 'remove' button

WiFiPage: always open the connection dialog on press and hold

2e1dcbf

WiFiConnectionDialog: make icon consistent with other checkmars in se…

64ed598

…ttings

WiFiConnectionDialog: resize elements to be consistent with other set…

8c970e1

…tings pages - make all rows same height - use new LabeledActionButton - make IP text fit in column - use dims to be consistent with other settings pages

WiFiConnectionDialog: switch to marquee to accommodate wider SSIDs

5af1c7a

WiFiPage: use marquee to accommodate longer SSIDs and realign text

5940966

- align all text to the middle

WiFiConnectionDialog: sync autoconnect checkbox with current value

7069f16

Wifi pages: finalise translation IDs

3deca50

WiFiConnectionDialog: make sure that 'remove network' isn't visible b…

b80e363

…efore network has been added

FlorentRevest reviewed Jun 6, 2023

View reviewed changes

AsteroidOS deleted a comment from Mathewsodhiambo Oct 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Wifi Settings #91

Add Wifi Settings #91

dodoradio commented Jun 1, 2023

eLtMosen commented Jun 1, 2023

dodoradio commented Jun 1, 2023

beroset commented Jun 1, 2023

dodoradio commented Jun 1, 2023

ManuelBoe14 commented Jun 2, 2023

dodoradio commented Jun 2, 2023

FlorentRevest Jun 6, 2023

FlorentRevest Jun 6, 2023

FlorentRevest Jun 6, 2023

dodoradio Jun 6, 2023

FlorentRevest Jun 6, 2023

FlorentRevest Jun 6, 2023

FlorentRevest Jun 6, 2023

FlorentRevest Jun 6, 2023

dodoradio Jun 6, 2023

FlorentRevest Jun 6, 2023

dodoradio Jun 6, 2023

FlorentRevest Jun 6, 2023

dodoradio Jun 6, 2023

FlorentRevest Jun 6, 2023

FlorentRevest commented Jun 6, 2023

dodoradio commented Jun 6, 2023 •

edited

Loading

beroset commented Jun 17, 2023


		footer: Item {height: wifiStatus.powered ? root.height*0.15 : 0; width: parent.width}

		delegate: MouseArea {

Add Wifi Settings #91

Are you sure you want to change the base?

Add Wifi Settings #91

Conversation

dodoradio commented Jun 1, 2023

eLtMosen commented Jun 1, 2023

dodoradio commented Jun 1, 2023

beroset commented Jun 1, 2023

The problem

Potential solutions

A proposed solution

dodoradio commented Jun 1, 2023

ManuelBoe14 commented Jun 2, 2023

dodoradio commented Jun 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FlorentRevest commented Jun 6, 2023

dodoradio commented Jun 6, 2023 • edited Loading

beroset commented Jun 17, 2023

dodoradio commented Jun 6, 2023 •

edited

Loading