Please login or register.

Login with username, password and session length
Advanced search  

News:

Latest sNews - sNews 1.7 - with its own forums - for discussion and user mods.

Author Topic: More secure initial setup  (Read 3970 times)

codetwist

  • Hero Member
  • *****
  • Karma: 50
  • Posts: 940
More secure initial setup
« on: March 04, 2007, 09:26:48 pm »

I've modified my sNews code snews_startup() function in a following manner:
Code: [Select]
   connect_to_db();
   if (get_id('category') == 'rss') {rss(); die;}
   else if (get_id('category') == 'sitemap.xml') {xmlsitemap(); die;}
   update_articles();
   if (isset($_POST['Loginform'])) {
      $user = checkUserPass($_POST['uname']);
      $pass = checkUserPass($_POST['pass']);
      // Pre-validate $user and $pass before production use ( no test or user equal to pass allowed )
      if ( ($user === $pass) || ($user === 'test') || ($pass === 'test') )
         $prodReady = false;
      else
         $prodReady = true;
      # math captcha check
      $calc = checkMathCaptcha( $_POST['sum'], $_POST['calc'] );
      if (md5($user) === s('username') && md5($pass) === s('password') && $calc && $prodReady) {
         $_SESSION[db('website').'Logged_In'] = token();
      }
   }
Just to make sure that after initial copying of brand new system it doesn't allow to enter administration with fancy username and or password 'out of the box'. May be something similar could be implemented in stock package as well.

Additionaly, added few more paranoidal thingies:
- added same check in function processing() to disallow changes with too simple values
- added input fields for old username&password in settings() form
- added math captcha in settings form for username/password
- added check if old username/password are correct ones and if captcha validates

The rest of code follows.

I introduced that function checkMathCaptcha() after release of 1.5.31 where obviously were bug instead of captcha checking. And I didn't wanted later to fix it again in several places across whole sNews code.
Code: [Select]
function checkMathCaptcha( $inSum = '', $inCalc = '') {
   $result = false;
   if ( is_numeric($inSum) && is_numeric($inCalc) && ($inSum == $inCalc) ) {
      $result = true;
   }
   return $result;
}
In function settings() in a section for username/password change added two fields that require entry of old username and password as well as added call for mathCaptcha():
Code: [Select]
...
   html_input('form', '', '', '', '', '', '', '', '', '', '', '', 'post', 'index.php?action=process&task=changeup', '');
   html_input('fieldset', '', '', '', '', '', '', '', '', '', '', '', '', '', '<a title="'.l('change_up').'" onclick="toggle(\'sub6\')" style="cursor: pointer;">'.l('change_up').'</a>');
   echo '<div id="sub6" style="display: none;">';
   echo '<p>'.l('login_limit').'</p>';
   // MOD - security - 20070304
   // Added fields for current username/password   
   html_input('text', 'curuname', 'curuname', '', '* '.l('old_username'), 'text', '', '', '', '', '', '', '', '', '');
   html_input('password', 'curpass', 'curpass', '', '* '.l('old_password'), 'text', '', '', '', '', '', '', '', '', '');
   //
   html_input('text', 'uname', 'uname', '', l('new_username'), '', '', '', '', '', '', '', '', '', '');
   html_input('password', 'pass1', 'pass1', '', l('new_password'), '', '', '', '', '', '', '', '', '', '');
   html_input('password', 'pass2', 'pass2', '', l('new_password2'), '', '', '', '', '', '', '', '', '', '');
   // MOD - security - 20070304
   // Added math captcha as well   
   echo mathCaptcha();
   //
   echo '<p>';
   html_input('hidden', 'task', 'task', 'changeup', '', '', '', '', '', '', '', '', '', '', '');
   html_input('submit', 'submit_pass', 'submit_pass', l('save'), '', '', '', '', '', '', '', '', '', '', '');
   echo '</p></div></fieldset></form>';
