On 2/20/19 11:24 AM, Tom Lane wrote: > Pierre Ducroquet <p.p...@pinaraf.info> writes: >> For simple functions like enum_eq/enum_ne, marking them leakproof is an >> obvious fix (patch attached to this email, including also textin/textout). > > This is not nearly as "obvious" as you think. See prior discussions, > notably > https://www.postgresql.org/message-id/flat/31042.1546194242%40sss.pgh.pa.us > > Up to now we've taken a very strict definition of what leakproofness > means; as Noah stated, if a function can throw errors for some inputs, > it's not considered leakproof, even if those inputs should never be > encountered in practice. Most of the things we've been willing to > mark leakproof are straight-line code that demonstrably can't throw > any errors at all. > > The previous thread seemed to have consensus that the hazards in > text_cmp and friends were narrow enough that nobody had a practical > problem with marking them leakproof --- but we couldn't define an > objective policy statement that would allow making such decisions, > so nothing's been changed as yet. I think it is important to have > an articulable policy about this, not just a seat-of-the-pants > conclusion about the risk level in a particular function.
What if we provided an option to redact all client messages (leaving logged messages as-is). Separately we could provide a GUC to force all functions to be resolved as leakproof. Depending on your requirements, having both options turned on could be perfectly acceptable. Patch for discussion attached. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index e88c45d..8c32d6c 100644
*** a/src/backend/utils/cache/lsyscache.c
--- b/src/backend/utils/cache/lsyscache.c
***************
*** 46,51 ****
--- 46,54 ----
#include "utils/syscache.h"
#include "utils/typcache.h"
+ /* GUC to force functions to be treated as leakproof */
+ extern bool force_leakproof;
+
/* Hook for plugins to get control in get_attavgwidth() */
get_attavgwidth_hook_type get_attavgwidth_hook = NULL;
*************** get_func_leakproof(Oid funcid)
*** 1595,1600 ****
--- 1598,1610 ----
HeapTuple tp;
bool result;
+ /*
+ * If client message suppression was requested,
+ * treat all functions as leakproof
+ */
+ if (force_leakproof)
+ return true;
+
tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for function %u", funcid);
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 8b4720e..9941756 100644
*** a/src/backend/utils/error/elog.c
--- b/src/backend/utils/error/elog.c
*************** int Log_destination = LOG_DESTINATION_
*** 107,112 ****
--- 107,113 ----
char *Log_destination_string = NULL;
bool syslog_sequence_numbers = true;
bool syslog_split_messages = true;
+ bool suppress_client_messages = false;
#ifdef HAVE_SYSLOG
*************** send_message_to_frontend(ErrorData *edat
*** 3166,3189 ****
/* M field is required per protocol, so always send something */
pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_PRIMARY);
- if (edata->message)
- err_sendstring(&msgbuf, edata->message);
- else
- err_sendstring(&msgbuf, _("missing error text"));
! if (edata->detail)
{
! pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_DETAIL);
! err_sendstring(&msgbuf, edata->detail);
! }
! /* detail_log is intentionally not used here */
! if (edata->hint)
! {
! pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_HINT);
! err_sendstring(&msgbuf, edata->hint);
}
if (edata->context)
{
--- 3167,3197 ----
/* M field is required per protocol, so always send something */
pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_PRIMARY);
! /* skip sending if requested */
! if (!suppress_client_messages)
{
! if (edata->message)
! err_sendstring(&msgbuf, edata->message);
! else
! err_sendstring(&msgbuf, _("missing error text"));
! if (edata->detail)
! {
! pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_DETAIL);
! err_sendstring(&msgbuf, edata->detail);
! }
! /* detail_log is intentionally not used here */
!
! if (edata->hint)
! {
! pq_sendbyte(&msgbuf, PG_DIAG_MESSAGE_HINT);
! err_sendstring(&msgbuf, edata->hint);
! }
}
+ else
+ err_sendstring(&msgbuf, _("redacted message text"));
if (edata->context)
{
*************** send_message_to_frontend(ErrorData *edat
*** 3274,3283 ****
if (edata->show_funcname && edata->funcname)
appendStringInfo(&buf, "%s: ", edata->funcname);
! if (edata->message)
! appendStringInfoString(&buf, edata->message);
else
! appendStringInfoString(&buf, _("missing error text"));
if (edata->cursorpos > 0)
appendStringInfo(&buf, _(" at character %d"),
--- 3282,3297 ----
if (edata->show_funcname && edata->funcname)
appendStringInfo(&buf, "%s: ", edata->funcname);
! /* skip sending if requested */
! if (!suppress_client_messages)
! {
! if (edata->message)
! appendStringInfoString(&buf, edata->message);
! else
! appendStringInfoString(&buf, _("missing error text"));
! }
else
! appendStringInfoString(&buf, _("redacted message text"));
if (edata->cursorpos > 0)
appendStringInfo(&buf, _(" at character %d"),
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 156d147..e8a1617 100644
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** char *event_source;
*** 495,500 ****
--- 495,501 ----
bool row_security;
bool check_function_bodies = true;
+ bool force_leakproof;
/*
* This GUC exists solely for backward compatibility, check its definition for
*************** static struct config_bool ConfigureNames
*** 1911,1916 ****
--- 1912,1935 ----
false,
NULL, NULL, NULL
},
+
+ {
+ {"suppress_client_messages", PGC_POSTMASTER, ERROR_HANDLING_OPTIONS,
+ gettext_noop("Suppress all client messages."),
+ },
+ &suppress_client_messages,
+ false,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"force_leakproof", PGC_POSTMASTER, ERROR_HANDLING_OPTIONS,
+ gettext_noop("Force all functions to behave as if leakproof."),
+ },
+ &force_leakproof,
+ false,
+ NULL, NULL, NULL
+ },
/* End-of-list marker */
{
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7ac37fd..11e9897 100644
*** a/src/include/utils/elog.h
--- b/src/include/utils/elog.h
*************** extern int Log_destination;
*** 407,412 ****
--- 407,413 ----
extern char *Log_destination_string;
extern bool syslog_sequence_numbers;
extern bool syslog_split_messages;
+ extern bool suppress_client_messages;
/* Log destination bitmap */
#define LOG_DESTINATION_STDERR 1
signature.asc
Description: OpenPGP digital signature
