Skip to content

Bug #38917 - Native SPKAC support request #267

Closed
wants to merge 16 commits into from

6 participants

@jas-
jas- commented Jan 30, 2013

Wed Jan 30 09:06:49 MST 2013 - Pull request addressing feature request #38917 for native SPKAC support. Four new functions along with test cases:

(string) openssl_spki_new((object) private_key, (string) challenge, (int) algorithm)
(bool) openssl_spki_verify((string) spkac)
(string) openssl_spki_export((string) spkac)
(string) openssl_spki_export_challenge((string) spkac)

@smalyshev smalyshev commented on an outdated diff Feb 17, 2013
ext/openssl/openssl.c
@@ -242,6 +245,16 @@ enum php_openssl_cipher_type {
ZEND_ARG_INFO(0, key)
ZEND_END_ARG_INFO()
+#if OPENSSL_VERSION_NUMBER >= 0x10000000L
+ZEND_BEGIN_ARG_INFO_EX(arginfo_openssl_pbkdf2, 0, 0, 4)
@smalyshev
smalyshev added a note Feb 17, 2013

Is this supposed to be part of the patch too? Discussion does not mention it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@smalyshev smalyshev commented on an outdated diff Feb 17, 2013
ext/openssl/openssl.c
+ if (spki == NULL) {
+ php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unable to allocate memory for public key");
+ goto cleanup;
+ }
+
+ RETVAL_STRING(ASN1_STRING_data(spki->spkac->challenge), 1);
+ goto cleanup;
+
+cleanup:
+ if (spkstr_cleaned != NULL) {
+ efree(spkstr_cleaned);
+ }
+}
+/* }}} */
+
+/* {{{ proto int openssl_spki_cleanup(const char *src, char *results)
@smalyshev
smalyshev added a note Feb 17, 2013

Internal functions don't need protos.

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

It would be nice to also add some tests for failure scenarios.

@smalyshev smalyshev commented on an outdated diff Feb 17, 2013
ext/openssl/openssl.c
+ if (spkstr_cleaned != NULL) {
+ efree(spkstr_cleaned);
+ }
+
+ if (i > 0) {
+ RETVAL_TRUE;
+ }
+}
+/* }}} */
+
+/* {{{ proto string openssl_spki_export(string spki)
+ Exports public key from existing spki to var */
+PHP_FUNCTION(openssl_spki_export)
+{
+ int spkstr_len;
+ char *spkstr, * spkstr_cleaned, * s;
@smalyshev
smalyshev added a note Feb 17, 2013

These variables are not initialized. This leads to segfault of wrong string.

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

This code segfaults: php -r 'var_dump(openssl_spki_export("2438"));'
Probably because of missing variable initializations. Please clean it up and add failure scenario tests.

@jas-
jas- commented Feb 20, 2013

@smalyshev I have fixed the requested memory leak, changed the internal function away from the proto keyword and created error test cases as requested. Thanks for your help!

@jas-
jas- commented Mar 30, 2013

@smalyshev sorry if your busy with other things, when you get a minute I was just wondering about the status of this patch.

@smalyshev

@jas- Well, right now for 5.5 we need approval of the RM (Julien Pauli) or we'll have to wait for 5.5.1.

@jas-
jas- commented Mar 31, 2013

Great, thanks!

@jas- jas- referenced this pull request in jas-/SPKAC-PHP-OpenSSL Apr 8, 2013
Closed

Crash with PHP 5.5 beta 2 #1

jas- added some commits Apr 28, 2013
@jas- jas- Sun Apr 28 22:34:52 MDT 2013 - Merged master with upstream from php-src 617c3a9
@jas- jas- Sun Apr 28 23:04:28 MDT 2013 - Resolving rebase 63620da
@jas- jas- Merge branch 'issue-38917-spkac', remote branch 'upstream/master' 307f201
@jas- jas- Merge branch 'PHP-5.4' into PHP-5.5
* PHP-5.4: (22175 commits)
  fix valgrind warning
  Fix bug #64023 (__toString() & SplFileInfo)
  Fix test related to change for #bug64007 and also fix in newInstanceArgs
  NEWS: Start PHP 5.5.0 Beta 1 section
  NEWS for PHP 5.5.0alpha4
  Fix NEWS
  fix tests
  Merge fix of #62836 to ?.re, and regenerate ?.c
  Fixed bug #64007 (There is an ability to create instance of Generator by hand).
  - Fixed ZTS build
  Class Name Resolution As Scalar Via "class" Keyword
  fix bug #63462 (Magic methods called twice for unset protected properties)
  NEWS for bug #64011. See 77ee200
  Fix bug #64011 (get_html_translation_table())
  Update config.sub to latest GPLv2 upstream version
  Fixed bug #63988 (Two Date tests fail) only for PHP-5.5
  Fix skipif section
  Added ARM accelerated implementations for ZEND_SIGNED_MULTIPLY_LONG()
  Added test cases for ZEND_SIGNED_MULTIPLY_LONG()
  Fix News
  ...
12130ed
@jas- jas- Merge branch 'PHP-5.5'
* PHP-5.5:
  Sat Apr  7 21:02:37 MDT 2012 - Fixed segfaults and should just need to clean up any memory leaks
  Tue Apr  3 22:27:41 MDT 2012 - Split SPKI family of test cases into individual files as requested
  Tue Apr  3 21:21:20 MDT 2012 - Additional checks for supported algorithm types
  Tue Apr  3 14:41:40 MDT 2012 - SPKI functions, recommended changes and patch for bug id #61421
  Tue Apr  3 14:40:49 MDT 2012 - SPKI functions, recommended changes and patch for bug id #61421

Conflicts:
	ext/openssl/openssl.c
	ext/openssl/tests/openssl_spki_export.phpt
	ext/openssl/tests/openssl_spki_export_challenge.phpt
	ext/openssl/tests/openssl_spki_new.phpt
	ext/openssl/tests/openssl_spki_verify.phpt
bb8d842
@jas- jas- Mon Apr 29 00:11:12 MDT 2013 - Merge e67d3be
@jas- jas- Mon Apr 29 01:12:33 MDT 2013 - Working copy 9748d6c
@pierrejoye

Can you do a PR with the latest code? Or request an account to maintain it in ext/openssl? add me as sponsor.

Thanks for your work!

@jas-
jas- commented May 1, 2013

No problem, its only a small contribution @pierrejoye. I ran into some snags getting the upstream PHP 5.5 changes merged into my local branch but they have been resolved and all changes should now be in this pull request.

Asking for an account involves posting a message to internals I am assuming?

@kaplanlior

@jas- a mail to internals would be a good start, but you also have to use this form: http://www.php.net/git-php.php

@jas-
jas- commented Jul 17, 2013
@kaplanlior

@jas- You're right, committed in http://git.php.net/?p=php-src.git;a=commit;h=8f56ac8401ed1c3e00b8fba3fb8e5efb565180c4 and the bug should also be closed.

@php-pulls

Comment on behalf of stas at php.net:

merged

@php-pulls php-pulls closed this Jul 18, 2013
@Tyrael Tyrael added a commit that referenced this pull request Feb 3, 2014
@Tyrael Tyrael mention the openssl SPKAC functions commited to master before branchi…
…ng out 5.6. see #267
5d3a2fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.