...
For above code added few language var-s:
Code: [Select]
   $l['old_password'] = 'Old Password';
   $l['old_username'] = 'Old Username';
   $l['new_password'] = 'New Password';
   $l['new_password2'] = 'New Password again';
   $l['new_username'] = 'New Username';
And finally in function processing() at section that deals with changeup (username and password) added following code that checks old username/password, new username/password and math captcha:
Code: [Select]
...
   # change user name and password
   case 'changeup':
      if (isset($_POST['submit_pass'])) {
         // MOD - security - 20070304
         // Added processing of  current username/password
         $curUser = checkUserPass($_POST['curuname']);
         $curPass = checkUserPass($_POST['curpass']);
         // Added math captcha checking
         $calc = checkMathCaptcha( $_POST['sum'], $_POST['calc'] );
         if ( md5($curUser) === s('username') && md5($curPass) === s('password') && $calc)
            $allowChange = true;
         else
            $allowChange = false;
         $user = checkUserPass($_POST['uname']);
         $pass1 = checkUserPass($_POST['pass1']);
         $pass2 = checkUserPass($_POST['pass2']);
         if (!empty($user) && !empty($pass1) && !empty($pass2) && ($pass1 === $pass2) ) {
            // MOD - security - 20070304
            // Added 'quality' checking of new username/password
            // Pre-validate $user and $pass before production use ( no test or user equal to pass allowed )
            if ( ($user === $pass1) || ($user === 'test') || ($pass1 === 'test') )
               $prodReady = false;
            else
                $prodReady = true;
            // MOD - security - 20070304
            // Proceed with change only if passed old credentials check and new 'quality' check
            if ( $allowChange && $prodReady ) {
               $uname = md5($user);
               $pass = md5($pass2);
               $query = "UPDATE ".db('prefix')."settings SET VALUE=";
               mysql_query($query."'$uname' WHERE name='username' LIMIT 1;");
               mysql_query($query."'$pass' WHERE name='password' LIMIT 1;");
               echo notification('','','administration/');
            }
            else {
               // FIX THIS: Little bit misleading notification
               echo notification(l('pass_mismatch'),'','settings/');
            }
         }
         else {echo notification(l('pass_mismatch'),'','settings/');}
      }
      break;
...
Why I added need to enter old username/password to make changes in them ... well, that's pretty basic practice that I prefer to stick to.

Why require some quality username and password ... what kind of question is this anyway?

That's mostly all folks.
« Last Edit: September 24, 2007, 11:53:57 am by codetwist »
Logged

David

  • Hero Member
  • *****
  • Karma: 7
  • Posts: 567
More secure initial setup
« Reply #1 on: March 04, 2007, 09:42:56 pm »

good idea!
Logged

Luka

  • Administrator
  • ULTIMATE member
  • ******
  • Karma: 36
  • Posts: 1717
    • http://www.snewscms.com
More secure initial setup
« Reply #2 on: March 04, 2007, 09:45:47 pm »

Codetwist, could you explain what is the point of this addition?

If you changed your username and password, why would you restrict access with the same username and password, and username and password being set to 'test'?

When you install sNews on your site it takes a few seconds to login to admin and change the default credentials. If we implemented this in the stock package users should be aware and warned that they will have to change the production variable and reupload snews.php if they want the system to behave sane.
Logged

David

  • Hero Member
  • *****
  • Karma: 7
  • Posts: 567
More secure initial setup
« Reply #3 on: March 04, 2007, 09:50:16 pm »

I think it's useful for newbies who don't change the default login/password. But then this code should be in the official sNews package...
Logged

Keyrocks

  • Doug
  • ULTIMATE member
  • ******
  • Karma: 449
  • Posts: 6020
  • Semantically Challenged
    • snews.ca
More secure initial setup
« Reply #4 on: March 04, 2007, 09:51:00 pm »

Yeah... Codetwist... maybe you could shed some more light on this... like what is the problem you are trying to overcome with the proposed solution... and how does it solve it? :)
Logged
Do it now... later may not come.
-------------------------------------------------------------------------------------------------
sNews 1.6 MESU | sNews 1.6 MEMU

codetwist

  • Hero Member
  • *****
  • Karma: 50
  • Posts: 940
More secure initial setup
« Reply #5 on: March 04, 2007, 10:52:29 pm »

I'm human being and tends to forget thingies. With one code base I'll set up several sites and just don't want to find out that because default setup allows admin entry with test/test and I just forgot to change them - all my stuff is being owned. That's all.

Regarding few seconds ... well that would be even less if sNews have some kind of automated installer that would force these changes at once. Unfortunately, it has not and there tends to be pauses ... longer than few seconds ... coffee and all, I confess ;)

As for complete newbies ... well, I'd think it's better to force a little security on them even if they don't understand anything about it. In my opinion that's better but, hey, I'm just moding my instance and sharing my point of view.
« Last Edit: September 24, 2007, 11:54:31 am by codetwist »
Logged

codetwist

  • Hero Member
  • *****
  • Karma: 50
  • Posts: 940
More secure initial setup
« Reply #6 on: March 04, 2007, 11:00:37 pm »

And somehow I don't think that user thinks that system is sane when he gots that Defaced by yet another uber-snooper just because there was that oh-so-convenient test/test gateway.

As for not allowing username equal to password ... well, why make life too easy for those uber-snoopers? But again ... this is just my point of view and I'm going to enforce it only on my customers.
« Last Edit: September 24, 2007, 11:54:55 am by codetwist »
Logged

Luka

  • Administrator
  • ULTIMATE member
  • ******
  • Karma: 36
  • Posts: 1717
    • http://www.snewscms.com
More secure initial setup
« Reply #7 on: March 04, 2007, 11:09:43 pm »

Quote from: codetwist
And somehow I don't think that user thinks that system is sane when he gots that Defaced by yet another uber-snooper just because there was that oh-so-convenient test/test gateway.
Sites got defaced because of the security flaw in 1.5.30. Also If the user installs sNews and leaves the default username and password it's pretty obvious it could get defaced.

Automatic installer for sNews could easily be done but it would require snews.php to be writable by server if we want to store the mysql database information on the top aswell as the path to sNews installation. Alternative would be a mysql populator.
Logged

codetwist

  • Hero Member
  • *****
  • Karma: 50
  • Posts: 940
More secure initial setup
« Reply #8 on: March 04, 2007, 11:13:15 pm »

I know that my approach isn't the best or universal. It's just solving my problems fast and to the point. And these posts that surfaced lately regarding defacement as well as popular relying on hidden login link made me a little paranoidal.
« Last Edit: September 24, 2007, 11:55:23 am by codetwist »
Logged

Luka

  • Administrator
  • ULTIMATE member
  • ******
  • Karma: 36
  • Posts: 1717
    • http://www.snewscms.com
More secure initial setup
« Reply #9 on: March 04, 2007, 11:18:59 pm »

You provided a suggestion with a list of features but included only a snippet of the whole code. Because the code is not complete and only serves to solve your problem with forgetting username and password, this won't be included in the default package.
Logged

piXelatedEmpire

  • MIA
  • ULTIMATE member
  • ******
  • Karma: 37
  • Posts: 1401
  • currently MIA
More secure initial setup
« Reply #10 on: March 05, 2007, 01:18:01 am »

I think these changes you have made codetwist are quite interesting.. perhaps you could provide those of us interested with a little step by step about what you have modded and why? I for one would be interested to take a peek :)
Logged
my apologies to the sNews crew, but I will be MIA for the forseeable future

codetwist

  • Hero Member
  • *****
  • Karma: 50
  • Posts: 940
More secure initial setup
« Reply #11 on: March 05, 2007, 09:01:26 am »

I'll drop in rest of code later, after my working day.

Initial post updated with rest of code.
« Last Edit: September 24, 2007, 11:55:55 am by codetwist »
Logged