Passing around Database Connection

[DoubleDee]

I am concerned that passing my DB Connection string from file to file is a security risk...


The actual DB Connection string is located in mysqli_connect.php...

    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
                OR die('Could not connect to database.  Contact System Administrator.'); 

Here is where I fear that I am doing something insecure...

In profile.php script, I include this DB connection like this...

        // Connect to the database.
        require_once(WEB_ROOT . 'private/mysqli_connect.php'); 

And then farther down in that same script I call this function...

            // Get # of Posts.
            $numberOfPosts = getNumberOfPosts($dbc, $id); 

The problem - as I found out last night - is that I can't require the DB Connection from inside my Function, so it looks like I have to pass it as an argument to my Function as seen above.

I am concerned that I am passing my Database's Username and Password for all to see in plain sight by doing this?!

I need some serious help/advice here!!


Debbie

[chris.upjohn]

No there are no security risks involved when passing in a database connection using a file include/require, as long as your file permissions for ALL php files are set to 0644 on Apache servers or 0666 on CGI servers you should never have to worry about someone been able to access your database information. As for passing in the database connection itself the easiest way is to just use the global keyword inside your function like the below example.

function getNumberOfPosts($id) {
    global $dbc;
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

[DoubleDee]

No there are no security risks involved when passing in a database connection using a file include/require, as long as your file permissions for ALL php files are set to 0644 on Apache servers or 0666 on CGI servers you should never have to worry about someone been able to access your database information. As for passing in the database connection itself the easiest way is to just use the global keyword inside your function like the below example.

function getNumberOfPosts($id) {
    global $dbc;
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

Using global anything is a bad idea, so I'd have to agree with that suggestion.


Debbie

[oddz]

Given your working with procedural code and *should avoid global variables I would recommend using statics.

function getDb($conn=null) { /* probably a bad name but I never do this and have no idea what to call it... I think drupal uses get for the same concept without OO */
    static $db;

    if($conn !== null) {
        $db = $conn;
    }
    
    return $db;
} 

Upon setting up the connection call the function passing the db resource and when you would like to get it after call the function without passing an arguments:

    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
                OR die('Could not connect to database.  Contact System Administrator.');  

   // set the connection reference
   getDb($dbh); 

function getNumberOfPosts($id) {

    $dbc = getDb(); /* now the function has a dependency on getDb() but I believe this is good method given the procedural nature of the code */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

In my opinion that is still really dirty and undesirable but it is probably the best solution aside from creating an object-oriented data access layer which would require completely changing existing architecture.

The other solution is passing the connection as an argument to every function that is dependent on it.

function getNumberOfPosts($id,$dbc) { /* all data access function would require passing the connection... using a function seems better */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

At least by using a function to get and set the connection some code repetition can be eliminated.

[DoubleDee]

Your right using a global would be the *simplest solution but not the preferred method. You would either need to pass the connection to the function as an argument, create a function that returns the db connection and call it from within the existing function or wrap all domain level database access functions in classes and save a reference to the db connection as a property upon instantiation. Your best bet is probably the second suggestion given what an already existing set-up.

I'm sorry, what are you saying that I should do?? (I could use a little hand-holding on this one...)

BTW, someone told me having my DB Connection string located in mysqli_connect.php and using constants like this was a bad idea...

<?php
    // This file contains the database access information.
    // It also establishes a connection to MySQL and selects the database.

    // Set the database access information as constants.
    DEFINE('DB_ENVIRONMENT', 'development');
//    define('DB_ENVIRONMENT', 'production');

    // Database Host.
    DEFINE('DB_HOST', ENVIRONMENT === 'development'
                    ? 'localhost'
                    : '');

    // Database User.
    DEFINE('DB_USER', ENVIRONMENT === 'development'
                    ? 'root'
                    : '');

    // Database Password.
    DEFINE('DB_PASSWORD', ENVIRONMENT === 'development'
                    ? 'root'
                    : '');

    // Database Name.
    define('DB_NAME', ENVIRONMENT === 'development'
                    ? 'doubledee'
                    : '');

    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
            OR die('Could not connect to database.  Contact System Administrator.');

They said using Constants was a dumb idea...


Debbie

[oddz]

I updated my response with more detail.

As for db configuration. Most systems store that information in some type of configuration file. Any configuration that changes based on environment should be stored in some type of configuration file and there should be a system in place to manage that info. Hard coding things may work but it is not ideal. Though the more ideal things become the more complex the code will become. So it is a trade off.

None the less, managing configuration is really another topic.

[DoubleDee]

Given your working with procedural code and *should avoid global variables I would recommend using statics.

function getDb($conn=null) { /* probably a bad name but I never do this and have no idea what to call it... I think drupal uses get for the same concept without OO */
    static $db;

    if($conn !== null) {
        $db = $conn;
    }
    
    return $db;
} 

Upon setting up the connection call the function passing the db resource and when you would like to get it after call the function without passing an arguments:

    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
                OR die('Could not connect to database.  Contact System Administrator.');  

   // set the connection reference
   getDb($dbh); 

I have LOTS of questions...

1.) This other person I was talking to made it sound like I should create DB Connection and then delete the components (e.g. DB name, DB password, etc) that created that DB Connection?!

What do you think?

2.) Can the DB Connection be "reverse engineered" so that a hacker could get the DB name, DB password, etc?

3.) According to the PHP Manual...

A static variable exists only in a local function scope, but it does not lose its value when program execution leaves this scope.

Doesn't that mean that $db would cease to exist out of the getDb function?

4.) What is wrong with how I am doing things now?

