New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constant-Time bin2hex() implementation #909

Closed
wants to merge 6,270 commits into
base: master
from

Conversation

@sarciszewski
Contributor

sarciszewski commented Nov 21, 2014

Contant time bin2hex() implementation, taken from jedisct1/libsodium@814df1e

@laruence

This comment has been minimized.

Member

laruence commented Nov 21, 2014

hmm... did you ever try to make after your fix?

 hex[i * 2U] = (char) (87 + b + (((b - 10) >> 31) & -39));

where is hex's definition?

and since hexconvtab not used anymore, the definition of it should also be removed.

thanks

@sarciszewski

This comment has been minimized.

Contributor

sarciszewski commented Nov 22, 2014

How does this look now? :)

@nikic

This comment has been minimized.

Member

nikic commented Nov 22, 2014

Can you provide information as to how this performs compared to the previous implementation? I'm not fully convinced that PHP has need for a constant time bin2hex implementation.

@sarciszewski

This comment has been minimized.

Contributor

sarciszewski commented Nov 22, 2014

I'm not fully convinced that PHP has need for a constant time bin2hex implementation.

Do PHP developers use bin2hex() (or base64_encode()) on cryptographic secrets for the purpose of storing in configuration files (.ini, .json, .yaml, or .conf.php)? If so, then PHP has a need for constant time implementations of these functions.

http://tonyarcieri.com/cream-the-scary-ssl-attack-youve-probably-never-heard-of
http://cr.yp.to/antiforgery/cachetiming-20050414.pdf

(Spoiler: Yes, we do use these features to encode cryptographic secrets. So I would argue that they are a good enhancement to include in PHP 7)

@smalyshev

This comment has been minimized.

Contributor

smalyshev commented Nov 23, 2014

I think this needs some discussion on internals. If we worry about such things just replacing random functions is not enough - you should be sure all functions that handle your secret are constant-time, including the engine primitives, etc. I'm not sure just having one function does anything. But maybe I'm missing something here.

@sarciszewski

This comment has been minimized.

Contributor

sarciszewski commented Nov 23, 2014

If we worry about such things just replacing random functions is not enough - you should be sure all functions that handle your secret are constant-time, including the engine primitives, etc.

This is just a start. I'd love to discuss it with "internals", but my only exposure to PHP's core dev is through github.

@laruence

This comment has been minimized.

Member

laruence commented Nov 25, 2014

@sarciszewski you can drop a mail to internals at lists.php.net :)

@nikic

This comment has been minimized.

Member

nikic commented Nov 29, 2014

@sarciszewski

This comment has been minimized.

Contributor

sarciszewski commented Nov 29, 2014

Ah, thank you.

laruence and others added some commits Jan 13, 2015

Ipmrove strtr() by maintaining a set of characters that may start a m…
…atched pattern and avoid zend_hash_find() calls for other paterns.
Improve "instanceof". Interfaces of the left operand should be checke…
…d only if the right operand is interafce itself.
@nikic

This comment has been minimized.

Member

nikic commented on Zend/zend_operators.c in 2325758 Jan 14, 2015

typo inerface -> interface

This comment has been minimized.

Contributor

dstogov replied Jan 14, 2015

dstogov and others added some commits Jan 14, 2015

Merge branch 'PHP-5.6'
* PHP-5.6:
  Update NEWS
  Fixed bug #55618 (use case-insensitive cert name matching)

laruence and others added some commits Jan 28, 2015

Merge branch 'master' of https://git.php.net/push/php-src
* 'master' of https://git.php.net/push/php-src:
  Fixed #68868 (Segfault in clean_non_persistent_constants() in SugarCRM 6.5.20)
Merge branch 'PHP-5.6'
Conflicts:
	ext/phar/phar_object.c
Yasuo Ohgaki
Merge branch 'PHP-5.6'
* PHP-5.6:
  Fixed Bug #68941 mod_files.sh is a bash-script
Yasuo Ohgaki
Yasuo Ohgaki
Merge branch 'pull-request/1016'
* pull-request/1016:
Implement RFC: Introduce session_start() options
Yasuo Ohgaki
Yasuo Ohgaki
Merge branch 'PHP-5.6'
* PHP-5.6:
  Use bash rather than sh
Yasuo Ohgaki
@sarciszewski

This comment has been minimized.

Contributor

sarciszewski commented Jan 29, 2015

Ugh, this got messy. Time to start over.

@sarciszewski sarciszewski deleted the sarciszewski:patch-1 branch Jan 29, 2015

@laruence

This comment has been minimized.

Member

laruence commented on ext/session/session.c in e6c8640 Apr 22, 2015

seems this function is not exposed in session_functions? is that by intentional?

@rlerdorf

This comment has been minimized.

Member

rlerdorf commented on ext/intl/uchar/uchar.c in ebb60ac Jul 1, 2015

Hey @sgolemon what's with these proto comments? They are getting pulled out by the various doc scripts used by IDEs and other tools and they aren't the right format

@githubjeka

This comment has been minimized.

githubjeka commented on c8576c5 Sep 21, 2015

Why do you use : ?

public function getAddress() AddressInterface {
   return $this->address;
}

instead of the code

public function getAddress() : AddressInterface {
   return $this->address;
}

This comment has been minimized.

Contributor

morrisonlevi replied Sep 21, 2015

I didn't really care about a symbol but it made it compatible with Hack/HHVM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment