Skip to content

Commit

Permalink
Fixed CORE-6208: Grant lost in security.db after backup/restore cycle
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlexPeshkoff
AlexPeshkoff committed Dec 27, 2019
1 parent a46a3df commit 808688a
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 3 deletions.
63 changes: 63 additions & 0 deletions src/burp/backup.epp
View file
Expand Up @@ -133,6 +133,7 @@ void write_global_fields();
void write_generators();
void write_sql_roles();
void write_mapping();
void write_db_creators();
void write_packages();
void write_procedures();
void write_procedure_prms(const GDS_NAME, const GDS_NAME);
Expand Down Expand Up @@ -400,6 +401,11 @@ int BACKUP_backup(const TEXT* dbb_file, const TEXT* file_name)
BURP_verbose(296);
// msg 296 writing mapping
write_mapping();

// Write database creators
BURP_verbose(391);
// msg 391 writing database creators
write_db_creators();
}

This comment has been minimized.

Sign in to view

Copy link
@dyemanov

dyemanov Apr 2, 2020

Member

Why are both these functions inside the DB_VERSION_DDL11 block? These tables exist in ODS12 only. Currently it's harmless due to the correct ODS check inside the functions, but I'd rather prefer to fix the IF condition here and then remove the duplicated ODS checks inside the functions (like it's done for write_packages(), for example).

// Finish up
Expand Down Expand Up @@ -4110,6 +4116,63 @@ void write_mapping()
}


void write_db_creators()
{
/**************************************
*
* w r i t e _ d b _ c r e a t o r s
*
**************************************
*
* Functional description
* write a record in the burp file for
* each grant to create database.
*
**************************************/
Firebird::IRequest* req_handle = nullptr;
TEXT temp[GDS_NAME_LEN];

BurpGlobals* tdgbl = BurpGlobals::getSpecific();

if (tdgbl->runtimeODS >= DB_VERSION_DDL12)
{
FOR (REQUEST_HANDLE req_handle)
C IN RDB$DB_CREATORS

bool fl = true;

if (!C.RDB$USER_TYPE.NULL)
{
put(tdgbl, rec_db_creator);
fl = false;
put_int32(att_dbc_type, C.RDB$USER_TYPE);
}

if (!C.RDB$USER.NULL)
{
if (fl)
put(tdgbl, rec_db_creator);
fl = false;
const SSHORT l = PUT_TEXT(att_dbc_user, C.RDB$USER);

MISC_terminate (C.RDB$USER, temp, l, sizeof(temp));
BURP_verbose (392, temp);
// msg 392 writing db creator %s
}

if (!fl)
put(tdgbl, att_end);

END_FOR;
ON_ERROR
general_on_error();
END_ERROR;
}

MISC_release_request_silent(req_handle);
}


void write_triggers()
{
/**************************************
Expand Down
11 changes: 9 additions & 2 deletions src/burp/burp.h
View file
Expand Up @@ -114,7 +114,8 @@ enum rec_type {
rec_collation, // Collations
rec_sql_roles, // SQL roles
rec_mapping, // Mapping of security names
rec_package // Package
rec_package, // Package
rec_db_creator // Database creator
};


Expand Down Expand Up @@ -625,7 +626,11 @@ enum att_type {
att_package_security_class,
att_package_owner_name,
att_package_description,
att_package_sql_security
att_package_sql_security,

// Database creators
att_dbc_user = SERIES,
att_dbc_type
};


Expand Down Expand Up @@ -1098,6 +1103,7 @@ class BurpGlobals : public Firebird::ThreadData, public GblPool
Firebird::IRequest* handles_get_security_class_req_handle1;
Firebird::IRequest* handles_get_sql_roles_req_handle1;
Firebird::IRequest* handles_get_mapping_req_handle1;
Firebird::IRequest* handles_db_creators_req_handle1;
Firebird::IRequest* handles_get_trigger_message_req_handle1;
Firebird::IRequest* handles_get_trigger_message_req_handle2;
Firebird::IRequest* handles_get_trigger_old_req_handle1;
Expand Down Expand Up @@ -1154,6 +1160,7 @@ class BurpGlobals : public Firebird::ThreadData, public GblPool
ULONG verboseInterval; // How many records should be backed up or restored before we show this message
bool flag_on_line; // indicates whether we will bring the database on-line
bool firstMap; // this is the first time we entered get_mapping()
bool firstDbc; // this is the first time we entered get_db_creators()
bool stdIoMode; // stdin or stdout is used as backup file
Firebird::AutoPtr<Firebird::SimilarToRegex> skipDataMatcher;
Firebird::AutoPtr<Firebird::SimilarToRegex> includeDataMatcher;
Expand Down
71 changes: 71 additions & 0 deletions src/burp/restore.epp
View file
Expand Up @@ -147,6 +147,7 @@ bool get_relation(BurpGlobals* tdgbl);
bool get_relation_data(BurpGlobals* tdgbl);
bool get_sql_roles(BurpGlobals* tdgbl);
bool get_mapping(BurpGlobals* tdgbl);
bool get_db_creator(BurpGlobals* tdgbl);
bool get_security_class(BurpGlobals* tdgbl);
void get_source_blob(BurpGlobals* tdgbl, ISC_QUAD&, bool);
USHORT get_text(BurpGlobals* tdgbl, TEXT*, ULONG);
Expand Down Expand Up @@ -8905,6 +8906,70 @@ bool get_mapping(BurpGlobals* tdgbl)
return true;
}

bool get_db_creator(BurpGlobals* tdgbl)
{
/**************************************
*
* g e t _ d b _ c r e a t o r
*
**************************************
*
* Functional description
* Restore database creators
*
**************************************/
att_type attribute;
scan_attr_t scan_next_attr;
TEXT temp[GDS_NAME_LEN];
SSHORT l;
Firebird::string role;

if (tdgbl->runtimeODS >= DB_VERSION_DDL12)
{
STORE (REQUEST_HANDLE tdgbl->handles_db_creators_req_handle1)
C IN RDB$DB_CREATORS

C.RDB$USER.NULL = TRUE;
C.RDB$USER_TYPE.NULL = TRUE;

skip_init(&scan_next_attr);
while (skip_scan(&scan_next_attr), get_attribute(&attribute, tdgbl) != att_end)
{
switch (attribute)
{
case att_dbc_user:
C.RDB$USER.NULL = FALSE;
GET_TEXT(C.RDB$USER);
if (tdgbl->firstDbc)
{
tdgbl->firstDbc = false;
BURP_verbose(394);
// msg 394 restoring database creators
}
BURP_verbose (393, C.RDB$USER);
break;

case att_dbc_type:
C.RDB$USER_TYPE.NULL = FALSE;
C.RDB$USER_TYPE = (USHORT) get_int32(tdgbl);
break;

default:
// msg 395 database creator
bad_attribute(scan_next_attr, attribute, 395);
break;
}
}

END_STORE;
ON_ERROR
general_on_error ();
END_ERROR;
}

return true;
}

