Re: Replay attack of query cancel - Mailing list pgsql-hackers

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> Tom Lane wrote:
>>> * Server accepts two different styles of cancel messages, identified
>>> by different protocol numbers.
> 
>> With the additional point that there is a GUC variable to turn this off
>> or warn about it, right?
> 
> I see pretty much no value in that.

The server is the point where you enforce security policies. The server
is where you say that SSL is required, for example. The same way, this
would let you at the server level say that secure cancels are required.


>>> The main drawback I can see to keeping this backward-compatible is that
>>> keeping the cancel key to 32 bits could still leave us vulnerable to
>>> brute force attacks: once you've seen a cancel message, just try all
>>> the possible keys till you get a match, and then you can generate a
>>> cancel that will work.  Can we refine the HMAC idea to defeat that?
>>> Otherwise we're assuming that 2^32 HMACs take longer than the useful
>>> life of a cancel key, which doesn't seem like a very future-proof
>>> assumption.
> 
>> Well, you're also going to have to increment <n> every time. We could
>> just cap <n> at <arbitrary level>. Say you can only cancel queries on a
>> single connection a million times or so. It's not perfect, but it gets
>> you somewhere.
> 
> Once you've brute-forced the secret key, you can just use an <n> that's
> somewhat more than the last one the client used, assuming you've been
> sniffing the connection the whole time.  Or use one that's just a bit
> less than what you can predict the server will take.
> 
> Not only do you get to kill the current query, but you'll have prevented
> the client from issuing legitimate cancels after that, since it won't
> know you bumped the server's <n> value.  So the idea still needs work.

Yeah, I like the idea below much better.


>> Another option would be to send a new, longer, cancel key as part of the
>> separate parameter we're sending during startup (when we're indicating
>> which version we support). Then we'll use the longer cancel key if we're
>> dealing with "new style cancel" but keep the old 32-bit one for
>> backwards compatibility.
> 
> Yeah.  So then we just need one added parameter: secure_cancel_key =
> string.

Yup. Seems a whole lot cleaner.


> BTW, should we make all of this conditional on the use of an SSL
> connection?  If the original sending of the cancel key isn't secure
> against sniffing, it's hard to see what anyone is buying with all the
> added computation.

Not sure. In practice it makes no difference, but our code is more
readable with less #ifdefs and branches. I don't think the added
computation makes any noticable difference at all in the normal
non-attacker scenario, so I'd just as well leave it in.

//Magnus