Greetings, * Michael Paquier (mich...@paquier.xyz) wrote: > On Tue, May 23, 2023 at 05:02:50PM -0400, Stephen Frost wrote: > > * Jacob Champion (jchamp...@timescale.com) wrote: > >> As touched on in past threads, our SCRAM implementation is slightly > >> nonstandard and doesn't always protect the entirety of the > >> authentication handshake: > >> > >> - the username in the startup packet is not covered by the SCRAM > >> crypto and can be tampered with if channel binding is not in effect, > >> or by an attacker holding only the server's key > > > > Encouraging channel binding when using TLS certainly makes a lot of > > sense, particularly in environments where the trust anchors are not > > strongly managed (that is- if you trust the huge number of root > > certificates that a system may have installed). Perhaps explicitly > > encouraging users to *not* trust every root server installed on their > > client for their PG connections would also be a good idea. We should > > probably add language to that effect to this section. > > Currently the user name is not treated by the backend, like that in > auth-scram.c: > > /* > * Read username. Note: this is ignored. We use the username from the > * startup message instead, still it is kept around if provided as it > * proves to be useful for debugging purposes. > */ > state->client_username = read_attr_value(&p, 'n'); > > Could it make sense to cross-check that with the contents of the > startup package instead, with or without channel binding?
Not without breaking things we support today and for what seems like an unclear benefit given that we've got channel binding today (though perhaps we need to make sure there's ways to force it on both sides to be on and to encourage everyone to do that- which is what this change is generally about, I think). As I recall, the reason we do it the way we do is because the SASL spec that SCRAM is implemented under requires the username to be utf8 encoded while we support other encodings, and I don't think we want to break non-utf8 usage. Thanks, Stephen
signature.asc
Description: PGP signature