function getNumberOfPosts($id) {

    $dbc = getDb(); /* now the function has a dependency on getDb() but I believe this is good method given the procedural nature of the code */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

5.) Based on an error in my earlier code, I am pretty sure that a Function cannot see outside of itself. So how can you call your function from within the getNumberOfPosts fucntion?

6.) Why can't you just create another parameter and pass $db?

7.) And again, why is it bad to pass any Database Connection? (I'm not understanding were the "Attack Vectors" exist...

In my opinion that is still really dirty and undesirable but it is probably the best solution aside from creating an object-oriented data access layer which would require completely changing existing architecture.

I don't understand why you don't like this...

The other solution is passing the connection as an argument to every function that is dependent on it.

function getNumberOfPosts($id,$dbc) { /* all data access function would require passing the connection... using a function seems better */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
} 

At least by using a function to get and set the connection some code repetition can be eliminated.

Hmm....


Debbie

[oddz]

This other person I was talking to made it sound like I should create DB Connection and then delete the components (e.g. DB name, DB password, etc) that created that DB Connection?

You could but I don't see a benefit unless the program allows input of php from the UI and evals it out. Though at that point your looking at a whole slew of potential security issues. i don't think this is the case with what your doing though so it isn't really worth getting into indepth.

Can the DB Connection be "reverse engineered" so that a hacker could get the DB name, DB password, etc?

Not unless you do the above allowing people to enter PHP from a text area and eval it out. Ideally the db authentication info itself should be stored outside the site root to prevent any type of access beyond someone hacking the server itself.

Doesn't that mean that $db would cease to exist out of the getDb function?

Correct. If it existed outside the function and was accessible in any function it would be a global variable. The whole idea of a static variable is that the value can be managed from a single point, the function which it is defined. It is a tad less evil than a global though still unideal. When you start talking about storing state the best option is to move to an OO approach but that becomes much more complex. So what I presented I believe to be the best solution given the existing architecture, code flow and learning curve.

Based on an error in my earlier code, I am pretty sure that a Function cannot see outside of itself. So how can you call your function from within the getNumberOfPosts fucntion?

Functions can call other functions as long as they exist. That is a none issue. It only becomes more complex when introducing OOP (methods) into the mix.

Why can't you just create another parameter and pass $db?

You can but my preference is a getter/setter function to eliminate passing the db connection all the time.

And again, why is it bad to pass any Database Connection? (I'm not understanding were the "Attack Vectors" exist...

It isn't necessarily bad. No security issues exist with that. Like I said, I was using a getter/setter function to eliminate function arguments and avoid passing the connection around.

I don't understand why you don't like this...

It just all seems really dirty considering I'm more an MVC, OOP guy. You have to start somewhere though and I can't say I have never written nor managed similar code because I have. Though a lot of things change in 3 years. If this is what your comfortable with than so be it. I was merely providing a disclaimer for someone who comes along and starts to mention a more advanced method to handle things, like a OO data access layer. If someone else wants to get into a whole conversation over using OO to do this they can but my recommendation was provided with the smallest leanring curve in mind, avoiding global variables.

[DoubleDee]

oddz,

Thanks for the detailed responses so far!!

This other person I was talking to made it sound like I should create DB Connection and then delete the components (e.g. DB name, DB password, etc) that created that DB Connection?

You could but I don't see a benefit unless the program allows input of php from the UI and evals it out. Though at that point your looking at a whole slew of potential security issues. i don't think this is the case with what your doing though so it isn't really worth getting into indepth.

I thought that sounded a little paranoid myself...

They made it sound like you start up things like a Web Server and then delete all of the evidence?!

Not to get ahead, but from what I have learned the following things would help protect my original mysqli_connect.php...
- Store File outside of Web Root
- Store File on another Server
- Possibly store in a Database on another Server

Can the DB Connection be "reverse engineered" so that a hacker could get the DB name, DB password, etc?

Not unless you do the above allowing people to enter PHP from a text area and eval it out. Ideally the db authentication info itself should be stored outside the site root to prevent any type of access beyond someone hacking the server itself.

Okay.

Doesn't that mean that $db would cease to exist out of the getDb function?

Correct. If it existed outside the function and was accessible in any function it would be a global variable. The whole idea of a static variable is that the value can be managed from a single point, the function which it is defined. It is a tad less evil than a global though still unideal.

Your use of a function seems pretty slick. Why do you say that? Please explain.

When you start talking about storing state the best option is to move to an OO approach but that becomes much more complex. So what I presented I believe to be the best solution given the existing architecture, code flow and learning curve.

Well, because I hope to start learning OOP after I get this Release #2 done, I am interested in learning more.

Could you please explain why you feel an OOP approach is so superior on this topic? Maybe explain it to me like you'd explain it to a friend who isn't so technical?

Based on an error in my earlier code, I am pretty sure that a Function cannot see outside of itself. So how can you call your function from within the getNumberOfPosts function?

Functions can call other functions as long as they exist. That is a none issue. It only becomes more complex when introducing OOP (methods) into the mix.

Okay.

Why can't you just create another parameter and pass $db?

You can but my preference is a getter/setter function to eliminate passing the db connection all the time.

Okay.

And again, why is it bad to pass any Database Connection? (I'm not understanding were the "Attack Vectors" exist...

It isn't necessarily bad. No security issues exist with that. Like I said, I was using a getter/setter function to eliminate function arguments and avoid passing the connection around.

So it is convenience and efficiency that you like, not security per se, right?

I don't understand why you don't like this...

It just all seems really dirty considering I'm more an MVC, OOP guy.

Well, to clarify, when you say "dirty" I think "insecure"...

If you meant that you just don't like the coding style, that is a whole other issue, and much less of a concern to me!

You have to start somewhere though and I can't say I have never written nor managed similar code because I have. Though a lot of things change in 3 years.

You lost me... What changed in 3 years? (OOP has been around for 20 years.)

If this is what your comfortable with than so be it. I was merely providing a disclaimer for someone who comes along and starts to mention a more advanced method to handle things, like a OO data access layer. If someone else wants to get into a whole conversation over using OO to do this they can but my recommendation was provided with the smallest leanring curve in mind, avoiding global variables.

I appreciate your time so far, oddz, and I think I pretty much follow your examples, which is more than I can say about anyone who starts talking OOP!!!

I'm just trying to be certain that I understand everything top to bottom so I don't hang myself...

Thanks,


Debbie

[oddz]

I thought that sounded a little paranoid myself...

They made it sound like you start up things like a Web Server and then delete all of the evidence?!

Not to get ahead, but from what I have learned the following things would help protect my original mysqli_connect.php...
- Store File outside of Web Root
- Store File on another Server
- Possibly store in a Database on another Server

I think the only thing worth considering there is placing the information outside the web root. Though if your configuration file is a php file than storing it outside the web root would be fine. Is it more secure to place it outside, perhaps but unless it is a readable format like XML than it isn't vital. For my personal framework I store configuration in an XML file so it must be outside the web root considering if someone knew the proper path they could read everything in the file which would be no good. However, with a PHP file you don't have that issue.

Your use of a function seems pretty slick. Why do you say that? Please explain.

Even though it *can be done I don't believe that functions should be used to store state in PHP. That is really the job of an object. If we were talking pure C it would be a different story but PHP adds on objects to easily manage state and have control over exposure. So really a more OO approach is what I would consider *ideal.

Could you please explain why you feel an OOP approach is so superior on this topic? Maybe explain it to me like you'd explain it to a friend who isn't so technical?

Always a difficult topic to explain. Once you have created several websites you will begin to see low level patterns emerging based on repeating similar tasks. Much of what OO accomplishes is creating reusable "snippets" to handle those tasks without repeating the same code over and over and over. That is fundamentally the purpose behind a code base like Codeignitor, Symfony 2, etc. Only all of tasks have already been organized and coded for you in a reusable manor. Now while the same thing *could be achieved using procedural code it wouldn't nearly be as elegant as with true OO. Drupal is a good example of that considering the core system is procedural.

One thing I hate doing is repeating things. I rather spend the time upfront to code something in a reusable manor if I anticipate it will be need to be reused in a slightly different context. Using OO concepts very much helps to aid in that goal. Though I don't think it is something you can really begin to fully understand until you have been through the motions of creating several different sites and see the patterns emerge yourself where a reusable code base would be useful for certain tasks.

So it is convenience and efficiency that you like, not security per se, right?

Not at all. Security is important. If anything I would say sacrifices come in terms of code efficiency to make something more maintainable or reusable. Any Framework will inherently sacrifice efficiency in some form to achieve maintainability goals. In most cases it is not really noticeable but it is something to be aware of when building one or using a pre-existing package.

Well, to clarify, when you say "dirty" I think "insecure"...

If you meant that you just don't like the coding style, that is a whole other issue, and much less of a concern to me!

Nahh… dirty doesn't mean insecure it has to do with code layout or "style: as you put it. Again… much of this comes back to OO but like I said we all have to start somewhere.

You lost me... What changed in 3 years? (OOP has been around for 20 years.)

I meant that over 3 years time my understanding and beliefs have changed. I mean… if you look back even 1 year from now and still believe in all the same things than your not progressing. Like anything we are constantly learning "better" ways to do things. That is all I meant. Though it comes from time and experience which can't be faked. So best thing to do in my opinion for someone just kinda starting out is provide the smallest learning curve and progress to more advanced topics as understanding increases. I mean… that seems to work for traditional academia.

I appreciate your time so far, oddz, and I think I pretty much follow your examples, which is more than I can say about anyone who starts talking OOP!!!

Exactly what I mean. While OOP/MVC may be what I prefer I don't think it is a good solution for you at this point. Stick with what you know and when your ready to learn OO/MVC than so be it. I think it does more harm than good to start talking about that before someone really has a solid understanding of the fundamentals so it is a conversation/approach I avoid recommending at this point until directly asked.

[DoubleDee]

oddz,

Thanks for all of the detailed responses to my questions!! (I think I have a slightly better understanding of things now.)

Now, not to look a gift horse in the mouth, but after sleeping on things I have to ask this...

As described in Post #1, it sounds like how my code and files are set up now are sufficiently secure, right?

And it sounds like your approach in Post #4, while maybe slightly more streamlined, is really no more secure than my code in Post #1, right?

If that is true, then I have to wonder if it would not make sense to just keep the code and approach I have for now...

Once I get this release (i.e. Release #2) done, I will seriously consider trying to convert things to OOP. And with that being said, there is no point of getting too fancy right now, since my current priorities are to write *secure* code, that delivers what it needs to, and get it out the door ASAP!!

While I understand your code in Post #4, it also means having to re-write all of my code in every script that needs a database connection which could get hairy.

By contrast, I currently only have one Function that needs a database connection passed to it, and my code seems to be doing that fine.

What do you think?


Debbie