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

Remove TXT record validation of custom DNS zones in VNet #46709

Merged
merged 4 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 6 additions & 37 deletions docs/pages/enroll-resources/application-access/guides/vnet.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ with a virtual IP address over which the connection will be proxied to the app.
case, the query is forwarded to the default DNS name server used by the OS.

If you want VNet to forward connections to an app that has a custom `public_addr` set, you need
to first update the VNet config in the Auth Service to include a matching DNS zone and authorize the
zone through a DNS TXT record.
to first update the VNet config in the Auth Service to include a matching DNS zone.

## Prerequisites

Expand All @@ -35,7 +34,7 @@ In this guide, we'll use the example app from [TCP Application Access guide](tcp
available through VNet at <Var name="public_addr" initial="tcp-app.company.test"/> with
<Var name="suffix" initial="company.test" /> as the custom DNS zone.

## Step 1/4. Configure custom DNS zone
## Step 1/3. Configure custom DNS zone

Create a file called `vnet-config.yaml` which specifies the custom DNS zone. In our case the
`public_addr` of the app is going to be `tcp-app.company.test`, so we're going to set
Expand All @@ -60,38 +59,11 @@ vnet_config has been created

<Admonition type="note" title="Relationship between suffix and public_addr">
`suffix` doesn't have to point to a domain that's exactly one level above the `public_addr` of an
app. Any level of nesting works. E.g., you can have an app under `tcp-app.foo.bar.qux.test` and the
suffix can be set to `bar.qux.test`.
app. Any level of nesting works. For example, you can have an app under `tcp-app.foo.bar.qux.test`
and the suffix can be set to `bar.qux.test`.
</Admonition>

## Step 2/4. Set DNS TXT record

Establish the name of your cluster:

```code
$ tctl status | awk '/Cluster/ {print $2}'
teleport.example.com
```

Set a DNS TXT record `teleport-cluster=<cluster name>` on the domain provided as `suffix` in the
VNet config. This is required by VNet to prevent a rogue cluster admin from hijacking DNS resolution
of arbitrary domains for unsuspecting users.

Wait a couple of minutes and verify that the DNS record changes have been propagated.

```code
$ host -t TXT <Var name="suffix" />
company.test descriptive text "teleport-cluster=teleport.example.com"
```

<Admonition type="note" title="DNS TXT and CNAME records">
If the domain you chose for the suffix has the CNAME record set, you won't be able to add a TXT
record. A CNAME record must be the only record for the given name. In that scenario, you can set the
suffix to a domain one level above if possible (e.g., `company.test` instead of
`internal.company.test`) or switch from CNAME to A records.
</Admonition>

## Step 3/4. Set `public_addr` of the app
## Step 2/3. Set `public_addr` of the app

Set `public_addr` field of the application in the Application Service configuration file
`/etc/teleport.yaml` and restart the `teleport` daemon.
Expand All @@ -107,7 +79,7 @@ app_service:
public_addr: "<Var name="public_addr" />"
```

## Step 4/4. Connect
## Step 3/3. Connect

Once you [start VNet](../../../connect-your-client/vnet.mdx), you should be able to connect to the
application over the custom `public_addr` using the application client you would normally use to
Expand Down Expand Up @@ -168,9 +140,6 @@ To make a [leaf cluster](../../../admin-guides/management/admin/trustedclusters.
$ tsh login --proxy=<Var name="leaf.example.com" /> --user=<Var name="email@example.com" />
```

The DNS TXT record of the domain provided as `suffix` needs to contain the name of the leaf cluster.
A single domain can have multiple TXT records if you want to use it in multiple clusters.

### Accessing web apps through VNet

VNet does not officially support [web apps](connecting-apps.mdx) yet. However, technically any web
Expand Down
47 changes: 12 additions & 35 deletions lib/vnet/app_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,10 @@ type DialOptions struct {

// TCPAppResolver implements [TCPHandlerResolver] for Teleport TCP apps.
type TCPAppResolver struct {
appProvider AppProvider
clusterConfigCache *ClusterConfigCache
customDNSZoneChecker *customDNSZoneValidator
slog *slog.Logger
clock clockwork.Clock
lookupTXT lookupTXTFunc
appProvider AppProvider
clusterConfigCache *ClusterConfigCache
slog *slog.Logger
clock clockwork.Clock
}

// NewTCPAppResolver returns a new *TCPAppResolver which will resolve full-qualified domain names to
Expand All @@ -118,7 +116,6 @@ func NewTCPAppResolver(appProvider AppProvider, opts ...tcpAppResolverOption) (*
}
r.clock = cmp.Or(r.clock, clockwork.NewRealClock())
r.clusterConfigCache = cmp.Or(r.clusterConfigCache, NewClusterConfigCache(r.clock))
r.customDNSZoneChecker = newCustomDNSZoneValidator(r.lookupTXT)
return r, nil
}

