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

bug: update-server bricked after POST /server/name with input that Avahi doesn't like #9960

Closed
SyntaxColoring opened this issue Apr 14, 2022 · 2 comments · Fixed by #10219
Closed
Assignees
Labels
bug robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). update server

Comments

@SyntaxColoring
Copy link
Contributor

SyntaxColoring commented Apr 14, 2022

[Note: This ticket has been heavily edited since it was written to scope it down.]

Summary

If you rename the robot to certain strings, update-server will fail to start up.

Steps to reproduce

  1. Ensure you have SSH access to your robot. You will need this to recover from the error after testing.

  2. Send an HTTP request to set the robot's name to something longer than 63-ish characters. For example, here's 70 characters:

    POST /server/name
    {"name": "1234567890123456789012345678901234567890123456789012345678901234567890"}

    The request may return a 500 error.

  3. Reboot the robot.

Current behavior

After the reboot, update-server will never come back online.

Expected behavior

update-server should not accept input that can brick it. It should sanitize or reject harmful input through POST /server/name.

Recovery

  1. SSH in.
  2. Edit /etc/machine-info, e.g. with vi.
  3. Look for a line like PRETTY_HOSTNAME=<very long string>. Replace <very long string> with something small, like hello.
  4. Reboot the robot.
@SyntaxColoring SyntaxColoring self-assigned this Apr 14, 2022
@SyntaxColoring SyntaxColoring added the robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). label Apr 27, 2022
@SyntaxColoring
Copy link
Contributor Author

SyntaxColoring commented Apr 28, 2022

Investigation

  • When an HTTP client POSTs a new name, we first store it as the PRETTY_HOSTNAME in /etc/machine-info. Then, we set the Avahi service name to be the same string.
    • The Avahi service name can be mostly arbitrary Unicode. But, experimentally, it's not entirely arbitrary. In particular, there seems to be a length limit. (I think this stems from the DNS-SD specification for "instance names", RFC 6763 § 4.1.1.)
    • If the new Avahi service name is invalid, the D-Bus call will raise an exception. We don't catch this exception, so it'll propagate up and cause a 500 error for the request. But note that we've already written the dangerous name to /etc/machine-info.
  • The next time update-server restarts, it will read the dangerous name from /etc/machine-info and then try setting it as the Avahi service name. This will raise an uncaught exception for the same reason as before. But this time, the exception will propagate out to main() and prevent the server from coming up.

What, precisely, counts as an "invalid name"?

We set the Avahi service name over Avahi's D-Bus API. I'm assuming Avahi's D-Bus API follows its C API, which says:

name - The name for the new service. Must be valid service name. i.e. a string shorter than 63 characters and valid UTF-8. May not be NULL [empty].

But I think this wrong in a few ways. As mentioned above, I think the Avahi service name corresponds to the DNS-SD instance name. If I'm correct, then:

  • It's actually <= 63, not < 63, per RFC 6763 § 4.1.1.
  • It's actually 63 octets when encoded as UTF-8, not 63 characters. This is an important distinction. For example, according to RFC 6763 § 4.1.1, more than 21 Kanji characters is too long.

These rules appear entirely separate from what we're allowed to encode in /etc/machine-info as the PRETTY_HOSTNAME. Although they're both "mostly arbitrary Unicode," they differ around the edges.

Implementation suggestion

I don't think we should attempt to duplicate Avahi's validation rules. Instead, we should either:

@mcous
Copy link
Contributor

mcous commented May 3, 2022

This ticket explicitly covers fatal update-server breakage. Ticket can be closed when:

@SyntaxColoring SyntaxColoring changed the title bug: POST /server/name accepts invalid names bug: POST /server/name with a long name will brick update-server May 3, 2022
@SyntaxColoring SyntaxColoring changed the title bug: POST /server/name with a long name will brick update-server bug: POST /server/name certain names will brick update-server May 3, 2022
@SyntaxColoring SyntaxColoring changed the title bug: POST /server/name certain names will brick update-server bug: POST /server/name with certain input will brick update-server May 3, 2022
@SyntaxColoring SyntaxColoring changed the title bug: POST /server/name with certain input will brick update-server bug: update-server bricked after POST /server/name with input that Avahi doesn't like May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug robot-svcs Falls under the purview of the Robot Services squad (formerly CPX, Core Platform Experience). update server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants