Security Review - implementazione password_hash per PHP

Question

Security Review - implementazione password_hash per PHP

#1 da (4 voti)
#2 da (1 voti)
#3 da (0 voti)

45

Attualmente sto lavorando a una "funzione di supporto" per il nucleo di PHP per rendere l'hashing delle password più sicuro e più semplice per la maggior parte degli sviluppatori. Fondamentalmente, l'obiettivo è renderlo così facile, che è più difficile inventare la propria implementazione piuttosto che usare quella core sicura. È anche progettato per essere aggiornato e ampliato in futuro con l'ottimizzazione dei metodi di hashing.

Ho scritto un RFC per l'aggiunta , e ho un implementazione delle funzioni .

La premessa di base è che crypt è troppo difficile da utilizzare direttamente correttamente per la maggior parte degli sviluppatori. L'errore strano ritorna, il sale base-like (ma usando un alfabeto diverso), ecc. Quindi queste funzioni sono progettate per portare fuori quella congettura e fornire un'API semplice e sporca.

string password_hash(string $password, string $algo = PASSWORD_DEFAULT, array $options = array())
bool password_verify(string $password, string $hash)
string password_make_salt(int $length, bool $raw_output = false)

Password_Hash accetta una password, uno specificatore di algoritmo opzionale (al momento, solo l'implementazione CRYPT_BCRYPT migliorata è supportata, ma vorrebbe aggiungere scrypt come opzione in seguito) e una matrice di opzioni. L'array di opzioni può specificare il parametro cost su bcrypt e un valore di sale predefinito.

Password_Verify accetta una password e un hash esistente. Quindi re-hash la password (identica a $tmp = crypt($password, $hash) ). Quindi, utilizza una funzione di confronto a tempo costante per determinare se i due hash sono effettivamente uguali.

Password_Make_Salt esiste per generare una stringa casuale della lunghezza specificata. Se raw_output è impostato su false (predefinito), l'output "salt" sarà codificato in base64 in un modo che è direttamente compatibile con crypt() . Se raw_output è true, restituirà la stessa stringa di lunghezza usando full-byte casuali ( 0-255 ).

Esempio:

$hash = password_hash("foo");
if (password_verify("foo", $hash)) {
    // always should be true
} else {
}

Una nota sulla lettura del codice sorgente di PHP: le funzioni PHP (quelle esposte al codice php) sono circondate dalla macro PHP_FUNCTION() . Inoltre, le variabili PHP (zval) vengono utilizzate in alcuni punti. Le macro che accedono a parti di esse sono

Z_TYPE_P() (trovare il tipo di puntatore a zval)
Z_STRVAL_P() (ottieni il puntatore al valore stringa)
Z_STRLEN_P() (ottieni la int lunghezza del tipo di stringa)
Z_LVAL_P() (ottieni il valore long del tipo intero)

Inoltre, zval_ptr_dtor() è un meccanismo refcount per ridurre il Refcount su zval , e pulirlo se colpisce 0 .

Sto cercando una revisione dell'attuazione da parte di almeno alcuni esperti di sicurezza prima di proporre ufficialmente il cambiamento. È ragionevolmente breve (solo circa 300 righe di codice) ...

Aggiornamento

L'API è stata approvata, quindi ho impostato una richiesta di pull per la funzionalità. Si prega di rivederlo se avete tempo. Grazie !: link

c php passwords hash code-review

posta ircmaxell 27.06.2012 - 05:00

fonte

3 risposte

Leggi altre domande sui tag c php passwords hash code-review

Ho appena inviato username e password su https. Va bene? Come funziona l'autenticazione con chiave pubblica ssh?

score 4 · Answer 1

In C, in genere raccomando di utilizzare coerentemente size_t per memorizzare tutti i valori di lunghezza, non int o long . L'utilizzo di int e long ti espone ai bug con segno intero / senza segno. Riferimento: INT01-C da lo standard CERT C Secure Coding .

Tuttavia, vi è una complicazione in questo caso, a causa del modo in cui l'interfaccia del codice nativo PHP funziona:

Quando leggi un intero usando zend_parse_parameters() , PHP si aspetta di memorizzarlo in long . Pertanto, dovresti passare zend_parse_parameters() un puntatore a long , quindi convertire il long in size_t con attenzione: convalidare che il numero sia non negativo e rientri nell'intervallo per size_t ( l >= 0 && l < SIZE_MAX ), quindi lanciarlo su size_t e utilizzare successivamente size_t .
Quando leggi una stringa usando zend_parse_parameters() , PHP si aspetta di passare un buffer per memorizzare la stringa e un puntatore a int per memorizzare la lunghezza della stringa. Pertanto, dovresti passare zend_parse_parameters() a ciò che si aspetta. Quindi, converti con cautela int in size_t : verifica che la lunghezza sia non negativa e rientri nell'intervallo per size_t ( len >= 0 && len < SIZE_MAX ), quindi esegui il cast in size_t e utilizza un size_t successivamente.

Allo stesso modo, Z_STRLEN_PP() restituisce un int (credo), quindi probabilmente vorrai fare lo stesso per questo.

(Attenzione a passare un long come lunghezza a memcpy() ; non lo so per certo, ma quel codice ha un odore abbastanza dubbioso per me e ad alto rischio per un bug intero di casting / troncamento.)

buffer = php_base64_encode((unsigned char*) str, str_len, NULL);
for (pos = 0; pos < out_len; pos++) {
    if (buffer[pos] == '+') {
         ...
    } else {
         ret[pos] = buffer[pos];

Sembra che possa leggere oltre la fine di buffer per me. Penso che tu abbia bisogno di un controllo aggiuntivo per assicurarti che pos sia minore della lunghezza di buffer .

score 1 · Answer 2

Iniziativa molto bella!

Commenti rapidi
(Leggerò la RFC più in dettaglio più tardi.)

"i sali devono essere unici in un sistema" è un'ipotesi errata; Leggi anche: questa risposta sulla salatura

Non vedo quale sia la necessità di fornire manualmente un sale per l'algoritmo di BCrypt. Per quanto posso vedere, si apre solo un'opzione per le persone di fare errori inutili.

password_make_salt sembra essere più correlato alla famiglia di funzioni crypt_; magari rinominarlo crypt_make_salt invece, per evitare confusione.

il primo esempio di utilizzo di base promuove l'uso di un valore costante 'usesomesillystringfor', molti sviluppatori faranno l'errore di utilizzare una sorta di valore costante invece di un appropriato (casuale) sale.

Il password_make_salt dovrebbe usare la migliore fonte casuale possibile, crittograficamente sicuro se possibile (non sono sicuro della qualità php_win32_get_random_bytes (), il / dev / urandom è buono)

l'esempio user_needs_rehash.php non è molto leggibile; potrebbe usare una pulizia.

l'esempio specific_salt.php suggerisce che è possibile fornire byte casuali come valore sale valido per l'algoritmo BCrypt. Questo contraddice il requisito dell'alfabeto base64 personalizzato per BCrypt.

score 0 · Answer 3

Da una prospettiva di hashing, utilizzare Bcrypt è una delle tecniche più consigliate in questi giorni, quindi va bene.

Lasciare che l'utente scelga di usare un altro algoritmo (come Scrypt, e forse la sua stessa implementazione in seguito?) sarebbe anche una grande opzione: per impostazione predefinita (e per gli sviluppatori PHP non esperti), dovranno semplicemente usare il BCrypt predefinito e, se lo desiderano, saranno in grado di adattarsi alle loro esigenze.