tailscale / tailscale

The easiest, most secure way to use WireGuard and 2FA.
https://tailscale.com
BSD 3-Clause "New" or "Revised" License
18.82k stars 1.46k forks source link

OAuth keys cannot create device keys with tags owned by OAuth key's tag #12402

Open arnecls opened 4 months ago

arnecls commented 4 months ago

What is the issue?

When creating an oauth key with scope devices and tag:x, that key cannot generate a device auth key with tag:x and tag:y, even if tag:y is owned by tag:x.

The documentation states that this should be possible

When creating an auth key owned by the tailnet (using OAuth), it must have tags. The auth tags specified for that new auth key must exactly match the tags that are on the OAuth client used to create that auth key (or they must be tags that are owned by the tags that are on the OAuth client used to create the auth key).

Usecase

We are currently implementing a key rotation through vault. Vault is configured with a single oauth client.

We now have defined different vault policies to allow specific service accounts to create specific kind of keys by adding constraints to the allowed api parameters (like only allowing ephemeral, non-reusable keys).

One of the constraints we'd like to add is enforcing a specific set of tags. This way we could allow a service account to generate keys only for specific kind of instances, like "only exit nodes", "only routers" or nodes with restricted access.

Currently we can only do this through multiple, different OAuth credentials, which adds additional complexity to the setup.

Steps to reproduce

Define two tags like this:

"tagOwners": {
  "tag:main":  ["autogroup:admin", "autogroup:owner"],
  "tag:subset": ["autogroup:admin", "autogroup:owner", "tag:main"],
}

Now create an OAuth client named "main", give it "devices:write" rights. As tags, select "tag:main".

The following go code will panic when calling CreateKey

    clientOpts := []tailscale.ClientOption{
        tailscale.WithBaseURL("https://api.tailscale.com"),
        tailscale.WithOAuthClientCredentials(clientID, clientSecret, []string{"devices"}),
    }

    client, err := tailscale.NewClient("", org, clientOpts...)
    if err != nil {
        panic(err)
    }

    var capabilities tailscale.KeyCapabilities
    // Use a tag that is owned by one of the key's tag
    capabilities.Devices.Create.Tags = []string{"tag:subset"} 
    capabilities.Devices.Create.Preauthorized = false
    capabilities.Devices.Create.Ephemeral = true

    key, err := client.CreateKey(context.Background(), capabilities)
    if err != nil {
        panic(err)
    }

Are there any recent changes that introduced the issue?

No response

OS

Other

OS version

-

Tailscale version

1.66.4

Other software

tailscale-client-go v1.17.0

Bug report

No response

kelivel commented 3 months ago

Hi @arnecls, can you provide the error message shown after the code panics?

JayWStapleton commented 3 months ago

Tag ownership is not transitive. If you are tagOwner of tag:tag1 and tag:tag1 is the tagOwner of tag:tag2, you cannot directly apply tag:tag2 to a device. You can apply tag:tag1 and then run tailscale set --adveritise-tags=tag:tag2 since you're operating within a context of tag:tag1

So that part is working as intended.

willnorris commented 3 months ago

As @kelivel noted, providing the error message would be really helpful.

I'm actually unable to reproduce this in my own testing. I've set my test tailnet up with the access control policy file:

{
    "tagOwners": {
        "tag:main":   ["autogroup:admin", "autogroup:owner"],
        "tag:subset": ["autogroup:admin", "autogroup:owner", "tag:main"],
    },
}

I have an OAuth client with the devices scope and tag:main:

image

And then I have a very slightly modified version of your code (just to read values from the environment and print the final key):

// Test case for https://github.com/tailscale/tailscale/issues/12402
package main

import (
    "context"
    "encoding/json"
    "os"

    "github.com/tailscale/tailscale-client-go/tailscale"
)

func main() {
    var clientID = os.Getenv("TS_CLIENT_ID")
    var clientSecret = os.Getenv("TS_CLIENT_SECRET")
    var org = "-"

    clientOpts := []tailscale.ClientOption{
        tailscale.WithBaseURL("https://api.tailscale.com"),
        tailscale.WithOAuthClientCredentials(clientID, clientSecret, []string{"devices"}),
    }

    client, err := tailscale.NewClient("", org, clientOpts...)
    if err != nil {
        panic(err)
    }

    var capabilities tailscale.KeyCapabilities
    // Use a tag that is owned by one of the key's tag
    capabilities.Devices.Create.Tags = []string{"tag:subset"}
    capabilities.Devices.Create.Preauthorized = false
    capabilities.Devices.Create.Ephemeral = true

    key, err := client.CreateKey(context.Background(), capabilities)
    if err != nil {
        panic(err)
    }

    b, _ := json.MarshalIndent(key, "", "  ")
    print(string(b))
}

