Magnus Hagander writes:
2010/2/22 Tom Lane <>:
If you want to do it, I'd be fine with it.
Seems easy enough, see attached. Comments?
Looks mostly okay to me, a few notes:

+ if (ssl_renegotiation_limit && port->count > ssl_renegotiation_limit * 1024)

You need "1024L" there to avoid risk of integer overflow when long is
wider than int --- MAX_KILOBYTES is set on the assumption that any such
multiplies will be done in long arithmetic. It doesn't matter that
count will never be wider than int, it can still get the wrong answer.

Also, the comment attached to ssl_renegotiation_limit needs to be fixed
to mention that zero disables renegotiation.

Also, the coding seems a bit confused about whether the
ssl_renegotiation_limit GUC exists when USE_SSL isn't set. I think we
have a project policy about whether GUCs should still exist when the
underlying support isn't compiled, but I forget what it is :-(.
Anyway you need to test that the patch compiles with USE_SSL off.

Also the xreflabel for the variable in the docs isn't consistent,
and you failed to mention the default value.
This version is set to superuser only. It's a security related
feature, so just letting a random user turn it off may be seen as
wrong. On the other hand, this is just about the connection security,
and if we have a malicious user on the other end, he can do a lot
worse things than disable renegotiation (such as resending the
plaintext after it's been decrypted).
I'd therefore suggest we make it USERSET. Anything wrong in that discussion?
SUSET seems less surprising to me. I agree that it's hard to make
a concrete case for a user doing anything terribly bad with it,
but on the other hand is there much value in letting it be USERSET?
Anyway it's not very important to me either way.
Also, do we want to add a specific <note> to the documentation saying
this is the way around broken SSL libraries? Or leave that to release
notes? Or just leave it to the mailinglist archives?
I think a short note in the description of the variable wouldn't be out
of place.

regards, tom lane

Search Discussions

Discussion Posts


Follow ups

Related Discussions



site design / logo © 2022 Grokbase