Fix magic dns and uppercase letters #387
Conversation
This function is called often. Normalization of the hostname will be written in database.
poll.go
Outdated
| Msg("Found machine in database") | ||
|
|
||
| machine.Name = req.Hostinfo.Hostname | ||
| hname, err := NormalizeNamespaceName( |
There was a problem hiding this comment.
We should probably generalise the name of this function now.
There was a problem hiding this comment.
Perhaps make them two different functions? NormalizeHostname and NormalizeNamespaceName?
There was a problem hiding this comment.
I'm in favor of something generic since hostname and namespace are both part of a FQDN. So NormalizeName or NormalizeDomainName or NormalizeHostname should work too.
I think I'll go with NormalizeName if that's ok with you.
There was a problem hiding this comment.
I think FQDN makes sense to have in the name, as that is what it is normalising for
| Str("hostinfo.name", req.Hostinfo.Hostname). | ||
| Err(err) | ||
| } | ||
| machine.Name = hname |
There was a problem hiding this comment.
hmm, doing this here feels kind of wrong, I think doing it here:
https://github.com/juanfont/headscale/blob/main/api.go#L144
Would be more correct, as this is when the machine registers.
However we need a migration path. It could potentially be done in the DB migration, what do you think?
We should probably also make a tracking ticket of a unique constraint for a machine name per user, so we dont have collisions.
There was a problem hiding this comment.
You're right, that was my first intention. When I found this solution I thought that we could get rid of the need of a database migration, but the --hostname at tailscale up wouldn't work at all.
For the machine name per user I think it would be more a unique name on all the headscale instance. If we tag a machine, this machine should not be accessed by the name of the user that created it (it's currently not implemented).
There was a problem hiding this comment.
After re-reading the code. What should we do in this place ? If we keep previous behavior, normalized hostname would be replaced. If we modify in https://github.com/juanfont/headscale/blob/main/api.go#L144 we would not be able to update the hostname with tailscale up --hostname since at line
Line 126 in 435ee36
There was a problem hiding this comment.
We could migrate all of them on server start up as part of db.go, but that would run everytime :/
There was a problem hiding this comment.
After some tests each time we update the hostname with tailscale up --hostname xxx a call to /machine/:id/map is done. We can also observe the following logs in the tailscaled daemon:
cmd-prometheus1-1 | 2022/03/07 08:49:11 EditPrefs: MaskedPrefs{ControlURL="https://my.redacted.headscale.domain" WantRunning=true Hostname="prometheus2"}
cmd-prometheus1-1 | 2022/03/07 08:49:11 control: HostInfo: {"IPNVersion":"1.22.0-t4e0b00ad8","BackendLogID":"3a5dce2711b4137729be89a95f315ee9ac733cf3a55dd2555c31aac5ab93a328","OS":"linux","OSVersion":"Alpine Linux v3.15; kernel=5.15.2-arch1-1","Hostname":"prometheus2","GoArch":"amd64","Services":[{"Proto":"peerapi4","Port":58436},{"Proto":"peerapi-dns-proxy","Port":1}],"NetInfo":{"MappingVariesByDestIP":false,"HairPinning":false,"WorkingIPv6":false,"WorkingUDP":true,"UPnP":false,"PMP":false,"PCP":false,"PreferredDERP":8,"DERPLatency":{"1-v4":0.088316282,"10-v4":0.158354527,"11-v4":0.237721537,"12-v4":0.099872237,"2-v4":0.15813595,"4-v4":0.026846961,"6-v4":0.224276797,"7-v4":0.254514052,"8-v4":0.018363134,"9-v4":0.121180425}}}
cmd-prometheus1-1 | 2022/03/07 08:49:11 control: PollNetMap: stream=false :0 ep=[90.27.176.219:60654 172.18.0.3:60654]
cmd-prometheus1-1 | 2022/03/07 08:49:11 wgengine: Reconfig done
cmd-prometheus1-1 | 2022/03/07 08:49:11 authReconfig: ra=false dns=true 0x01: <nil>
cmd-prometheus1-1 | 2022/03/07 08:49:11 [unexpected] peerapi listen("fd7a:115c:a1e0::1") error: listen tcp6 [fd7a:115c:a1e0::1]:0: bind: cannot assign requested address
cmd-prometheus1-1 | 2022/03/07 08:49:11 peerapi: serving on http://100.64.0.1:58436
I could not find solid proof in reasonable time from the code of tailscale that each modification of the preference would lead to a NetPollMap call, but it's very likely.
There was a problem hiding this comment.
So we need it in both places?
There was a problem hiding this comment.
I think that would be better yes !
There was a problem hiding this comment.
Lets give it a go then :)
There was a problem hiding this comment.
Ok, I'll made the modifications on the name of the function and add this code to both places tonight.
acls.go
Outdated
| ) | ||
| } | ||
| grp, err := NormalizeNamespaceName(group, stripEmailDomain) | ||
| grp, err := NormalizeName(group, stripEmailDomain) |
There was a problem hiding this comment.
I think as mentioned this is a bit to generic, for a unknown user to understand the context, it needs to either have FQDN or Domain or some reference to the RFC and limitations in the name.
This will fix #363 by applying normalization on the hostname. This normalization function will lowercase capital letters and replace forbidden chars. If it cannot find a solution it will raise an error on headscale server.