Skip to content

bpo-43762: Add audit events for loading of sqlite3 extensions #25246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Apr 26, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 7, 2021

@erlend-aasland
Copy link
Contributor Author

cc. @tiran

@erlend-aasland erlend-aasland force-pushed the sqlite-audit-load-ext branch from fcbf3a3 to 36f075f Compare April 7, 2021 12:53
@erlend-aasland
Copy link
Contributor Author

@tiran & @zooba, can one of you review this? FYI, the previous sqlite3 audit events were added in #14301 by Steve.

Copy link
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor tweaks, mainly for readability/ease of use.

I wonder if it's worth returning the connection object when it's created (through a new event in module.c) and then reference it in these events? That can then correlate these (and other) events with the file - we do this already for sockets.

After some thought, I think it's probably not worth it for these ones. The important information is in the extension being loaded, and it doesn't really relate to the connection at all. However, if we wanted to add it later, we couldn't. So might be worth doing now?

@erlend-aasland erlend-aasland force-pushed the sqlite-audit-load-ext branch from 20dda83 to ea0116c Compare April 24, 2021 23:08
@erlend-aasland erlend-aasland force-pushed the sqlite-audit-load-ext branch from ea0116c to dd2ffde Compare April 24, 2021 23:13
@erlend-aasland
Copy link
Contributor Author

FYI, rebased onto main to resolve conflicts.

Copy link
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with the doc update suggestions

Erlend Egeberg Aasland and others added 2 commits April 26, 2021 22:02
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
@erlend-aasland
Copy link
Contributor Author

Approved with the doc update suggestions

Thanks for reviewing! BTW, I used /handle iso. /result (ctypes uses that already). I figured it aligned better with the SQLite jargon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants