Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Clemens Ladisch 
Richard Hipp wrote:
> On 5/11/17, Clemens Ladisch  wrote:
>> Richard Hipp wrote:
>>> ** ^When a table is referenced by a [SELECT] but no column values are
>>> ** extracted from that table (for example in a query like
>>> ** "SELECT count(*) FROM tab") then the [SQLITE_READ] authorizer callback
>>> ** is invoked once for that table with a NULL column name.
>>
>> The documentation fails to mention that in this case, the authorizer
>> callback does not authorize anything, i.e., that its return value is
>> ignored.  (Assuming that this the desired behaviour ...)
>
> I don't understand what you are saying.  Can you give an example of
> the undocumented behavior?

Sorry, I overlooked that sqlite3AuthCheck records the error as a side effect.


Regards,
Clemens
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Gwendal Roué 

> Le 11 mai 2017 à 14:29, Richard Hipp  a écrit :
> 
> On 5/11/17, Gwendal Roué  wrote:
> 
>> 1. Existing callbacks that catch SQLITE_READ expect a non-NULL column
>> 
> 
> Very well.  The behavior has been changed so that an SQLITE_READ with
> an empty-string column name, instead of a NULL column name, is invoked
> when a table referenced but not used.  Also, the documentation has
> been updated to clearly state that any of the string arguments to the
> authorizer callback may be NULL.

Many thanks, Richard, for your understanding and SQLite :-)

Gwendal

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Richard Hipp 
On 5/11/17, Gwendal Roué  wrote:

> 1. Existing callbacks that catch SQLITE_READ expect a non-NULL column
>

Very well.  The behavior has been changed so that an SQLITE_READ with
an empty-string column name, instead of a NULL column name, is invoked
when a table referenced but not used.  Also, the documentation has
been updated to clearly state that any of the string arguments to the
authorizer callback may be NULL.

-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Richard Hipp 
On 5/11/17, Clemens Ladisch  wrote:
> Richard Hipp wrote:
>> ** ^When a table is referenced by a [SELECT] but no column values are
>> ** extracted from that table (for example in a query like
>> ** "SELECT count(*) FROM tab") then the [SQLITE_READ] authorizer callback
>> ** is invoked once for that table with a NULL column name.
>
> The documentation fails to mention that in this case, the authorizer
> callback does not authorize anything, i.e., that its return value is
> ignored.  (Assuming that this the desired behaviour ...)

I don't understand what you are saying.  Can you give an example of
the undocumented behavior?
-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Clemens Ladisch 
Richard Hipp wrote:
> ** ^When a table is referenced by a [SELECT] but no column values are
> ** extracted from that table (for example in a query like
> ** "SELECT count(*) FROM tab") then the [SQLITE_READ] authorizer callback
> ** is invoked once for that table with a NULL column name.

The documentation fails to mention that in this case, the authorizer
callback does not authorize anything, i.e., that its return value is
ignored.  (Assuming that this the desired behaviour ...)


Regards,
Clemens
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-11 Thread Gwendal Roué 