Expand All @@ -131,13 +128,6 @@ func withClock(clock clockwork.Clock) tcpAppResolverOption {
}
}

// withLookupTXTFunc is a functional option to override the DNS TXT record lookup function (for tests).
func withLookupTXTFunc(lookupTXT lookupTXTFunc) tcpAppResolverOption {
return func(r *TCPAppResolver) {
r.lookupTXT = lookupTXT
}
}

// WithClusterConfigCache is a functional option to override the cluster config cache.
func WithClusterConfigCache(clusterConfigCache *ClusterConfigCache) tcpAppResolverOption {
return func(r *TCPAppResolver) {
Expand Down Expand Up @@ -194,8 +184,8 @@ func (r *TCPAppResolver) clusterClientForAppFQDN(ctx context.Context, profileNam
return nil, errNoMatch
}

if isSubdomain(fqdn, profileName) {
// The queried app fqdn is direct subdomain of this cluster proxy address.
if isDescendantSubdomain(fqdn, profileName) {
// The queried app fqdn is a subdomain of this cluster proxy address.
return rootClient, nil
}

Expand All @@ -221,25 +211,9 @@ func (r *TCPAppResolver) clusterClientForAppFQDN(ctx context.Context, profileNam
continue
}
for _, zone := range clusterConfig.DNSZones {
if !isSubdomain(fqdn, zone) {
// The queried app fqdn is not a subdomain of this zone, skip it.
continue
}

// Found a matching cluster.

if zone == clusterConfig.ProxyPublicAddr {
// We don't need to validate a custom DNS zone if this is the proxy public address, this is a
// normal app public_addr.
if isDescendantSubdomain(fqdn, zone) {
return clusterClient, nil
}
// The queried app fqdn is a subdomain of this custom zone. Check if the zone is valid.
if err := r.customDNSZoneChecker.validate(ctx, clusterConfig.ClusterName, zone); err != nil {
// Return an error here since the FQDN does match this custom zone, but the zone failed to
// validate.
return nil, trace.Wrap(err, "validating custom DNS zone %q matching queried FQDN %q", zone, fqdn)
}
return clusterClient, nil
Comment on lines -224 to -242
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be basically "Given the FQDN of an app, if you find a matching custom DNS zone configured in the cluster, validate its TXT records and return the cluster client, unless it's a zone of the proxy service, then just skip the validation and return the client".

Now we do not care about the TXT records, so we just return the client if the FQDN on the app matches a zone for this cluster.

}
}
return nil, errNoMatch
Expand Down Expand Up @@ -392,8 +366,11 @@ func (i *appCertIssuer) IssueCert(ctx context.Context) (tls.Certificate, error)
return cert.(tls.Certificate), trace.Wrap(err)
}

func isSubdomain(appFQDN, suffix string) bool {
return strings.HasSuffix(appFQDN, "."+fullyQualify(suffix))
// isDescendantSubdomain checks if appFQDN belongs in the hierarchy of zone. For example, both
// foo.bar.baz.example.com and bar.baz.example.com belong in the hierarchy of baz.example.com, but
// quux.example.com does not.
func isDescendantSubdomain(appFQDN, zone string) bool {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming this as I was already confused by this in the past. For me "subdomain" means like a direct descendant, e.g. foo.example.com is a subdomain of example.com.

return strings.HasSuffix(appFQDN, "."+fullyQualify(zone))
}

// fullyQualify returns a fully-qualified domain name from [domain]. Fully-qualified domain names always end
Expand Down
95 changes: 0 additions & 95 deletions lib/vnet/customdnszonevalidator.go

This file was deleted.

28 changes: 1 addition & 27 deletions lib/vnet/vnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,6 @@ type testPack struct {
type testPackConfig struct {
clock clockwork.FakeClock
appProvider AppProvider

// customDNSZones is a map where the keys are custom DNS zones that should be valid, and the values are a
// slice of all Teleport clusters they should be valid for.
customDNSZones map[string][]string
}

func newTestPack(t *testing.T, ctx context.Context, cfg testPackConfig) *testPack {
Expand Down Expand Up @@ -130,21 +126,7 @@ func newTestPack(t *testing.T, ctx context.Context, cfg testPackConfig) *testPac

dnsIPv6 := ipv6WithSuffix(vnetIPv6Prefix, []byte{2})

lookupTXT := func(ctx context.Context, customDNSZone string) ([]string, error) {
clusters, ok := cfg.customDNSZones[customDNSZone]
if !ok {
return nil, nil
}
txtRecords := make([]string, 0, len(clusters))
for _, cluster := range clusters {
txtRecords = append(txtRecords, clusterTXTRecordPrefix+cluster)
}
return txtRecords, nil
}

tcpHandlerResolver, err := NewTCPAppResolver(cfg.appProvider,
withClock(cfg.clock),
withLookupTXTFunc(lookupTXT))
tcpHandlerResolver, err := NewTCPAppResolver(cfg.appProvider, withClock(cfg.clock))
require.NoError(t, err)

// Create the VNet and connect it to the other side of the TUN.
Expand Down Expand Up @@ -480,13 +462,9 @@ func TestDialFakeApp(t *testing.T) {
"echo.myzone.example.com",
"echo.nested.myzone.example.com",
"not.in.a.custom.zone",
"in.an.invalid.zone",
},
customDNSZones: []string{
"myzone.example.com",
// This zone will not have a valid TXT record because it is not included in the
// customDNSZones passed to newTestPack.
"an.invalid.zone",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After removing the TXT record requirement, this custom domain becomes yet another custom DNS zone in use. It's not that much different in behavior to myzone.example.com above, so I just removed it.

},
cidrRange: "192.168.2.0/24",
leafClusters: map[string]testClusterSpec{
Expand All @@ -511,9 +489,6 @@ func TestDialFakeApp(t *testing.T) {
p := newTestPack(t, ctx, testPackConfig{
clock: clock,
appProvider: appProvider,
customDNSZones: map[string][]string{
"myzone.example.com": {"root1.example.com", "another.random.cluster.example.com"},
},
})

validTestCases := []struct {
Expand Down Expand Up @@ -602,7 +577,6 @@ func TestDialFakeApp(t *testing.T) {
invalidTestCases := []string{
"not.an.app.example.com.",
"not.in.a.custom.zone",
"in.an.invalid.zone",
}
for _, fqdn := range invalidTestCases {
t.Run(fqdn, func(t *testing.T) {
Expand Down
53 changes: 21 additions & 32 deletions rfd/0163-vnet.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,38 +146,27 @@ spec:
- suffix: goteleport.com
```

In this example, whenever the user is logged in to the cluster where this `vnet_config`
configuration resource is present, VNet will install itself as the DNS
nameserver for subdomains of `example.com` and `goteleport.com`.
When the DNS server receives a query for any subdomain of those suffixes, it
will search the cluster for an app with a matching `public_addr`.

In the interest of being good citizens of the internet and to avoid hijacking
DNS resolution for domains which do not wish for us to do so, VNet will not
resolve requests for names under a custom DNS zone unless that zone contains a
specific DNS TXT record.
This TXT record must include the value `teleport-cluster=<proxy-address>`, e.g.
`teleport-cluster=teleport.example.com`.
If a single DNS zone is to be served by VNet in multiple Teleport clusters,
multiple TXT records can be configured on the domain, one for each cluster.
VNet will look up this TXT record with at the default nameservers configured in
the client OS, so the records should be public or available to all clients.

So if your Teleport proxy is hosted at `teleport.example.com` and you want
`api.example.com` to be resolvable within VNet, you need:

1. The app registered in Teleport must have `public_addr: api.example.com`.
2. The following `vnet_config` resource:
```
version: v1
kind: vnet_config
metadata:
name: vnet_config
spec:
custom_dns_zones:
- suffix: example.com
```
3. The following DNS TXT record on `example.com`: `teleport-cluster=teleport.example.com`.
#### Domain verification

Initial versions of VNet required the admin to configure a special TXT record on
the domain in the form of `teleport-cluster=<proxy-address>`. This prevented
customers from using domains not accessible over public DNS as custom DNS zones
in VNet. We considered changing this behavior so that VNet validates the TXT
records only if the domain lookup doesn't return an NXDOMAIN response. However,
after some consideration we've decided to completely drop domain verification
for the following reasons:

1. It had limited security benefits. DNS over UDP is an unreliable protocol,
any network MitM can spoof an NXDOMAIN response or even respond with the TXT
record to bypass this requirement.
1. It was an extra configuration burden that added time-to-value for VNet users.
Often you need another team and approval to add TXT records. Sometimes it's
just not possible.
1. We already require all custom domains to be added to the cluster vnet_config
by a Teleport cluster admin. If they already are okay with adding it there,
the TXT record is just another burden.
1. It prevents an entire class of potential VNet usecases for putting public
addresses behind VNet.

#### Leaf and parallel clusters

Expand Down
Loading