FirebirdSQL / firebird

Firebird server, client and tools
https://www.firebirdsql.org/
1.19k stars 204 forks source link

DDL-Changes in replication does not set the correct grantor #8058

Closed realcolaflasche closed 1 month ago

realcolaflasche commented 1 month ago

Hi,

I am using the async-Replication and in with DDL-Changes (chreate table), the grantor is not set correct. This ensures that replication stops Here is my way to reproduce it in FB 4.0.4 and 5.0. I'm using a Windows 11-machine, but also had this seen on a Linux-machine.

  1. Create USER ISQL isql employee -user SYSDBA create user DBOwner password '1234' Grant Admin role;
  2. Create Database
    
    CREATE DATABASE 'localhost/3050:primary.fdb'
    USER 'DBOWNER' PASSWORD '1234'
    PAGE_SIZE 16384
    DEFAULT CHARACTER SET UTF8 COLLATION UTF8;

RECREATE TABLE TEST1 ( ID INTEGER GENERATED BY DEFAULT AS IDENTITY, CONSTRAINT PK_TEST1 PRIMARY KEY (ID) );

3. Copy Database to secondary.fdb
4. Setup an async replication between primary.fdb and secondary.fdb
5. Activate replication on primary side
```sql
ALTER DATABASE INCLUDE ALL TO PUBLICATION
ALTER DATABASE ENABLE PUBLICATION
  1. Activate replication on secondary side
    gfix -user DBOwner -pass 1234 -replica read_only  localhost/3051:secondary.fdb
  2. Create an other Table on primary side (Connected as DBOwner)
    RECREATE TABLE TEST2 (
    ID  INTEGER GENERATED BY DEFAULT AS IDENTITY,
    CONSTRAINT PK_TEST2 PRIMARY KEY (ID)
    )
  3. Have a look, who is the Grantor
    select * from RDB$USER_PRIVILEGES where RDB$USER_PRIVILEGES.rdb$relation_name = 'TEST2'

In Primary: RDB$User = DBOWNER and RDB$GRANTOR = DBOWNER In Secondary: RDB$User = DBOWNER and RDB$GRANTOR = SYSDBA

  1. Revoke delete from DBOwner on Table TEST2 on primary side
    REVOKE delete on TEST2 from DBOWNER;
  2. Replication dosent works anymore
    replocation.log: DBOWNER is not grantor of DELETE on TEST2 to DBOWNER.

My expectation is that the Grantor on the secondary side is the same as on the primary side. Or that you can specify in the config file who executes the replication statments on the secondary side. I didn't find something in the Documentation to this.

Regards, Jan

dyemanov commented 1 month ago

This could be fixed in two ways:

  1. Execute replicated DDL as original user rather than as the real connected user. This user is not required to exist on the replica side but its privileges should exist and allow that operation.
  2. Fake CURRENT_USER to return att_ss_user rather than att_user if it's called from the applier code.

Can anyone think of any other solution?

aafemt commented 1 month ago

Cannot Applier just set both att_user and att_ss_user in executeSql()?

dyemanov commented 1 month ago

This is what I meant by (1) above. But I'm not sure whether it will be enough. What if some privileges weren't loaded and cached yet (for the original att_user) and it will be initiated during replication (for the temporary att_user)?

aafemt commented 1 month ago

Alternatively Applier can be allowed to perform DML on system tables and DDL being replicated as DML. And bypass security completely by setting of FLAG_POWERFUL.

hvlad commented 1 month ago

Alternatively Applier can be allowed to perform DML on system tables and DDL being replicated as DML.

AFAIU, it will bind layout of replica's system tables to the master's one, and thus disable replication of DDL statements between different Firebird versions.

aafemt commented 1 month ago

The same problem as with new/modified DDL statements. But at DML level it can be solved more easily by some kind of "smart" processing of system tables on source and target side while modification of plain SQL text to comply current version is more tricky.

PS: May be we should just solve actual error and allow grantee to revoke privileges from itself regardless of who they were granted by?..

dyemanov commented 1 month ago

The initial implementation was pure DML and it was proven to be fragile in practice, hence I switched to statement level replication for DDL.

dyemanov commented 1 month ago

Currently access checks are disabled for the applier, so maybe it's really safe to just substitute att_user. I will double check.