tomoakin / RPostgreSQL

Automatically exported from code.google.com/p/rpostgresql
64 stars 19 forks source link

Create Roles and users from RPostgreSQL #118

Closed marbotte closed 2 years ago

marbotte commented 2 years ago

I am currently making a package for helping ecologist to manage database with vegetation data. When I try to create role and users for a database using a connection with the postgres user, Note that I changed the pg_hba.conf for authorizing locally a connection to postgres with the postgres user. I am using a debian installation

con_su<- dbConnect(drv="PostgreSQL",db="postgres",user="postgres") dbSendQuery(con_su,"CREATE ROLE 'db_user'")

I get some errors which look like intricated in the libpq library (but that is way more complicated that what I really understand) I've tried to pass through posgresqlExecStatement, without any difference in the errors. If that is not possible to pass through an SQL statement, would it be possible to create functions dbCreateRole and dbCreateUser?

marbotte commented 2 years ago

It seems that the error only happens when I use the 'postgres' user (with the 'trust' option for connecting locally in pg_hba.conf...) If I create another super-user, everything work fine with dbSendStatement

marbotte commented 2 years ago

Sorry, the errors appear to be because of bad quoting from my part... I am now unable to reproduce with well quoted statements. Still, since the quoting is complicated to manage with the functions protecting from SQL injection in those cases, it would be interesting to have functions which could help creating Roles... I do not know if there is something I did not get right, but there are no function (type dbQuoteIdentifier, dbQuoteLiteral etc.) which protect against injection attacks for unquoted parameters, right? I am thinking about parameters from a R function like superUser, createDb, createRole which would be booleans, and would pass "SUPERUSER"/"NOSUPERUSER", "CREATEDB"/"NOCREATEDB" to the Sql CREATE ROLE statement. For now, I have used grepl("--",statement) and grepl(";",statement) to avoid SQL injection, but I am not sure it is safe.

create_sib_role<- function(adm_con, rolName="sib_user", passwd=NA, superUser= F, login=!is.na(passwd), createDb= F, createRole= F, inGroup="sib") { existingRoles<- dbGetQuery(adm_con,"SELECT rolname FROM pg_catalog.pg_roles")$rolname if(rolName %in% existingRoles) { warning(rolName, "already exists as a role in the cluster, the statement is bloqued before going to postgreSQL. If what you want is to modify an existing role, please go directly to the postgres server with administrative rights.") return(NULL) } if(length(inGroup)>1) {stop("Attributing more than one group is not supported for now")} noGroup<- is.na(inGroup) log<- login
superUser<- SQL(ifelse(superUser,"SUPERUSER","NOSUPERUSER")) createDb<- SQL(ifelse(createDb,"CREATEDB","NOCREATEDB")) createRole<- SQL(ifelse(createRole, "CREATEROLE", "NOCREATEROLE")) login<- SQL(ifelse(login, "LOGIN", "NOLOGIN")) if(log){ passwd<- dbQuoteString(adm_con, passwd) } (que<- paste0("CREATE ROLE ",rolName, " WITH ",paste(superUser, createDb, createRole, login), ifelse(log,paste0(" PASSWORD " ,passwd),""), ifelse(noGroup,"",paste0(" IN GROUP " ,inGroup, " INHERIT")) )) if(grepl(";",que)|grepl("--",que)){stop("SQL injection attack... Why are you doing that?")} return(dbSendStatement(adm_con,que)) }

tomoakin commented 2 years ago

Never concatenate strings derived from untrusted source to SQL statement for security. Use placeholder instead are recommended. That is, dbSendQuery() with params parameter. Passwords, especially, should be allowed to contain ";" or "--".

dbQuoteString() may be simply created by renaming dbEscapeStrings() after carefully checking the spec.

dbQuoteIdentifier, dbQuoteLiteral is more complicated as to what to do by that function: Escpecially for dbQuoteId, how to deal with database.schema.table.culumn structure?

Individual quote is simple and internally defined and used as:

postgresqlQuoteId <- function(identifiers){
    ret <- paste('"', gsub('"','""',identifiers), '"', sep="")
    ret
}
postgresqlTableRef <- function(identifiers){
    ret <- paste('"', gsub('"','""',identifiers), '"', sep="", collapse=".")
    ret
}

dbQuoteLiteral needs to know the literal type expected in SQL side only based on the class of R, this may not be very clear.

marbotte commented 2 years ago

Thank you very much! I guess I worried to much: Since the superUser, login etc parameters are booleans in the R function, if the R function enforce them to be booleans (for example putting them in "if" clause), there are no risk of a user using them for SQL injection. Another thing I did not know is that the role names can be used in CREATE ROLE with double quote (so: dbQuoteIdentifier) I thought they needed to be unquoted, which shows that I still have to understand some basics of sql... Anyway I might not be the only one, that's why I write this before closing the issue!

So:

That would make something safe concerning SQL injection, without needing grepl(";",...) and grepl("--";)