> Le 10 mai 2017 à 18:22, Richard Hipp  a écrit :
> 
> On 5/10/17, Dominique Devienne  wrote:
>> 
>> We haven't heard from Richard, but I hope we will eventually.
>> 
> 
> No new authorizer codes will be added, since that would present
> compatibility problems for legacy authorizer callbacks.  Instead, the
> fix is to invoke the authorizer callback with SQLITE_READ but with a
> NULL column name for any table that is referenced but for which no
> columns are extracted.
> 
> This change is more likely to be compatible with legacy authorizer
> callbacks.  In particular, the authorizer callback used by Fossil
> (https://www.fossil-scm.org/fossil/artifact/ee53ffbf7?ln=161-221)
> continues to work fine, and with the enhanced SQLITE_READ, no prevents
> users from creating a report using
> 
> SELECT count(*) FROM user
> 
> That returns the number of users, for example.
> 
> The fix is implemented by https://www.sqlite.org/src/timeline?c=92ab1f72 
> 

Thanks a lot, Richard!

I would naively expect SQLITE_READ with a NULL column a bigger threat for 
backward compatibility than a new authorizer code, because:

1. Existing callbacks that catch SQLITE_READ expect a non-NULL column

2. Existing callbacks are more likely to implement a blacklist, and use the 
authorizer code in a switch statement that does not do anything in its default 
case. The reason for favouring blacklisting over whitelisting is that the list 
of authorizer codes is not documented as closed, and that the documentation 
shows that the list of authoriser codes has already changed in the past (the 
no-longer used SQLITE_COPY).

The example below (a callback that denies access to the `token` column of the 
`users` table) would be harmed by SQLITE_READ with a NULL column because 
strcmp() isn't supposed to accept NULL input:

int denyTokenAccess(void *pUserData, int code, const char *s1, const char *s2, 
const char *s3, const char *s4) {
switch(code) {
case SQLITE_READ:
if (strcmp(s1, "users") == 0 && strcmp(s2, "token") == 0) {
return SQLITE_DENIED;
} else {
return SQLITE_OK;
}
default:
return SQLITE_OK
}
}

What do you think?
Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-10 Thread Richard Hipp 
On 5/10/17, Dominique Devienne  wrote:
>
> We haven't heard from Richard, but I hope we will eventually.
>

No new authorizer codes will be added, since that would present
compatibility problems for legacy authorizer callbacks.  Instead, the
fix is to invoke the authorizer callback with SQLITE_READ but with a
NULL column name for any table that is referenced but for which no
columns are extracted.

This change is more likely to be compatible with legacy authorizer
callbacks.  In particular, the authorizer callback used by Fossil
(https://www.fossil-scm.org/fossil/artifact/ee53ffbf7?ln=161-221)
continues to work fine, and with the enhanced SQLITE_READ, no prevents
users from creating a report using

 SELECT count(*) FROM user

That returns the number of users, for example.

The fix is implemented by https://www.sqlite.org/src/timeline?c=92ab1f72
-- 
D. Richard Hipp
d...@sqlite.org
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-10 Thread Gwendal Roué 

> Le 10 mai 2017 à 15:06, Dominique Devienne  a écrit :
> 
> On Wed, May 10, 2017 at 1:35 PM, Gwendal Roué 
> wrote:
> 
>>> Le 9 mai 2017 à 15:41, Gwendal Roué  a écrit :
 How are you going to handle TRIGGERs ?
>>> 
>>> That's a very good question.
>> 
>> Very good news: foreign keys and triggers are 100% handled by authorizer
>> callbacks, for free :-D
>> 
> 
> Thanks for confirming. That's what I thought, from previous discussions on
> this list.
> 
> We haven't heard from Richard, but I hope we will eventually.

So do I.

Since my real topic is answering the question "were the results of statement S 
modified by transaction T?", I can imagine how this SQLITE_READ_TABLE proposal 
looks indirect, and potentially misled.

I tried to explain before that this question requires a lot of careful use of 
commit/rollback/update hooks on top of authorizer callbacks in order to provide 
as few false positives as possible.

A better written feature request could remain strictly at the statement level. 
Here is a documentation for a potential sqlite3_stmt_independent() function, 
inspired from the documentation of sqlite3_stmt_readonly:

# Statement Independence Test

int sqlite3_stmt_independent(sqlite3_stmt*, sqlite3_stmt*);

The sqlite3_stmt_independent(S1, S2) returns true (non-zero) if and 
only if the prepared statement S1 makes no changes to the database table and 
columns used by the prepared statement S2. Changes that are taken in account 
are direct changes to the table and columns used by S2, but also indirect 
changes through foreign keys cascades, and indirect changes through triggers.

For example, given the following SQL statement:

SELECT a, b FROM t1;

The following statements are considered independent:

SELECT * FROM t1;
INSERT INTO t2 (...);
UPDATE t1 SET c = c + 1;

The following statements are considered dependent:

DELETE FROM t1;
UPDATE t1 SET a = 1;
INSERT INTO t1 (a, b) VALUES (1, 2);

Note that application-defined SQL functions or virtual tables might 
change the database indirectly as a side effect. For example, if an application 
defines a function "eval()" that calls sqlite3_exec(), then the following SQL 
statement would change the database through side-effects:

SELECT eval('DELETE FROM t1') FROM t2;

But because the SELECT statement does not change the database directly, 
sqlite3_stmt_independent would still return true, regardless of the second 
parameter.


> I very much agree with you the authorizer API should be "complete" and 
> "exhaustive" in what is truly accessed.

That's my current strategy :-)

The core team may prefer the sqlite3_stmt_independent(S1, S2) function above. 
But I'm not sure: currently authorizer callbacks are called during the parsing 
phase: SQLite doesn't internally keep any information about the impact of a 
statement besides its execution plan. An eventual sqlite3_stmt_independent 
function may be difficult to implement, depending on how obsfuscated is the 
information it needs after the statement has been compiled. I don't know the 
inner SQLite guts enough.

With SQLITE_READ_TABLE, we have a way to let the library user gather the needed 
information, with a simple implementation (the only one I could provide with my 
current knowledge of the library).

> A flag could be set to not issue the new calls you propose, for backward 
> compatibility purposes (and also perhaps because both FKs and TRIGGERs are an 
> implementation "detail" of the schema, that not all clients should be aware 
> of, unless explicitly requested by the client, perhaps even checked by the 
> authorizer itself, to complete the circle :)). --DD

I don't know if backward compatibility is an issue here: code that did not 
check for SQLITE_READ_TABLE has no reason to break after SQLITE_READ_TABLE has 
been introduced.

Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-10 Thread Dominique Devienne 
On Wed, May 10, 2017 at 1:35 PM, Gwendal Roué 
wrote:

> > Le 9 mai 2017 à 15:41, Gwendal Roué  a écrit :
> >> How are you going to handle TRIGGERs ?
> >
> > That's a very good question.
>
> Very good news: foreign keys and triggers are 100% handled by authorizer
> callbacks, for free :-D
>

Thanks for confirming. That's what I thought, from previous discussions on
this list.

We haven't heard from Richard, but I hope we will eventually.

I very much agree with you the authorizer API should be "complete" and
"exhaustive"
in what is truly accessed. A flag could be set to not issue the new calls
you propose, for
backward compatibility purposes (and also perhaps because both FKs and
TRIGGERs
are an implementation "detail" of the schema, that not all clients should
be aware of,
unless explicitly requested by the client, perhaps even checked by the
authorizer itself,
to complete the circle :)). --DD
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-10 Thread Gwendal Roué 

> Le 9 mai 2017 à 15:41, Gwendal Roué  a écrit :
> 
>>> As a reminder, I intend to use the authorisation system in order to tell if 
>>> a statement has an opportunity to impact on another statement, as a support 
>>> for a general database observation feature.
>> 
>> How are you going to handle TRIGGERs ?  The authoriser analyses each 
>> statement individually.  It will not tell you everything that’s in the 
>> TRIGGER until it decides to execute those statements.
> 
> That's a very good question.

