Add Tailscale ZTA module

[hkrutzer]

No description provided.

[CLAassistant]

All committers have signed the CLA.

[hkrutzer]

I don't know how to test the unix socket unfortunately

[josevalim]

Hi @hkrutzer, while reviewing your PR, I noticed potential improvements in the ZTA implementations: e37b2ce

Can you please align your PR accordingly?

@wojtekmach, any suggestions on how to mock the socket for tests?

[josevalim]

You may be able to use the basic: uri.userinfo option. :)

[hkrutzer]

Is that a Req option? I can't find it in the Req codebase

[josevalim]

Sorry, it might be auth: {:basic, uri.user_info}?

[wojtekmach]

neither is supported. We can do auth: {user, pass}. In hindsight {:basic, userinfo} is not a bad idea.

[hkrutzer]

I changed it to [auth: "Basic #{Base.encode64(uri.userinfo)}"] which saves a few characters, but I think that's the best we can do right now :)

[josevalim]

Btw, please remember to sign the CLA once it is ready to merge (if you agree with it, of course). :)

[wojtekmach]

Regarding testing unix socket, I think the best option is to open a real socket. I'd install Bandit for tests and:

defmodule TestPlug do
end

@tag :tmp_dir
test "it works", %{tmp_dir: tmp_dir} do
  socket = "#{tmp_dir}/bandit.sock"
  start_supervised!({Bandit, plug: Plug, ip: {:local, socket}})
  Req.get(unix_socket: socket, ...)
end

alternatively we can do a low level test server. I haven't changed the following snippet to use UNIX socket but it's easy, need to update the :gen_tcp.listen call:

    test "it works" do
      %{url: url} =
        TestSocket.serve(fn socket ->
          assert {:ok, "GET / HTTP/1.1\r\n" <> _} = :gen_tcp.recv(socket, 0)
          body = "hello"

          data = """
          HTTP/1.1 200 OK
          content-length: #{byte_size(body)}

          #{body}
          """

          :ok = :gen_tcp.send(socket, data)
        end)
end

defmodule TestSocket do
  def serve(fun) do
    {:ok, listen_socket} = :gen_tcp.listen(0, mode: :binary, active: false)
    {:ok, port} = :inet.port(listen_socket)
    pid = ExUnit.Callbacks.start_supervised!({Task, fn -> accept(listen_socket, fun) end})
    %{pid: pid, url: "http://localhost:#{port}"}
  end

  defp accept(listen_socket, fun) do
    case :gen_tcp.accept(listen_socket) do
      {:ok, socket} ->
        fun.(socket)
        :ok = :gen_tcp.close(socket)

      {:error, :closed} ->
        :ok
    end

    accept(listen_socket, fun)
  end
end

[hkrutzer]

Cheers @wojtekmach went with the first option.

Can you please align your PR accordingly?

I hope I've done that :)

Btw, please remember to sign the CLA once it is ready to merge (if you agree with it, of course). :)

I have signed it but it wasn't obvious to me that there was one. If I knew beforehand I might not have built this, perhaps you can add it to the readme?

[github-actions]

Uffizzi Preview deployment-36142 was deleted.

[josevalim]

LGTM but CI is unhappy with setting up the unix port. Could it be a permission issue?

[hkrutzer]

I just tried on my Mac and it seems to be a path (length) issue? I get the EINVAL error there as well unless I use /tmp/bandit.sock.

[wojtekmach]

Ah yeah I've seen it recently, Erlang doesn't like a long socket path. This is a workaround that worked for me: (tmp_dir is always absolute)

- socket = "#{tmp_dir}/bandit.sock")
+ socket = Path.relative_to_cwd("#{tmp_dir}/bandit.sock")

[wojtekmach]

per:

% tailscale ip --help
USAGE
  ip [-1] [-4] [-6] [peer hostname or ip address]

Show Tailscale IP addresses for peer. Peer defaults to the current machine.

FLAGS
  --1, --1=false
    	only print one IP address (default false)
  --4, --4=false
    	only print IPv4 address (default false)
  --6, --6=false
    	only print IPv6 address (default false)

we could pass --1 or --4 to get just one IP.

[wojtekmach]

LIVEBOOK_IP=$(exec ...)

in my local testing (against Mac App Store-installed tailscale) it was enough to set LIVEBOOK_IP to $(tailscale ip --1) so maybe we show that here too?

As an aside, @josevalim: would it be possible to just set LIVEBOOK_IDENTITY_PROVIDER=tailscale and do the rest in Elixir e.g. when the app boots? I think it would be a much better UI if users don't have to copy paste a bunch of shell commands.

Similarly, perhaps we don't need to require users to set LIVEBOOK_IP but can do that ourselves internally.

That being said I think it is totally fine to start with something really simple if verbose and improve this as more people use the feature.