This comment has been minimized.

Sign in to view

Copy link
@dyemanov

dyemanov Apr 2, 2020

Member

I don't see the "skip" path here, for the case when the target ODS does not have RDB$DB_CREATORS.

This comment has been minimized.

Sign in to view

Copy link
@AlexPeshkoff

AlexPeshkoff via email Apr 2, 2020

Author Member

This comment has been minimized.

Sign in to view

Copy link
@dyemanov

dyemanov Apr 2, 2020

Member

At least we need a clear decision about that (I don't remember this being discussed). If not, a major cleanup in restore.epp is needed.

I observed this problem for RDB$PACKAGES and RDB$DB_CREATORS -- tables introduced in ODS12. RDB$AUTH_MAPPINGS has the skip path but it looks incomplete (no support for ODS < 11.1).

This comment has been minimized.

Sign in to view

Copy link
@dyemanov

dyemanov Apr 2, 2020

Member

And anyway, I doubt the legacy gbak behaviour (ability to restore into older ODS) should be changed in FB3.

This comment has been minimized.

Sign in to view

Copy link
@AlexPeshkoff

AlexPeshkoff via email Apr 17, 2020

Author Member
bool is_ascii_name (const TEXT *name, const SSHORT len)
{
/**************************************
Expand Down Expand Up @@ -10921,6 +10986,12 @@ bool restore(BurpGlobals* tdgbl, Firebird::IProvider* provider, const TEXT* file
flag = true;
break;

case rec_db_creator:
if (!get_db_creator(tdgbl))
return false;
flag = true;
break;

default:
BURP_error(43, true, SafeArg() << record);
// msg 43 don't recognize record type %ld
Expand Down
2 changes: 1 addition & 1 deletion src/msgs/facilities2.sql
View file
Expand Up @@ -9,7 +9,7 @@ set bulk_insert INSERT INTO FACILITIES (LAST_CHANGE, FACILITY, FAC_CODE, MAX_NUM
('2018-06-22 11:46:00', 'DYN', 8, 309)
('1996-11-07 13:39:40', 'INSTALL', 10, 1)
('1996-11-07 13:38:41', 'TEST', 11, 4)
('2018-04-26 20:40:00', 'GBAK', 12, 391)
('2019-12-27 20:10:00', 'GBAK', 12, 396)
('2019-04-13 21:10:00', 'SQLERR', 13, 1047)
('1996-11-07 13:38:42', 'SQLWARN', 14, 613)
('2018-02-27 14:50:31', 'JRD_BUGCHK', 15, 308)
Expand Down
5 changes: 5 additions & 0 deletions src/msgs/messages2.sql
View file
Expand Up @@ -2553,6 +2553,11 @@ ERROR: Backup incomplete', NULL, NULL);
(NULL, 'burp_usage', 'burp.c', NULL, 12, 388, NULL, ' @1INCLUDE(_DATA) backup data of table(s)', NULL, NULL);
(NULL, NULL, 'burp.cpp', NULL, 12, 389, NULL, 'missing regular expression to include tables', NULL, NULL);
(NULL, NULL, 'burp.cpp', NULL, 12, 390, NULL, 'regular expression to include tables was already set', NULL, NULL);
(NULL, 'BACKUP_backup', 'backup.epp', NULL, 12, 391, NULL, 'writing database create grants', NULL, NULL);
(NULL, 'write_db_creators', 'backup.epp', NULL, 12, 392, NULL, ' database create grant for @1', NULL, NULL);
(NULL, 'get_db_creators', 'restore.epp', NULL, 12, 393, NULL, ' restoring database create grant for @1', NULL, NULL);
(NULL, 'get_db_creators', 'restore.epp', NULL, 12, 394, NULL, 'restoring database create grants', NULL, NULL);
(NULL, 'get_db_creators', 'restore.epp', NULL, 12, 395, NULL, 'database create grant', NULL, NULL);
-- SQLERR
(NULL, NULL, NULL, NULL, 13, 1, NULL, 'Firebird error', NULL, NULL);
(NULL, NULL, NULL, NULL, 13, 74, NULL, 'Rollback not performed', NULL, NULL);
Expand Down

0 comments on commit 808688a

Please sign in to comment.