Do not log inserted or updated columns against the SELECT grant.#323
Do not log inserted or updated columns against the SELECT grant.#323dwsteele wants to merge 2 commits into
Conversation
log_select_dml() builds auditPerms as the union of all permissions the statement requires, which for INSERT ... RETURNING or UPDATE ... RETURNING includes ACL_SELECT alongside ACL_INSERT/ACL_UPDATE. The inserted- and updated-column checks passed this combined mask to audit_on_any_attribute(), so a column-level SELECT grant matched a column that was only written (inserted or updated) but never read. This produced a spurious audit record, e.g. GRANT SELECT (col2) caused INSERT INTO t (col1, col2) VALUES (...) RETURNING col1 to be logged even though col2 is not returned. Check insertedCols against ACL_INSERT and updatedCols against ACL_UPDATE so each column set is tested only for the access it actually represents. The select-column check already used ACL_SELECT and is unchanged. Add regression coverage with INSERT/UPDATE ... RETURNING cases that exercise both the written-but-not-returned (not logged) and returned (logged) paths. Reported-by: @fabiobaiao
| -- Object logged because of: | ||
| -- select (col2) on test5 (col2 is read via RETURNING) | ||
| UPDATE public.test5 | ||
| SET col1 = 2 |
There was a problem hiding this comment.
Are non-matching columns here significant? i.e., would SET col2 = ... returning col2 behave the same way? Also do any OLD/NEW references matter here, or am I thinking of a trigger-only behavior?
There was a problem hiding this comment.
Are non-matching columns here significant?
Yes. We are trying to show that only the returning clause generates an audit record.
i.e., would SET col2 = ... returning col2 behave the same way?
It would, since the part triggering the audit is the returning clause. I added a test in 4625539 to demonstrate this.
Also do any OLD/NEW references matter here, or am I thinking of a trigger-only behavior?
They are not significant here since we are only examining the column ACLs. The actual values are not relevant. I added a test in 4625539 to prove this.
log_select_dml() builds auditPerms as the union of all permissions the statement requires, which for INSERT ... RETURNING or UPDATE ... RETURNING includes ACL_SELECT alongside ACL_INSERT/ACL_UPDATE. The inserted- and updated-column checks passed this combined mask to audit_on_any_attribute(), so a column-level SELECT grant matched a column that was only written (inserted or updated) but never read. This produced a spurious audit record, e.g. GRANT SELECT (col2) caused INSERT INTO t (col1, col2) VALUES (...) RETURNING col1 to be logged even though col2 is not returned.
Check insertedCols against ACL_INSERT and updatedCols against ACL_UPDATE so each column set is tested only for the access it actually represents. The select-column check already used ACL_SELECT and is unchanged.
Add regression coverage with INSERT/UPDATE ... RETURNING cases that exercise both the written-but-not-returned (not logged) and returned (logged) paths.
Reported-by: @fabiobaiao