[josevalim]

Let's update the examples to use tailscale ip --1 but require those env vars to be set. I don't want to rely on lsof for macos and the setting of LIVEBOOK_IP happens at a very different place which I am not sure it is worth coupling.

[hkrutzer]

in my local testing (against Mac App Store-installed tailscale) it was enough to set LIVEBOOK_IP to $(tailscale ip --1) so maybe we show that here too?

It works if you have a Tailscale alias in your shell, I got this command here because it (should) work on all types of MacOS installs without requiring an alias to be set.

[wojtekmach]

Ah, good point, the CLI doesn’t get installed by default. Fine by me.

[wojtekmach]

Or in our instructions we do the alias. I think that’s better. And if we can somehow get the port and password out of the cli with less code that’s even better. But no strong opinion.

[wojtekmach]

if the socket path is incorrect (see my comment above), the error is pretty bad:

[error] retry: got exception, will retry in 1000ms, 3 attempts left
[error] ** (Mint.TransportError) no such file or directory
[error] retry: got exception, will retry in 2000ms, 2 attempts left
[error] ** (Mint.TransportError) no such file or directory
[error] retry: got exception, will retry in 4000ms, 1 attempt left
[error] ** (Mint.TransportError) no such file or directory
** (Mint.TransportError) no such file or directory
    (req 0.3.8) lib/req.ex:372: Req.get!/2
    iex:3: (file)

my suggestion is to handle this particular scenario early on, perhaps something like this:

Suggested change

	# Assume address not starting with http is a Unix socket
	unless File.exists?(address) do
	raise "socket doesn't exist: #{inspect(address)}"
	end

[hkrutzer]

Added!

[wojtekmach]

I was using Tailscale and Livebook with this branch on two computers and accessed Livebook started in one computer from another (using Tailscale IP). I showed up as one user, the Tailscale-authenticated one, instead of two since there were two browser sessions so to speak. Is that expected?

In any case @hkrutzer this is pretty awesome, thank you!

[hkrutzer]

I showed up as one user, the Tailscale-authenticated one, instead of two since there were two browser sessions so to speak. Is that expected?

I am not sure what happens for the other identity providers, and how Livebook determines user uniqueness, I guess based on the ID field? As you are the same user it doesn't sound strange to me at least.

[wojtekmach]

RE multiple users, just to be clear I was asking in general about ZTA feature not necessarily about this one.

Btw, I found out we can do:

$ tailscale debug local-creds
curl -u:ac48xxx http://localhost:52808/localapi/v0/status

and so we can parse this. Extracting information out of curl invocation doesn't sound super reliable but the the lsof, even though is actually what happens under the hood, looks pretty scary. Just to be clear, I think what we have right now is great but trying to figure out if we can make it easier for people.

[josevalim]

RE multiple users, just to be clear I was asking in general about ZTA feature not necessarily about this one.

Each user would go through authentication provided by the proxy. In this case, it seems we are authenticating the machine Livebook is on, not the user accessing the page, right? If so, we need to document it.

---

@wojtekmach I think asking users to install the tailscale CLI and relying on it for commands is better.

[josevalim]

Each user would go through authentication provided by the proxy. In this case, it seems we are authenticating the machine Livebook is on, not the user accessing the page, right? If so, we need to document it.

It seems I was wrong. Tailscale is authenticating the user indeed. :) So it all looks good to me. Unless you create another tailscale account, it will be the same user.

[josevalim]

💚 💙 💜 💛 ❤️

[wojtekmach]

@josevalim the app is the CLI, in other words they even tell us to:

$ alias tailscale=/Applications/Tailscale.app/Contents/MacOS/Tailscale
$ tailsale version
1.48.1
  tailscale commit: 528f95da6a5e12961b36a39bb3fa512c093d2330
  other commit: c4e4acad7220e23eb6f835e2b846c0378612d43a
  go version: go1.21.0

so the users don't need to do anything fancy, just do the alias or a symlink. I think it's fine to do the alias in our setup instructions.

(app being cli is giving me ideas but that's for another day)

[josevalim]

Ah, PRs to use the app alias then are very welcome :)

[hkrutzer]

Indeed the app also serves as CLI, and I can't find a page in the Tailscale docs that explains how to add it. It seems it's only documented in the Github issue I linked elsewhere. The way to add the alias is different for every method of installation (there are 3).

So we could mention a user needs to have that alias, or link to that issue that explains how to add it, or replace the lsof with $(ps -xo comm | grep MacOS/Tailscale$) debug local-creds and parse its output (removing the lsof might make it look less scary).

[hkrutzer]

It seems Windows tests don't/didn't run on this PR. I don't think we can skip tests based on the platform?

[josevalim]

Good catch. I pushed an exclude in a later commit.