Very good news: foreign keys and triggers are 100% handled by authorizer 
callbacks, for free :-D

How much a boon is SQLite, frankly ???

Look:

CREATE TABLE table1 ( ... );
CREATE TABLE table2 ( ... );
CREATE TRIGGER trigger1 AFTER INSERT ON table2 BEGIN DELETE FROM table1; END;

INSERT INTO table2 (id) VALUES (NULL);
-- SQLITE_INSERT table2 main
-- SQLITE_DELETE table1 main trigger1 :-)

Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-09 Thread Dominique Devienne 
On Tue, May 9, 2017 at 3:02 PM, Simon Slavin  wrote:

> On 9 May 2017, at 7:23am, Gwendal Roué  wrote:
> > As a reminder, I intend to use the authorisation system in order to tell
> if a statement has an opportunity to impact on another statement, as a
> support for a general database observation feature.
> How are you going to handle TRIGGERs ?  The authoriser analyses each
> statement individually.  It will not tell you everything that’s in the
> TRIGGER until it decides to execute those statements.
>
> Aren't triggers compiled into statements themselves? In that case, if
authorizers are called on the full statement, I don't see why it wouldn't
work.


> I don’t think the authorisation system can be efficiently used in this
> way.  If you’re analysing statements speculatively you’ll get more
> information out of EXPLAIN.


On the contrary, a few months bad I was machine-parsing EXPLAIN, and was
told to use authorizers instead.
So it's probably completely appropriate, and I don't see why it wouldn't be
efficient. --DD
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-09 Thread Gwendal Roué 

> Le 9 mai 2017 à 15:02, Simon Slavin  a écrit :
> 
> On 9 May 2017, at 7:23am, Gwendal Roué  > wrote:
> 
>> As a reminder, I intend to use the authorisation system in order to tell if 
>> a statement has an opportunity to impact on another statement, as a support 
>> for a general database observation feature.
> 
> I’ve read your proposed mechanism.  Providing support for all those callbacks 
> looks like it would considerably slow down SQLite.

Observation is an opt-in service. The user decides which queries are observed.

I haven't done any benchmark yet. Still, I care about performance, and GRDB is 
the fastest wrapper in its category: 
https://github.com/groue/GRDB.swift/wiki/Performance.

Database observation is such an awesome service I could pay a little bit for 
it, though. If performance hit is too high (yet to be measured), then the user 
can still send change notifications in another way.

> How are you going to handle TRIGGERs ?  The authoriser analyses each 
> statement individually.  It will not tell you everything that’s in the 
> TRIGGER until it decides to execute those statements.

That's a very good question. I have indeed to think about triggers, and also 
foreign key cascades.

Unless I'm wrong, cascades can be handled through foreign keys introspection. 
Triggers, on the other side...

Well, if triggers can not be handled, then the documentation will have to tell 
it in an explicit way: this does not invalidate the whole idea.

> I don’t think the authorisation system can be efficiently used in this way.  
> If you’re analysing statements speculatively you’ll get more information out 
> of EXPLAIN.

Thanks for the suggestion. Even if EXPLAIN can give enough information, this 
does not invalidate the information provided by authorizer callbacks.

Gwendal

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-09 Thread Simon Slavin 

On 9 May 2017, at 7:23am, Gwendal Roué  wrote:

> As a reminder, I intend to use the authorisation system in order to tell if a 
> statement has an opportunity to impact on another statement, as a support for 
> a general database observation feature.

I’ve read your proposed mechanism.  Providing support for all those callbacks 
looks like it would considerably slow down SQLite.

How are you going to handle TRIGGERs ?  The authoriser analyses each statement 
individually.  It will not tell you everything that’s in the TRIGGER until it 
decides to execute those statements.

I don’t think the authorisation system can be efficiently used in this way.  If 
you’re analysing statements speculatively you’ll get more information out of 
EXPLAIN.

Simon.
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-09 Thread Gwendal Roué 

> Le 9 mai 2017 à 08:23, Gwendal Roué  a écrit :
> 
> As a reminder, I intend to use the authorisation system in order to tell if a 
> statement has an opportunity to impact on another statement, as a support for 
> a general database observation feature.
> 
> Here is the general process:
> 
> 1. During the compilation of a statement S1, authorizer callbacks tell which 
> tables and columns are read by the statement.
> 2. During the compilation of any other statement S2, authorizer callbacks 
> tell which tables and columns are written by the statement.
> 3. If the two sets of tables and columns do not intersect, then S2 can not 
> change the results of S1: exit.
> 4. Until callbacks registered by sqlite3_commit_hook or sqlite3_rollback_hook 
> [2] are invoked, authoriser callbacks allows the app to follow the savepoint 
> stack. This is important for the next step:
> 5. During the execution of S2, sqlite3_update_hook [1] tell if S2 actually 
> performs any change. Those changes are remembered until the transaction 
> commits [2], or the eventual current savepoint is rollbacked (see step 4 
> above).
> 6. After the transaction has been committed, if S2 has performed changes, 
> then S1 is reputed "dirty", and the application is notified that S1 results 
> may have changed.

Let me please rewrite the above process, to make it more clear.

Goal: notify that the last commit may have changed the results of statement S1, 
without any false negative, and a number of false positives that is as low as 
possible.

Let's use `SELECT col1 FROM t1` as S1.


# LEVEL 1: notify after each commit with sqlite3_commit_hook.

