Dex: Kubernetes CRD storage hashes incorrectly

[simonswine]

I was trying to create some bootstraping CRDs for running dex based on the CRD storage provider.

I had some issues setting the correct metadata.name on my objects.

After digging a little bit deeper I figured that we don't hash correctly and any input to idToName (https://github.com/dexidp/dex/blob/f6741d18376fbc8d71aef29d6e709f5400654bec/storage/kubernetes/client.go#L71)
will create a longer name than kubernetes allows (> 63 chars).

I created this little example: https://play.golang.org/p/kWO9-kUXfNA

I think this is quite critical for users with longer email addresses than 32 characters or a longer Client ID. I am happy to provide an implementation based on proper hashing. But it will certainly be a quite interesting issues to solve with backwards compatibility. Also I have the feeling collision handling isn't implemented fully/correctly right now. This will become more important once we actually can have collisions.

I also suggestion using something more standard like sha256. It's 32 bytes will be 52 chars at most which will be within the limit of kubernetes object names (63 chars)

[simonswine]

@ericchiang you might have some more input to this as author of #631

[ericchiang]

Ha, idToName is wrong. It should be:

var encoding = base32.NewEncoding("abcdefghijklmnopqrstuvwxyz234567")

func idToName(s string, h func() hash.Hash) string {
    w := h()
    io.WriteString(w, s)
    return strings.TrimRight(encoding.EncodeToString(w.Sum(nil)), "=")
}

Well that's really bad. We're basically not hashing.

I don't know how we're going to fix this in a backward incompatible way. This mapping needs to be consistent between versions of dex.

[srenatus]

I don't know how we're going to fix this in a backward incompatible way. This mapping needs to be consistent between versions of dex.

...or be migrated on update? I guess that would be an alternative. Might be trouble if there's multiple instances, but in the end, it would be _less wrong_ 😉

[ericchiang]

How about this:

• In vN we attempt to GET the correct hash value in addition to the wrong one.
• In vN+1 we start POSTing the correct hash value.

At some arbitrary point in the future we could depreciate the wrong one.