Skip to content

[RFC]: Add Argon2 to password_* #1997

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Sep 8, 2016
Merged

[RFC]: Add Argon2 to password_* #1997

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
de85c2e
Implementing password_verify and password_get_info for Argon2
charlesportwoodii Jul 8, 2016
3c7fb71
Introducing Argon2 memory, time, and lanes constants
charlesportwoodii Jul 8, 2016
c2551a7
Working implementation with password_hash, password_verify
charlesportwoodii Jul 9, 2016
0a1274f
Adding test cases for Argon2i and Argon2d
charlesportwoodii Jul 9, 2016
deba2b4
Fixing spacing in php_password.h
charlesportwoodii Jul 9, 2016
1bc3818
Reverting PASSWORD_DEFAULT to PASSWORD_BCRYPT
charlesportwoodii Jul 10, 2016
bcfccdd
Removing argon2 library files in favor of --with-argon2[=DIR]
charlesportwoodii Jul 11, 2016
9f37be5
Fixing failing tests for Argon2
charlesportwoodii Jul 11, 2016
d398657
Adding Windows build flag for --with-argon2
charlesportwoodii Jul 12, 2016
f4aa3a4
Fixing linker issue on linux when DIR is specified on --with-argon2
charlesportwoodii Jul 12, 2016
9bedcb7
Adding to PHP library includes
charlesportwoodii Jul 12, 2016
1c954c9
Untouching old tests
charlesportwoodii Jul 12, 2016
9872208
Updating config.w32 to correctly add Argon2RefLib to LIBS flag
charlesportwoodii Jul 13, 2016
ab837a6
Fixing potential memory leak with encoded in password_hash
charlesportwoodii Jul 18, 2016
0d4d8ea
Removing Argon2d, changing config arg to --with-password-argon2
charlesportwoodii Aug 1, 2016
d883f65
Fixing issue with config.m4 script not correctly checking for PHP_PAS…
charlesportwoodii Aug 2, 2016
0e3b3b0
Changing m_cost and t_cost to memory_cost and time_cost
charlesportwoodii Aug 5, 2016
35a74b9
Fixing typo in tests
charlesportwoodii Aug 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions ext/standard/config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,38 @@ dnl Check for getrandom on newer Linux kernels
dnl
AC_CHECK_DECLS([getrandom])

dnl
dnl Check for argon2
dnl
PHP_ARG_WITH(argon2, for Argon2 support,
[ --with-argon2[=DIR] Include Argon2 support in password_*. DIR is the Argon2 shared library path]])

if test "$PHP_ARGON2" != "no"; then
AC_MSG_CHECKING([for Argon2 library])
for i in $PHP_ARGON2 /usr /usr/local ; do
if test -r $i/include/argon2.h; then
ARGON2_DIR=$i;
AC_MSG_RESULT(found in $i)
break
fi
done

if test -z "$ARGON2_DIR"; then
AC_MSG_RESULT([not found])
AC_MSG_ERROR([Please ensure the argon2 header and library are installed])
fi

PHP_ADD_LIBRARY_WITH_PATH(argon2, $ARGON2_DIR)
Copy link
Member

@remicollet remicollet Jul 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be $ARGON2_DIR/$PHP_LIBDIR

Copy link
Contributor Author

@charlesportwoodii charlesportwoodii Jul 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing.

PHP_ADD_INCLUDE($ARGON2_DIR/include)

AC_CHECK_LIB(argon2, argon2_hash, [
LIBS="$LIBS -largon2"
AC_DEFINE(HAVE_ARGON2LIB, 1, [ Define to 1 if you have the <argon2.h> header file ])
], [
AC_MSG_ERROR([Problem with libargon2.(a|so). Please verify that Argon2 header and libaries are installed])
])
fi

dnl
dnl Setup extension sources
dnl
Expand Down
12 changes: 12 additions & 0 deletions ext/standard/config.w32
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
// vim:ft=javascript
// $Id$

ARG_WITH("argon2", "Argon2 support", "no");

if (PHP_ARGON2 != "no") {
if (CHECK_LIB("Argon2Ref.lib", "argon2", PHP_ARGON2)
&& CHECK_HEADER_ADD_INCLUDE("argon2.h", "CFLAGS_ARGON2")) {
AC_DEFINE('HAVE_ARGON2LIB', 1);
EXTENSION("argon2", null, false);
Copy link
Member

@KalleZ KalleZ Jul 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right, simply drop the call to EXTENSION() here and leave the AC_DEFINE().

Copy link
Contributor Author

@charlesportwoodii charlesportwoodii Jul 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting linker errors without the call to EXTENSION. Is there another macro I can use to get Argon2Ref.lib to be added to the linker? I wasn't seeing any other function for handling that.

Copy link
Member

@KalleZ KalleZ Jul 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! It's been a while since I was tinkering around with this, but from vague memory this is how it works:

CHECK_LIB':

  • The second parameter is the target of where to link the Argon2Ref.lib to (as per extension), this can be set to empty ("") -- See win32\build\confutils.js
  • The third parameter is fine (as it uses PHP_ARGON2, which is the one from the --with-argon2=XXX)

CHECK_HEADER_ADD_INCLUDE

  • This sets the compiler flags for an extension (CFLAGS_ARGON2), and should simply be CFLAGS (we use this value for core things, like determining IPv6 support in win32\build\config.js).

TL;DR:
Line 7 should be: if (CHECK_LIB("Argon2Ref.lib", "", PHP_ARGON2)
Line 8 should be: && CHECK_HEADER_ADD_INCLUDE("argon2.h", "CFLAGS")) {

which should allow you to drop the EXTENSION() call.

(bear in mind I'm rusty, but else poke around the sources in win32\build)!

Copy link
Contributor Author

@charlesportwoodii charlesportwoodii Jul 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for pointing me in the right direction! I really appreciate it.

It looks like the trick is to set the second arguement of CHECK_LIB (target) to null instead of "". An empty string creates LIBS_ in the Makefile, whereas null correctly assigns it to LIBS.

Thanks again!

} else {
WARNING("Argon2 not enabled; libaries and headers not found");
}
}

ARG_WITH("config-file-scan-dir", "Dir to check for additional php ini files", "");

AC_DEFINE("PHP_CONFIG_FILE_SCAN_DIR", PHP_CONFIG_FILE_SCAN_DIR);
Expand Down
Loading