Need for improvement: there are many many false positives. Even an empty 
transaction `BEGIN; COMMIT;` triggers a notification.


# LEVEL 2: notify only for statements that write in the columns and tables read 
by S1 (thanks to the authorizer callbacks)

Good: `INSERT INTO t2 ...` and `UPDATE t1 SET col2 = 'foo'` no longer trigger a 
change notification, because authorizer callbacks prove that they have no 
impact of S1.

Need for improvement 1: if S1 uses the COUNT function, then all statements may 
have an impact on it, and we're back to level 1 above (read my previous emails 
about his trouble that SQLITE_READ_TABLE aims at solving)

Need for improvement 2: a statement that may modify S1 may not modify the 
database: for example `DELETE FROM t1 WHERE 0` won't delete a single line, but 
the change notification will be triggered anyway.


# LEVEL 3: notify only for statements that perform database modifications 
(thanks to the database change callbacks)

Good: `DELETE FROM t1 WHERE 0` no longer triggers a change notification, 
because it did not update, delete, or insert a single row.

Need for improvement: a database change callback may be invalidated by a 
rollbacked savepoint.


# LEVEL 4: filter out database changes that are rollbacked by a savepoint 
(thanks to the authorizer callbacks that allow the sqlite3 client to track the 
savepoint stack)

Good: `INSERT INTO t1 ...` no longer triggers a change notification if it is 
rollbacked.


# LEVEL 5: there is no level 5, I think we've gone as far as SQLite can do, and 
this is already quite tremendous.


Again, the problem is with the COUNT function. By I should stop repeating 
myself :-)

Thanks for reading,
Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-09 Thread Gwendal Roué 

> Le 9 mai 2017 à 00:21, Simon Slavin  a écrit :
> 
> Hardly anyone uses the authentication system, so far fewer people know the 
> answers.

As a reminder, I intend to use the authorisation system in order to tell if a 
statement has an opportunity to impact on another statement, as a support for a 
general database observation feature.

Here is the general process:

1. During the compilation of a statement S1, authorizer callbacks tell which 
tables and columns are read by the statement.
2. During the compilation of any other statement S2, authorizer callbacks tell 
which tables and columns are written by the statement.
3. If the two sets of tables and columns do not intersect, then S2 can not 
change the results of S1: exit.
4. Until callbacks registered by sqlite3_commit_hook or sqlite3_rollback_hook 
[2] are invoked, authoriser callbacks allows the app to follow the savepoint 
stack. This is important for the next step:
5. During the execution of S2, sqlite3_update_hook [1] tell if S2 actually 
performs any change. Those changes are remembered until the transaction commits 
[2], or the eventual current savepoint is rollbacked (see step 4 above).
6. After the transaction has been committed, if S2 has performed changes, then 
S1 is reputed "dirty", and the application is notified that S1 results may have 
changed.

The process above is able to provide false positives: for example `UPDATE TABLE 
t1 SET a = a` will trigger a notification, even though no change did occur.

What is important is that the process above doesn't miss any potential change.

Because of the current authorizer callbacks for queries like `SELECT COUNT(*) 
FROM t1`, which do not tell anything about t1, the step 3 above has to assume 
that *any* statement has the opportunity to modify the results of this select. 
This yields too many false positives.

My suggestion is there to allow a less paranoid observation of statements that 
use the COUNT function.

It is important to stress that I perfectly know that all those authorizer, 
update, commit, rollback hooks *can not* help observing a database as soon as 
there are several writer connections. But they *can* help as soon as there is a 
single writer connection.

That's exactly why I'm here: the above algorithm is already used by GRDB.swift 
[3], a database library focused on application development that puts all bets 
on SQLite. GRDB has a serious and robust concurrency model [4] which supports a 
single connection to a regular database, or a pool of several connections on a 
WAL database. Both use a single writer connection. Both would be improved with 
my suggestion.

Of course, the core team may prefer not implementing my suggested 
SQLITE_READ_TABLE. But I wish they would react to the above scenario.

Gwendal Roué

[1] https://www.sqlite.org/c3ref/update_hook.html 

[2] https://sqlite.org/c3ref/commit_hook.html 

[3] http://github.com/groue/GRDB.swift 
[4] https://github.com/groue/GRDB.swift/blob/master/README.md#concurrency 



___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-08 Thread Gwendal Roué 

> Le 9 mai 2017 à 00:21, Simon Slavin  a écrit :
> 
> 
> On 8 May 2017, at 10:11pm, petern  wrote:
> 
>> Who is the author of the Authorizer Action Code source?
> 
> Although SQLite is in the public domain, development of it is not typical for 
> an open source project.  Almost everything you download when you download 
> SQLite was written by a development team of three or four people.  
> Contributions from outside that group are rarely (? ever ?) incorporated into 
> the project as source code supplied.  Instead the development group takes 
> suggestions submitted on this list and sometimes decides to write code to 
> implement them.

Thanks for the information: I didn't know that.

> Instead, SQLite provides many hooks and callback opportunities, and people 
> are encouraged to write their own extensions and host them themselves.

Forking SQLite is a very hard path, unfortunately :-)

Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-08 Thread Gwendal Roué 

> Le 8 mai 2017 à 23:11, petern  a écrit :
> 
> Gwendal.  I understand all that.  It's also good that you've confirmed how
> SQLITE_READ is actually queried by the authorizer callback interface.  I
> was wondering about that.  Reading your earlier post, one might get the
> impression that the SQLITE_READ authorizer action was not queried by the
> engine for aggregate table reads for some reason.  Presumably that would be
> a bug.
> 
> My question about your solution is illustrated by looking at the existing
> defines for orthogonal operations.  Consider how SELECT, INSERT, and UPDATE
> are currently defined as below.
> 
> #define SQLITE_INSERT   18   /* Table Name  NULL
> */
> #define SQLITE_SELECT   21   /* NULLNULL
> */
> #define SQLITE_UPDATE   23   /* Table Name  Column Name
> */
> 
> If this interface is logically missing SQLITE_READ_TABLE then shouldn't all
> the orthogonal authorizer action codes in that same dimension also be
> implemented?  Thus, why not also add authorizer action codes for
> SQLITE_WRITE_TABLE, SQLITE_READ_COLUMN, SQLITE_WRITE_COLUMN,
> SQLITE_READ_SCHEMA, and SQLITE_WRITE_SCHEMA?  Why only just
> SQLITE_READ_TABLE?   If SQLITE_READ_TABLE is missing why aren't the others
> also missing?

Because they are not missing: existing authorizer callbacks already provide a 
detailed information for all possible updates. We just miss information about 
selected tables.

> Why is this forum so silent on this question?  Usually there are half a
> dozen responses on the "correct way" to do it.  This time, crickets.

I did propose a patch as a way to show that my proposal doesn't come from thin 
air, but can be implemented.

Yes, I wish the core team would give at least an acknowledgement that something 
could be improved.

Gwendal Roué

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-08 Thread Simon Slavin 

On 8 May 2017, at 10:11pm, petern  wrote:

> Who is the author of the Authorizer Action Code source?

Although SQLite is in the public domain, development of it is not typical for 
an open source project.  Almost everything you download when you download 
SQLite was written by a development team of three or four people.  
Contributions from outside that group are rarely (? ever ?) incorporated into 
the project as source code supplied.  Instead the development group takes 
suggestions submitted on this list and sometimes decides to write code to 
implement them.

Instead, SQLite provides many hooks and callback opportunities, and people are 
encouraged to write their own extensions and host them themselves.

> Why is this forum so silent on this question?  Usually there are half a
> dozen responses on the "correct way" to do it.  This time, crickets.

Hardly anyone uses the authentication system, so far fewer people know the 
answers.

Simon.
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-08 Thread petern 
Gwendal.  I understand all that.  It's also good that you've confirmed how
SQLITE_READ is actually queried by the authorizer callback interface.  I
was wondering about that.  Reading your earlier post, one might get the
impression that the SQLITE_READ authorizer action was not queried by the
engine for aggregate table reads for some reason.  Presumably that would be
a bug.

My question about your solution is illustrated by looking at the existing
defines for orthogonal operations.  Consider how SELECT, INSERT, and UPDATE
are currently defined as below.

#define SQLITE_INSERT   18   /* Table Name  NULL
*/
#define SQLITE_SELECT   21   /* NULLNULL
*/
#define SQLITE_UPDATE   23   /* Table Name  Column Name
*/

If this interface is logically missing SQLITE_READ_TABLE then shouldn't all
the orthogonal authorizer action codes in that same dimension also be
implemented?  Thus, why not also add authorizer action codes for
SQLITE_WRITE_TABLE, SQLITE_READ_COLUMN, SQLITE_WRITE_COLUMN,
SQLITE_READ_SCHEMA, and SQLITE_WRITE_SCHEMA?  Why only just
SQLITE_READ_TABLE?   If SQLITE_READ_TABLE is missing why aren't the others
also missing?

Who is the author of the Authorizer Action Code source?  Does anyone know?
Does the author have an opinion?  Are these new SQLITE_READ_X and
SQLITE_WRITE_X authorizer codes truly missing from the intended design? If
so, are they on the development road map?  Or, was the presumption that
practical applications would handle access control by denying everything
except operations within an application defined view and trigger layer?
[See idea about the 6th parameter of the callback, view or trigger name, I
mentioned in the previous post.]

Why is this forum so silent on this question?  Usually there are half a
dozen responses on the "correct way" to do it.  This time, crickets.

On Mon, May 8, 2017 at 8:04 AM, Gwendal Roué  wrote:

> Hello Peter,
>
> It's the generally the responsability of the callback implementor to test
> or not each authorization, depending on her needs. See
> https://sqlite.org/c3ref/set_authorizer.html
>
> -- Allow user to run select statements, and read col1 of t1:
> -- SQLITE_SELECT
> -- SQLITE_READ t1 col1 main
> SELECT col1 FROM t1;
>
> -- Allow user to run select statements, read col1 of t1, and insert in
> t2:
> -- SQLITE_INSERT t2 main
> -- SQLITE_SELECT
> -- SQLITE_READ t1 col1 main
> INSERT INTO t2 SELECT col1 FROM t1;
>
> There are also authorization callbacks for functions:
>
> -- Allow user to run select statements, read col1 of t1, execute count
> function:
> -- SQLITE_SELECT
> -- SQLITE_FUNCTION max
> -- SQLITE_READ t1 col1 main
> SELECT MAX(col1) FROM t1
>
> But here is why I'm suggesting a new code SQLITE_READ_TABLE:
>
> -- Allow user to run select statements, and execute count function:
> -- SQLITE_SELECT
> -- SQLITE_FUNCTION count
> SELECT COUNT(*) FROM t1
>
> In the previous query, no one knows that the table t1 is about to be used.
>
> The authorizer callback can not be extended so that it tells everything
> about a function arguments. That's because a function arguments can be too
> complex to fit in the callback arguments:
>
> -- SQLITE_SELECT
> -- SQLITE_FUNCTION count
> SELECT COUNT(*) FROM t1, t2, t3, t4, t5
>
> -- SQLITE_SELECT
> -- SQLITE_FUNCTION count
> -- SQLITE_READ t1 col1 main
> -- SQLITE_READ t2 col1 main
> -- SQLITE_READ t3 col1 main
> SELECT COUNT(DISTINCT t1.col1 + t2.col1 + t3.col1) FROM t1, t2, t3
>
> With the newly introduced SQLITE_READ_TABLE code, we have instead:
>
> -- SQLITE_SELECT
> -- SQLITE_READ t1 main
> -- SQLITE_FUNCTION count
> SELECT COUNT(*) FROM t1
>
> And now the client knows that the table t1 is used, and can forbid this
> access.
>
> Gwendal Roué
>
> > Gwendal.  Your proposal last month for adding column names to the
> callback parameters seemed more sensible.
> >
> > The first question that comes to mind when new callback modes are to
> being proposed is what else would be missing if the same standard were
> applied to every possible operation?
> >
> > My thought.  A cursory read of the relevant code comments (see below)
> suggests the author had in mind only precise security control of views and
> triggers - the ubiquitous 6th parameter mentioned in the comment.  If
> that's the idea, then one presumably denies everything by default and then
> handles requests only to a purpose built secure view and trigger layer.
> >
> > It would be nice to hear from the author about what they actually had in
> mind for those who need total iron clad security of every row or aggregate
> query of any table.  For example, if SQLITE_READ authorization is not being
> tested, why not?  Is it tested later?  Perhaps the architecture of the
> authorizer is not self explanatory from the names of the #defines or is
> described elsewhere.

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-08 Thread Gwendal Roué 
Hello Peter,

It's the generally the responsability of the callback implementor to test or 
not each authorization, depending on her needs. See 
https://sqlite.org/c3ref/set_authorizer.html

-- Allow user to run select statements, and read col1 of t1:
-- SQLITE_SELECT
-- SQLITE_READ t1 col1 main
SELECT col1 FROM t1;

-- Allow user to run select statements, read col1 of t1, and insert in t2:
-- SQLITE_INSERT t2 main
-- SQLITE_SELECT
-- SQLITE_READ t1 col1 main
INSERT INTO t2 SELECT col1 FROM t1;

There are also authorization callbacks for functions:

-- Allow user to run select statements, read col1 of t1, execute count 
function:
-- SQLITE_SELECT
-- SQLITE_FUNCTION max
-- SQLITE_READ t1 col1 main
SELECT MAX(col1) FROM t1

But here is why I'm suggesting a new code SQLITE_READ_TABLE:

-- Allow user to run select statements, and execute count function:
-- SQLITE_SELECT
-- SQLITE_FUNCTION count
SELECT COUNT(*) FROM t1

In the previous query, no one knows that the table t1 is about to be used.

The authorizer callback can not be extended so that it tells everything about a 
function arguments. That's because a function arguments can be too complex to 
fit in the callback arguments:

-- SQLITE_SELECT
-- SQLITE_FUNCTION count
SELECT COUNT(*) FROM t1, t2, t3, t4, t5

-- SQLITE_SELECT
-- SQLITE_FUNCTION count
-- SQLITE_READ t1 col1 main
-- SQLITE_READ t2 col1 main
-- SQLITE_READ t3 col1 main
SELECT COUNT(DISTINCT t1.col1 + t2.col1 + t3.col1) FROM t1, t2, t3

With the newly introduced SQLITE_READ_TABLE code, we have instead:

-- SQLITE_SELECT
-- SQLITE_READ t1 main
-- SQLITE_FUNCTION count
SELECT COUNT(*) FROM t1

And now the client knows that the table t1 is used, and can forbid this access.

Gwendal Roué

> Gwendal.  Your proposal last month for adding column names to the callback 
> parameters seemed more sensible.
> 
> The first question that comes to mind when new callback modes are to being 
> proposed is what else would be missing if the same standard were applied to 
> every possible operation?
> 
> My thought.  A cursory read of the relevant code comments (see below) 
> suggests the author had in mind only precise security control of views and 
> triggers - the ubiquitous 6th parameter mentioned in the comment.  If that's 
> the idea, then one presumably denies everything by default and then handles 
> requests only to a purpose built secure view and trigger layer.
> 
> It would be nice to hear from the author about what they actually had in mind 
> for those who need total iron clad security of every row or aggregate query 
> of any table.  For example, if SQLITE_READ authorization is not being tested, 
> why not?  Is it tested later?  Perhaps the architecture of the authorizer is 
> not self explanatory from the names of the #defines or is described elsewhere.
> 
> 
___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-06 Thread petern 
Gwendal.  Your proposal last month for adding column names to the callback
parameters seemed more sensible.  The first question that comes to mind
when new callback modes are to being proposed is what else would be missing
if the same standard were applied to every possible operation?

My thought.  A cursory read of the relevant code comments (see below)
suggests the author had in mind only precise security control of views and
triggers - the ubiquitous 6th parameter mentioned in the comment.  If
that's the idea, then one presumably denies everything by default and then
handles requests only to a purpose built secure view and trigger layer.

It would be nice to hear from the author about what they actually had in
mind for those who need total iron clad security of every row or aggregate
query of any table.  For example, if SQLITE_READ authorization is not being
tested, why not?  Is it tested later?  Perhaps the architecture of the
authorizer is not self explanatory from the names of the #defines or is
described elsewhere.

/*
** CAPI3REF: Authorizer Action Codes
**
** The [sqlite3_set_authorizer()] interface registers a callback function
** that is invoked to authorize certain SQL statement actions.  The
** second parameter to the callback is an integer code that specifies
** what action is being authorized.  These are the integer action codes that
** the authorizer callback may be passed.
**
** These action code values signify what kind of operation is to be
** authorized.  The 3rd and 4th parameters to the authorization
** callback function will be parameters or NULL depending on which of these
** codes is used as the second parameter.  ^(The 5th parameter to the
** authorizer callback is the name of the database ("main", "temp",
** etc.) if applicable.)^  ^The 6th parameter to the authorizer callback
** is the name of the inner-most trigger or view that is responsible for
** the access attempt or NULL if this access attempt is directly from
** top-level SQL code.
*/
/*** 3rd  4th
***/
#define SQLITE_CREATE_INDEX  1   /* Index Name  Table Name
*/
#define SQLITE_CREATE_TABLE  2   /* Table Name  NULL
*/
#define SQLITE_CREATE_TEMP_INDEX 3   /* Index Name  Table Name
*/
#define SQLITE_CREATE_TEMP_TABLE 4   /* Table Name  NULL
*/
#define SQLITE_CREATE_TEMP_TRIGGER   5   /* Trigger NameTable Name
*/
#define SQLITE_CREATE_TEMP_VIEW  6   /* View Name   NULL
*/
#define SQLITE_CREATE_TRIGGER7   /* Trigger NameTable Name
*/
#define SQLITE_CREATE_VIEW   8   /* View Name   NULL
*/
#define SQLITE_DELETE9   /* Table Name  NULL
*/
#define SQLITE_DROP_INDEX   10   /* Index Name  Table Name
*/
#define SQLITE_DROP_TABLE   11   /* Table Name  NULL
*/
#define SQLITE_DROP_TEMP_INDEX  12   /* Index Name  Table Name
*/
#define SQLITE_DROP_TEMP_TABLE  13   /* Table Name  NULL
*/
#define SQLITE_DROP_TEMP_TRIGGER14   /* Trigger NameTable Name
*/
#define SQLITE_DROP_TEMP_VIEW   15   /* View Name   NULL
*/
#define SQLITE_DROP_TRIGGER 16   /* Trigger NameTable Name
*/
#define SQLITE_DROP_VIEW17   /* View Name   NULL
*/
#define SQLITE_INSERT   18   /* Table Name  NULL
*/
#define SQLITE_PRAGMA   19   /* Pragma Name 1st arg or NULL
*/
#define SQLITE_READ 20   /* Table Name  Column Name
*/
#define SQLITE_SELECT   21   /* NULLNULL
*/
#define SQLITE_TRANSACTION  22   /* Operation   NULL
*/
#define SQLITE_UPDATE   23   /* Table Name  Column Name
*/
#define SQLITE_ATTACH   24   /* FilenameNULL
*/
#define SQLITE_DETACH   25   /* Database Name   NULL
*/
#define SQLITE_ALTER_TABLE  26   /* Database Name   Table Name
*/
#define SQLITE_REINDEX  27   /* Index Name  NULL
*/
#define SQLITE_ANALYZE  28   /* Table Name  NULL
*/
#define SQLITE_CREATE_VTABLE29   /* Table Name  Module Name
*/
#define SQLITE_DROP_VTABLE  30   /* Table Name  Module Name
*/
#define SQLITE_FUNCTION 31   /* NULLFunction Name
*/
#define SQLITE_SAVEPOINT32   /* Operation   Savepoint Name
*/
#define SQLITE_COPY  0   /* No longer used */
#define SQLITE_RECURSIVE33   /* NULLNULL
*/


On Sat, May 6, 2017 at 6:24 AM, Gwendal Roué  wrote:

>
> > Le 6 mai 2017 à 15:12, Gwendal Roué  a écrit :
> >
> > Hello,
> >
> > This email contains a patch that introduces a new authorizer action
> code: SQLITE_READ_TABLE.
>
> My patch did not work when the authorizer callback would not return
> SQLITE_OK.
>
> Please find the fixed patch below:
>
> $ fossil info
> project-name: SQLite
> repository:

Re: [sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-06 Thread Gwendal Roué 

> Le 6 mai 2017 à 15:12, Gwendal Roué  a écrit :
> 
> Hello,
> 
> This email contains a patch that introduces a new authorizer action code: 
> SQLITE_READ_TABLE.

My patch did not work when the authorizer callback would not return SQLITE_OK.

Please find the fixed patch below:

$ fossil info
project-name: SQLite
repository:   /Users/groue/Documents/git/sqlite/sqlite.fossil
local-root:   /Users/groue/Documents/git/sqlite/
config-db:/Users/groue/.fossil
project-code: 2ab58778c2967968b94284e989e43dc11791f548
checkout: b9a58daca80a815e87e541cb5fff9bc8b93f131d 2017-05-04 11:13:50 UTC
parent:   e24b73820cdca07eee87853fe6dd9f60d76e039e 2017-05-03 19:36:50 UTC
tags: trunk
comment:  Fix a collision of the "B0" identifier name between the termios.h 
header file and the SHA3 implementation in the shell. (user: drh)
check-ins:18701

$ fossil diff
Index: src/select.c
==
--- src/select.c
+++ src/select.c
@@ -4370,10 +4370,15 @@
 }else{
   /* An ordinary table or view name in the FROM clause */
   assert( pFrom->pTab==0 );
   pFrom->pTab = pTab = sqlite3LocateTableItem(pParse, 0, pFrom);
   if( pTab==0 ) return WRC_Abort;
+  int iDb = sqlite3SchemaToIndex(db, pTab->pSchema);
+  if( sqlite3AuthCheck(pParse, SQLITE_READ_TABLE, pTab->zName, 0, 
db->aDb[iDb].zDbSName) ){
+pFrom->pTab = 0;
+return WRC_Abort;
+  }
   if( pTab->nTabRef>=0x ){
 sqlite3ErrorMsg(pParse, "too many references to \"%s\": max 65535",
pTab->zName);
 pFrom->pTab = 0;
 return WRC_Abort;

Index: src/sqlite.h.in
==
--- src/sqlite.h.in
+++ src/sqlite.h.in
@@ -2824,10 +2824,11 @@
 #define SQLITE_DROP_VTABLE  30   /* Table Name  Module Name */
 #define SQLITE_FUNCTION 31   /* NULLFunction Name   */
 #define SQLITE_SAVEPOINT32   /* Operation   Savepoint Name  */
 #define SQLITE_COPY  0   /* No longer used */
 #define SQLITE_RECURSIVE33   /* NULLNULL*/
+#define SQLITE_READ_TABLE   34   /* Table Name  NULL*/
 
 /*
 ** CAPI3REF: Tracing And Profiling Functions
 ** METHOD: sqlite3
 **

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users

[sqlite] Proposition: introduce a new SQLITE_READ_TABLE Authorizer Action Code

2017-05-06 Thread Gwendal Roué 
Hello,

This email contains a patch that introduces a new authorizer action code: 
SQLITE_READ_TABLE.

The goal of this new action code is to fill a hole in the current authorization 
API, which does not tell about all tables read by a statement. For example, the 
statement "SELECT COUNT(*) FROM table1" currently invokes the callback twice: 
SQLITE_SELECT, SQLITE_FUNCTION. Nothing is said about table1.

With the provided patch, we add a third invocation of the callback, with the 
new code SQLITE_READ_TABLE. Its 3d parameter is "table1", and the schema name 
is the 5th parameter.

Following the current practice which calls sqlite3AuthCheck() during the 
parsing phase, I have added a call to sqlite3AuthCheck() in the 
selectExpander() function, right after the call to sqlite3LocateTableItem().

Basically, for each table used by the select statement, either it is not found 
by sqlite3LocateTableItem(), either it has to be authorized by the 
authorization callback.

I'm not familiar with the way code and feature requests are handled within the 
SQLite community. If you are interested about this patch, let me know how I can 
help!

Gwendal Roué


$ fossil info
project-name: SQLite
repository:   /Users/groue/Documents/git/sqlite/sqlite.fossil
local-root:   /Users/groue/Documents/git/sqlite/
config-db:/Users/groue/.fossil
project-code: 2ab58778c2967968b94284e989e43dc11791f548
checkout: b9a58daca80a815e87e541cb5fff9bc8b93f131d 2017-05-04 11:13:50 UTC
parent:   e24b73820cdca07eee87853fe6dd9f60d76e039e 2017-05-03 19:36:50 UTC
tags: trunk
comment:  Fix a collision of the "B0" identifier name between the termios.h 
header file and the SHA3 implementation in the shell. (user: drh)
check-ins:18701


$ fossil diff
Index: src/select.c
==
--- src/select.c
+++ src/select.c
@@ -10,10 +10,11 @@
 **
 *
 ** This file contains C code routines that are called by the parser
 ** to handle SELECT statements in SQLite.
 */
+#include 
 #include "sqliteInt.h"
 
 /*
 ** Trace output macros
 */
@@ -4370,10 +4371,14 @@
 }else{
   /* An ordinary table or view name in the FROM clause */
   assert( pFrom->pTab==0 );
   pFrom->pTab = pTab = sqlite3LocateTableItem(pParse, 0, pFrom);
   if( pTab==0 ) return WRC_Abort;
+  int iDb = sqlite3SchemaToIndex(db, pTab->pSchema);
+  if( sqlite3AuthCheck(pParse, SQLITE_READ_TABLE, pTab->zName, 0, 
db->aDb[iDb].zDbSName) ){
+return WRC_Abort;
+  }
   if( pTab->nTabRef>=0x ){
 sqlite3ErrorMsg(pParse, "too many references to \"%s\": max 65535",
pTab->zName);
 pFrom->pTab = 0;
 return WRC_Abort;

Index: src/sqlite.h.in
==
--- src/sqlite.h.in
+++ src/sqlite.h.in
@@ -2824,10 +2824,11 @@
 #define SQLITE_DROP_VTABLE  30   /* Table Name  Module Name */
 #define SQLITE_FUNCTION 31   /* NULLFunction Name   */
 #define SQLITE_SAVEPOINT32   /* Operation   Savepoint Name  */
 #define SQLITE_COPY  0   /* No longer used */
 #define SQLITE_RECURSIVE33   /* NULLNULL*/
+#define SQLITE_READ_TABLE   34   /* Table Name  NULL*/
 
 /*
 ** CAPI3REF: Tracing And Profiling Functions
 ** METHOD: sqlite3
 **

___
sqlite-users mailing list
sqlite-users@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/sqlite-users