Clone
Cliff Gray <cliff.gray@hp.com>
committed
on 24 Sep 14
PrivMgr code review rework/fixes
This code has been reviewed and/or tested by some of the members of the
security team.

Overview:

1) Repl… Show more
PrivMgr code review rework/fixes

This code has been reviewed and/or tested by some of the members of the

security team.

Overview:

1) Replaced calls to sprintf with calls to std::to_string.  This avoids the

   problem of the buffer potentially being too small for the written data.

2) Eliminated use of STATUS_INTERNAL error return in PrivMgr code.  Usage was

   inconsistent and could lead to confusion and errors.  Internal errors are

   now reported as STATUS_ERROR.

3) Previously construction of internal errors was not supplying the filename.

   Two defines were created, one for PrivMgr (PRIVMGR_INTERNAL_ERROR) and one

   for SeabaseDDL (SEABASEDDL_INTERNAL_ERROR) to generate the error completely,

   using the provided string.

4) Code was aligned where previous changes had left it misaligned.

5) The command GET COMPONENT PRIVILEGES ON component FOR authID was

   implemented.  Also, the header for the related GET COMPONENT PRIVILEGES ON

   component was updated.

6) ALTER USER now supports remapping the DB__ROOT user.  Also, the command

   now correctly checks the REMAP_USER privilege.

7) Checks for ID/Name mapping were not always checking the return code.

   Checks were added and errors (sometimes internal) were added.

8) Buffer size for usernames was previously hardcoded, now

   MAX_DBUSERNAME_LEN is used.

9) DROP ROLE now checks for granted privileges prior to removing the system

   grant for the role.

10) GRANT/REVOKE ROLE now support the MANAGE_ROLES privilege for using the

   GRANTED BY clause.  Now a non-DB__ROOT user can grant or revoke a role on

   behalf of another user, but only if they have the MANAGE_ROLES privilege.

   This authority may be moved to a separate privilege in the future.

11) A member variable (myTable_) was not being freed in the destructor.

12) GRANT/REVOKE ROLE is now checking the return from insert, delete, and

   update operations.

Externals:

Previously error messages 1356 and 1357 were missing a parameter.

E.g.

*** ERROR 1356 *** Cannot create the component privilege specified.

Component privilege code for the component already exists.

is now:

*** ERROR 1356 *** Cannot create the component privilege specified.

Component privilege code CR for the component already exists.

Error 1357 reports an existing component operation name.

Internals:

/cli

Context.cpp

Only change is alignment changes from a previous review.

/executor

ExExeUtilGet.cpp

o Fixed the getComponentPrivilegesForUser query-missing a parenthesis.

o Updated the headers generated for GET COMPONENT PRIVILEGES ON component and

 GET COMPONENT PRIVILEGES ON component FOR authID.

o Within the work() function, corrected code for handling dual GET COMPONENT

 PRIVILEGES command based on presence/absence of param1 (authID).

/generator

GenRelExeUtil.cpp

Reverted component operations back to component privileges for use in dual

command.

/sqlcomp

CmpSeabaseDDL.h

Added SEABASEDDL_INTERNAL_ERROR define.

CmpSeabaseDDLauth.cpp

CmpSeabaseDDLauth.h

o verifyAuthority() in the base class was removed, and the version in the user

 class was enhanced to support verifying the authority to remap users.

o Internal errors are now generated using the SEABASEDDL_INTERNAL_ERROR define.

o CmpSeabaseDDLuser::alterUser now supports remap the external username for

 DB__ROOT.  Previously reserved names were prohibited by the ALTER USER

 command, now an exception is made for DB__ROOT if the operation is setting a

 new external name.

o CmpSeabaseDDLrole::dropRole now checks for privileges granted to a role

 before removing the system grant of the role.  Previously the system grant

 could have been removed leaving the role ungrantable.

CmpSeabaseDDLcommon.cpp

o Internal errors are now generated using the SEABASEDDL_INTERNAL_ERROR define.

o grantRevokeSeabaseRole() now uses the MANAGE_ROLES privilege to determine if

 a user can grant or revoke a role on behalf of another user.  Previously

 this was restricted to DB__ROOT.  This authority may be moved to a separate

 component privilege in the future.

o GRANTED BY clause now restricted to DB__ROOT for GRANT/REVOKE COMPONENT PRIVILEGE.

PrivMgrCommands.cpp

o Replaced usage of STATUS_INTERNAL with STATUS_ERROR.

o Removed unused grantRoleToCreator() - CREATE ROLE calls PrivMgrRoles class

 directly.

PrivMgrComponentOperations.cpp

o Member variable myTable_ is now deleted when the instance is freed.

o Corrected generation of error message for existing component name or code to

 use the correct parameter constructor type.

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o UIDToString (which calls std::to_string) used instead of sprintf to convert

 component UIDs to strings.

PrivMgrComponentPrivileges.cpp

o Member variable myTable_ is now deleted when the instance is freed.

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o UIDToString (which calls std::to_string) used instead of sprintf to convert

 component UIDs to strings.  Similarly, authIDToString is called to convert

 granteeIDs and grantorIDs to strings.

o grantPrivilege() no longer removes WITH GRANT OPTION when a privilege a user

 has WITH GRANT OPTION is re-granted to the user without WITH GRANT OPTION.

 Instead, the grant is now a nop.

PrivMgrComponents.cpp

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o UIDToString (which calls std::to_string) used instead of sprintf to convert

 component UIDs to strings.

PrivMgrDefs.h

Removed STATUS_INTERNAL from the list of PrivStatus enums.

PrivMgrMD.h

o Added define PRIVMGR_INTERNAL_ERROR.

o Added static inline functions authIDToString and UIDToString.  Respectively

 they take an int32_t and int64_t and return a std:string.  Internally they

 both cast the value to long long int as that is the only equivalent type

 supported by the current (4.4.6) gcc compiler.

PrivMgrMD.cpp

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o UIDToString (which calls std::to_string) used instead of sprintf to convert

 component UIDs to strings.

o A grant of select to the AUTHS table to PUBLIC was removed.  This was added

 as a workaround, but another workaround (allow SELECT on all metadata tables)

 superseded this one.

PrivMgrPrivileges.cpp

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o Checks for STATUS_INTERNAL were removed.

PrivMgrRoles.cpp

o Member variable myTable_ is now deleted when the instance is freed.

o Internal errors are now generated with PRIVMGR_INTERNAL_ERROR define.

o authToString is called to convert roleIDs, granteeIDs, and grantorIDs to strings.

o grantRole() no longer removes WITH ADMIN OPTION when a privilege a user has

 been granted a role WITH ADMIN OPTION and the role is re-granted to the user

 without WITH ADMIN OPTION.  Instead, the grant is now a nop.

o The error return from insert or update is checked following a grant as well

 as from delete and update following a revoke, and an internal error is

 reported if the operation fails.

Change-Id: I6d9fc31455222f28cdc5d0db65dc75fc8bb4a99e

Show less

default + 10 more