Support turning off https from channel.ini

Bug #1278589 reported by Stéphane Graber on 2014-02-10
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ubuntu system image
Wishlist
Barry Warsaw

Bug Description

Now that the server code can be used to build alternate server for porters, it's doubtful that all those folks will have valid SSL certificates (due to their cost and requirement for a separate public IP), so it'd be good to have a way to turn off https.

We clearly don't want to do any fallback to http automatically as this would allow an attacker to make us drop to http by blocking tcp/443, but having this be an option in channel.ini would be perfectly acceptable.

My suggestion for the implementation would be to support:
 - http_port: 0 => the server only supports https, don't even attempt an http connection
 - https_port: 0 => the server only supports http, don't even attempt an https connection

The first option likely won't be used by many setups, but it's still good to have if only for consistency. The second option is the one we need to make our porters' life easier.

Changed in ubuntu-system-image:
status: New → Triaged
importance: Undecided → Wishlist
tags: added: client
Changed in ubuntu-system-image:
assignee: nobody → Barry Warsaw (barry)
Barry Warsaw (barry) wrote :

I think rather, that I would add a `use_https` flag to the [service] section of the configuration file. This would of course default to yes/true/1 (we'd need a systemimage.helpers function as_boolean() to convert from any of those string values to Python True/False. lazr.config has such a converter, but we don't need to pull that whole dep in just for that function; we can do a straight rip, it's not much code).

Then in config.py, where we calculate self.service['https_base'], we'd first check this flag. If it was False, we'd just copy the value of self.service['http_base'] into self.service['https_base']. Everything else should then Just Work.

Probably the most work will be in setting up a test of this. The test infrastructure may need some code changes in order to set up the right scenario.

We'd also have to update ini-manpage.rst to document the new setting, and add that flag to all the .ini files in the code.

Stéphane Graber (stgraber) wrote :

So the problem with client.ini is that last time we made it writeable we ended up with quite a mess on upgrades, do we really want to repeat the experience?

The advantage of my proposed solution (using = 0) is that this wouldn't need any extra writeable file, would use the already existing channel.ini file which is already entirely owned by the server and this wouldn't require any server side change since most people without https already set https_port = 0 in etc/config on the server (which then gets written into channel.ini).

Barry Warsaw (barry) wrote :

channel.ini should be thought of as an override for client.ini, although at the moment, only the [system] settings can be overridden. It would be a trivial change to allow it to also override the [service] section. So for the moment, let's treat client.ini and channel.ini as essentially equivalent. In fact, in that case, [service]use_https would always default to yes/true/1 but channel.ini could override that to no/false/0.

I still like being explicit about whether to use https or not, rather than implying it from the port settings. I guess one advantage of stgraber's suggestion though is that you could run a server *only* over https and never hit http. Is that a valid use case? One confusion (and the source of my hesitancy) about allowing *_port=0 to mean "disable" is that in some cases, I've seen *_port=0 to mean "use the default port".

The other implication in the irc discussion is whether system-image should auto-detect whether https is available, or perhaps there should be a flag in channels.json. There's a bootstrapping problem with that though (and in fact with setting this in channels.ini too), in that several https-only steps have to be performed before you can trustfully download the channels.json file (e.g. the blacklist is downloaded over https). Worse, setting this in channel.ini means you'd have to do at least one update to end up with a channel.ini file, since that is not laid down with the initial flashing.

We do also have a settings facility, so you could imagine having a system-settings ui panel that allowed you to turn off https (or alternatively http). This would call into the existing SetSetting() D-Bus API, flipping the appropriate flags, which would be consulted when the configuration object is created. This would be safe from bootstrapping issues, would require an explicit action on the part of the user (something I'd prefer), and would not require access to a read-only file system. It would of course mean that there'd be a ui component to this change.

(Aside: how are we actually going to do configuration of services and apps when we can't write to /etc? ;)

Stéphane Graber (stgraber) wrote :

We shouldn't auto-detect as this opens the door for downgrade attacks.

The https-only use case does matter for some internal/OEM projects where they don't wish any traffic to ever go over http, so that's why I suggested both settings in the bug description.

I'm not entirely against having an https_only and http_only settings but then it gets confusing because you'll still need to set the ports to some value (even if they then get ignored) and you'll have to deal with the case where both _only are set to True ;)

Barry Warsaw (barry) wrote :

Totally agreed about auto-detect.

The question remains about the bootstrapping issue. Thus I'm starting to favor using the settings facility rather than specifying it in client.ini/channel.ini.

Stéphane Graber (stgraber) wrote :

Not sure what the bootstrapping problem is, a port server that's http only would simply:
 1) Setup the server with http_port = 80 and https_port = 0
 2) Publish an image
 3) Flash it with: phablet-flash --alternate-server si.example.net --disable-https --channel my-channel
 4) Done

The image contains a version tarball which has http_port = 80 and https_port = 0, so system-image will have the right value whenever it starts looking for updates.

If you were thinking about switching from an https server to a http-only server from the device, this isn't supported and can't work anyway as the two servers will have a different set of GPG keys, so validation of the updates by system-image and by the upgrader will fail in that scenario.

Vojtech Bocek (vbocek) wrote :

My two cents: I agree that port == 0 might look like "use the default one", so what about using "https_port = -1" instead? It is clearly invalid port number and I don't think it many people would recognize that as "use the default one". This whole thing is in python too, so there shouldn't be any problems with it being signed integer.

Barry Warsaw (barry) wrote :

Ah stgraber, right. I was thinking about a use case where a flashed device got switched to an http-only server. Ignoring that, if you flash directly against an alternative server, it would be that service's responsibility to lay down a proper channel.ini file.

Here's an implementation plan then:

* Instead of some magical numeric port number, let's use the string "disabled" (and "disable" as an acceptable but undocumented alternative, hello Postel's law). No quotes.

* You'll need to change the conversion function in config.py to accept either a numeric port number or disabled/disable. In the latter case, set http_port to a special marker object defined and exported in config.py

* Extra credit: explicitly disallow the port from being negative (raise an exception). For now, we'll still accept zero.

* Where we calculate http_base and https_base, check against the marker object and collapse the value of the disabled protocol to the enabled protocol, as discussed above. Extra credit: explicitly disallow setting both to disabled (raise an exception).

* Change Configuration.load() to allow overriding of the [service] section in channel.ini.

* Update ini-manpage.rst

* Add tests. You'll need tests in test_config.py for all the various corner cases, and probably one end-to-end test in test_state.py and/or test_dbus.py

I think that should do it, and it's not that much code. Most of the changes will be in config.py, other than the tests, and I predict test delta will outnumber non-test delta. :)

Changed in ubuntu-system-image:
assignee: Barry Warsaw (barry) → nobody
milestone: none → 2.2
Barry Warsaw (barry) on 2014-02-26
Changed in ubuntu-system-image:
status: Triaged → In Progress
assignee: nobody → Barry Warsaw (barry)
Barry Warsaw (barry) on 2014-02-26
Changed in ubuntu-system-image:
status: In Progress → Fix Committed
Barry Warsaw (barry) on 2014-04-01
Changed in ubuntu-system-image:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers