Logging system safety

I've created a logging system with one-use passwords (sent via SMS API). What I would like to know is if it's "safe". I know it is hard to estimate safety but I am curious if it is safer than regular password. I would appreciate any advice to improve following code.

Scheme is:

Logging:

Loggin at a webpage -> Ask external server for password -> Store the password in session var and when form is sent compare it with the sent one

External script:

When asked with proper apikey -> give random password and send it via SMS API

The code is below:

External script:

Logging script (at page foo.com)

Log in again'; die();
        }
        if($_GET['destroyer']=='no'){
            session_destroy(); echo 'New pass has been sent!'; session_start();
        }
    }

    if (isset($_POST['password'])){
        if($_POST['password']==$_SESSION['pass']) $_SESSION[$sessi]="log";
        else echo'Wrong Password Send again';
    }

    if ($_SESSION[$sessi]!="log") { 
        if(!isset($_SESSION['pass'])){
            $adres='http://www.foobar.com/index.php?api=foobar123';
            $c = curl_init();
            curl_setopt($c, CURLOPT_URL, $adres);
            curl_setopt($c, CURLOPT_POST, 1);
            curl_setopt($c, CURLOPT_POSTFIELDS, "ip=$ip&www=$www"); 
            curl_setopt($c, CURLOPT_RETURNTRANSFER, 1); /
            $dane=curl_exec($c); 
            curl_close($c); 
            $_SESSION['pass']=$dane;
            echo 'Password has been sent';
        }
        echo'Log in';
    }
    else{//authorised
         echo 'Logged Logout

-
This probably makes integration and debugging harder:

if ($_GET['api']!=$apiKey) {
   header('HTTP/1.0 404 Not Found');
   die();
}

The error message is misleading, if someone use an older or wrong API key they probably will spend some time searching the error in a wrong place since it indicates that the URL is wrong, not the key. 403 Forbidden would be more convenient here.

-
This should be in a configuration file, it might change more often than the code itself:

$apiKey='foobar123';

-
This generates a random number:

$k=rand(1,10);

Documentation of mt_rand says the following:


mt_rand — Generate a better random value

So I guess using mt_rand would we better. Furthermore, the linked page contains another interesting thing:


[...]


Caution


This function does not generate cryptographically secure values,
and should not be used for cryptographic purposes. If you need a
cryptographically secure value, consider using openssl_random_pseudo_bytes() instead.

It really depends on the application and the sensitivity of your data which is more appropriate.

-
This usually referred as one-time password:


one-use passwords

-
This check could be at the beginning of the script:

if (!isset($_POST['ip'],$_POST['ip'])) {echo 'error'; die();}

It's unnecessary to create an one-time password if the ip is empty.

Additionally, I guess the following is the same:

if (!isset($_POST['ip'])) {echo 'error'; die();}

-
You could eliminate the comment here with better variable naming:

$ile=12; //char number

Just rename it to $passwordLength.

See also: Clean Code by Robert C. Martin, Bad Comments, p67: Don’t Use a Comment When You Can Use a Function or a Variable

-
Two typos:

• $adres should be $address

• $dane to $done

-
I'd rename $sessi to a descriptive name. What does this variable store, what's its purpose? Put that into the name.

-
I'd change log to something more descriptive here:

$_SESSION[$sessi]="log";

logged_in would be easier to understand since it describes a state not an action (like log).

StackExchange Code Review Q#18781, answer score: 5