Lxd: Make it so we always have a `user` context on requests

[stgraber]

We currently have a user context field which we fill with the certificate fingerprint or candid username when available.

To make it more generally useful, I believe we should make sure it's always set and is prefixed with the type of connection used.
That would allow values such as:

• candid:stgraber@usso
• unix:1000
• tls:e30cd729e816fc9dc3c460360929e25fdd2c141db6d6d4ebba3f60c2a5a3feeb

Which will end up being quite useful especially when considering #8011

[IngCr3at1on]

I'd like to work on this.

[stgraber]

Thanks, assigning it to you.

[IngCr3at1on]

I'd like to start on this this evening. Since this will be my first contribution to LXD is there any direction you can provide as a starting point?

[tomponline]

@IngCr3at1on follow the build instructions at https://github.com/lxc/lxd#installing-lxd-from-source to get latest version running and then you can make changes the source and iterate.

[IngCr3at1on]

@tomponline I was more referring to the issue in question; the code base is _not_ small as I'm sure you're aware it would be nice to have a starting point for what I'm actually looking for in the code; just a nudge, I'm more than willing to read and learn the code once I know the best place to start from.

[tomponline]

@IngCr3at1on I believe what @stgraber is referring to are the instances of context.WithValue(r.Context(), "username", username) in the code base, as well as the usage of it r.Context().Value("username").(string).

There are functions like userIsAdmin and userHasPermission that will likely need to be made aware that username value can be prefixed with a type: pattern.

Also, validation may need to be updated to reject usernames with colons in them.

[stgraber]

I'm not concerned about usernames with colons in them if we always prefix with the protocol and the protocol itself may not contain colons.

[stgraber]

And yeah, I suspect updating Authenticate() to return usernames in the suggested format and for all auth sources should do the trick.

[stgraber]

For unix, getting the ucred will be slightly tricky I suspect. I may look up some example logic for that (from our devlxd code).

[IngCr3at1on]

Thanks y'all, I'll look into this after work today. That should be enough for me to get a lay of the land and at least get things rolling.

[tomponline]

@stgraber I noticed there is also a protocol context value, which is either "tls" or "cluster", does this have any relation to this task?

[stgraber]

Oh, right, forgot we had that. So then I think it's fine to not prefix username with the protocol but just make sure that whenever we log the username, we also log the protocol alongside it.

[IngCr3at1on]

The more I look at this it doesn't seem particularly straight forward or easy (as the issue itself implies) to me, there are several cases where I can see that we would want to return a user value from Authenticate (instead of an empty string) but getting the data for that doesn't really seem directly possible in several of these:

    // Allow internal cluster traffic
    if r.TLS != nil {
        cert, _ := x509.ParseCertificate(d.endpoints.NetworkCert().KeyPair().Certificate[0])
        clusterCerts := map[string]x509.Certificate{"0": *cert}
        for i := range r.TLS.PeerCertificates {
            trusted, _ := util.CheckTrustState(*r.TLS.PeerCertificates[i], clusterCerts, nil, false)
            if trusted {
                // TODO: return a user value
                return true, "", "cluster", nil
            }
        }
    }

"Internal cluster traffic" to me makes it sound like it's not even coming from a user in this case, so if anything all I can think would be to add "cluster" to the user value as well... Perhaps I'm misunderstanding the meaning of "internal cluster traffic"?

    // Local unix socket queries
    if r.RemoteAddr == "@" {
        // TODO: return a user value
        return true, "", "unix", nil
    }

This is the case you mentioned @stgraber getting the uid is as simple as a call to os.Getuid() but then to get anything useful from that I believe we'd have to search /etc/passwd for the provided uid, this case actually seems relatively easy but I wonder if in most non-source installed cases it would simply show up as lxd (this seems unhelpful if that's the case).

        // Valid macaroon with no identity information
        // TODO: return a user value
        return true, "", "candid", nil

This is the case I literally have no idea on, if we've made it here for my perspective there is no way to fill in this value...

---

I'm wondering if it's feasible for me to even continue here; I haven't wound up having as much time to work on this as I hoped and the code base is as I noted before not small, while it appears most of what I would need that exists is in this one file for all I know I'm missing a much bigger picture than I realize. There's also the issue of even though I like LXD I'm not currently using it for anything anymore and with the lack of test coverage, it makes it hard for me to be confident in anything I do without using it.

[stgraber]

I think there's very little to do actually. Looking at Authenticate() the only cases we care about is when it's returning true.
The only of those cases where the user isn't already set is the local unix case.

That one is admittedly a bit involved to resolve though as you need to extract the client uid for the unix socket connection and then resolve it to the uid. I just spent a few minutes poking at that and I think I have it working.

[stgraber]

DBUG[10-27|19:05:37] Handling                                 protocol=unix url=/1.0 username=stgraber ip=@ method=GET
DBUG[10-27|19:05:37] Handling                                 ip=@ method=GET protocol=unix url=/1.0 username=stgraber
DBUG[10-27|19:05:44] Handling                                 protocol=unix url=/1.0 username=root ip=@ method=GET
DBUG[10-27|19:05:44] Handling                                 ip=@ method=GET protocol=unix url=/1.0 username=root