When I run it, I get an auth key created:

% TS_CLIENT_SECRET=tskey-client-kuy8tq9jzy11CNTRL-XXXXX go run .

{
  "id": "kD7jqE6mh811CNTRL",
  "key": "tskey-auth-kD7jqE6mh811CNTRL-XXXXX",
  "description": "",
  "created": "2024-07-03T17:10:43Z",
  "expires": "2024-10-01T17:10:43Z",
  "revoked": "0001-01-01T00:00:00Z",
  "invalid": false,
  "capabilities": {
    "devices": {
      "create": {
        "reusable": false,
        "ephemeral": true,
        "tags": [
          "tag:subset"
        ],
        "preauthorized": false
      }
    }
  }
}

Is there anything I did different from your setup?

arnecls commented 3 months ago

You can apply tag:tag1 and then run tailscale set --adveritise-tags=tag:tag2 since you're operating within a context of tag:tag1

That's what we're currently doing. However, this means that whoever has that key can register devices with any of the tags owned by the key. What we would like to do is create a device key that can only register a specific set of tags. And that through the same oauth key.

That still is an issue if that oauth key is leaked, but that key is only available to vault and that's pretty safe. The device keys however are available on e.g. GitHub actions and these are not safe and are prone as they are prone to exfiltration.

What we would like to do is to minimize the blast radius of an exfiltrated key so we constraint it as much as possible and rotate it as often as possible.


[!NOTE] The following is out of scope of this issue, but related

The automatic deactivation of expiration when a device is registered is also critical in this scenario. It's a nice default but there should be a way to disabled it. And that should be "baked" into the device-key, too.

For example - if the key is rotated once a day and the key is exfiltrated, any device that is registered with that key during the day will stay in the tailnet until identified and removed. If the expiration would hold, these devices would disappear as soon as the key times out.

arnecls commented 3 months ago

Here's the code I use for testing: https://github.com/arnecls/vault-plugin-tailscale/blob/master/cmd/oauth_test/main.go

If I run ./oauth_test ${TAILNET} tag:b ${ID} ${SECRET} it works (now?) if I run ./oauth_test ${TAILNET} tag:a,tag:b ${ID} ${SECRET} it panics

panic: requested tags [tag:a] are invalid or not permitted (400)

I checked and in the second case both tags are being passed to the API call.

willnorris commented 3 months ago

When creating a tagged auth key from a tagged OAuth client, the tags must match exactly, or all of the auth key tags must be owned by the client tags. In your previous example, where you specified the ACL file:

"tagOwners": {
  "tag:main":  ["autogroup:admin", "autogroup:owner"],
  "tag:subset": ["autogroup:admin", "autogroup:owner", "tag:main"],
}

An OAuth client with tag:main would be able to create an auth key with tag:main (because then it's an exact match), or an auth key with tag:subset (since it's owned by tag:main). You would not, however, be able to create an auth key with both tag:main and tag:subset, because that doesn't fit into either of the two allowed cases. If that's what you want, you can achieve that by making tag:main an owner of itself:

"tagOwners": {
  "tag:main":  ["autogroup:admin", "autogroup:owner", "tag:main"],
  "tag:subset": ["autogroup:admin", "autogroup:owner", "tag:main"],
}

This feels a little weird because we're trying to address two distinct use cases with tagged auth keys. In the simple case, the client tags ARE the auth key tags... there is no way to change them, they are always going to be the same exact set. In the advanced case (which you clearly fall into), we allow customizing tags based on ownership, but in order to prevent any surprises you have to be very explicit about ownership, including identifying a tag as an owner of itself if you want to allow it to assign that tag.


As to your question about expiration, as you've discovered that's just not how expiration works today. The expiration of a device is completely unrelated to the expiration of the auth key used to register that device. Being able to embed a device expiration time into the auth key is a really interesting idea though. I can't promise it, but I'll bring it up with the team.

arnecls commented 3 months ago

because that doesn't fit into either of the two allowed cases

Ah, now I get it. That would resolve the issue. The only thing would be to maybe add this as an example to the docs, as I found the current section a bit hard to understand (hence my issue).

Thanks 👍🏻

Being able to embed a device expiration time into the auth key is a really interesting idea though

I would be very happy if this would become a thing.