Skip to content
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

Fix #80344: mysqlnd support for SASL SCRAM-SHA-1 and SCRAM-SHA-256 au… #6417

Closed
wants to merge 6,829 commits into from
Closed

Fix #80344: mysqlnd support for SASL SCRAM-SHA-1 and SCRAM-SHA-256 au… #6417

wants to merge 6,829 commits into from

Conversation

fjanisze
Copy link

…thentication

This will introduce support for LDAP SASL authentication using
SCRAM-SHA-1 and SCRAM-SHA-256, at the moment those methods
are supported only by the enterprise version of MySQL, but are
widely used.

Girgias and others added 30 commits October 23, 2020 20:47
Small drive by refactoring to use HashTables

Closes GH-6371
* PHP-8.0:
  Throw Value/TypeError for invalid $bodies in imap_mail_compose()
Otherwise, `ADD_EXTENSION_DEP('foo', 'json')` fails, even though the
JSON extension is available.
* PHP-8.0:
  Define config var PHP_JSON
* PHP-8.0:
  [ci skip] Fix typos in UPGRADING
This reverts commit ef6adb4.

Per Ondrej's comment, this is already being used by BetterReflection
adaptors, ugh.
* PHP-8.0:
  Revert "Make ReflectionUnionType final"
* PHP-8.0:
  Document breaking change in strspn/strcspn

[ci skip]
* PHP-8.0:
  Fix parameter stats generation

[ci skip]
Make sure the directory is not modified while we're iterating it,
which may give unstable results.
* PHP-8.0:
  Use separate directory in dit_004.phpt
* PHP-7.4:
  Fix bug 76618
  Fix bug 76618
* PHP-8.0:
  Fix bug 76618
  Fix bug 76618
* PHP-7.4:
  Fix #80280: ADD_EXTENSION_DEP() fails for ext/standard and ext/date
* PHP-8.0:
  Fix #80280: ADD_EXTENSION_DEP() fails for ext/standard and ext/date
* PHP-7.4:
  Fix #80258: Windows Deduplication Enabled, randon permission errors
* PHP-8.0:
  Fix #80258: Windows Deduplication Enabled, randon permission errors
If Zip operations fails due to PHP error conditions before libzip even
has been called, there is no meaningful indication what failed; the
functions just return false, and the Zip status indicated that no error
did occur.  Therefore we raise `E_WARNING` in these cases.

Closes GH-6356.
* PHP-7.4:
  Fix #62474: com_event_sink crashes on certain arguments
* PHP-8.0:
  Fix #62474: com_event_sink crashes on certain arguments
These are not exactly representative anymore.
* PHP-7.4:
  Declare may_retry_reparse_point on windows only
* PHP-8.0:
  Declare may_retry_reparse_point on windows only
This throws a deprecation warning in XDebug 3.

Closes GH-6324.
* PHP-8.0:
  Don't set xdebug.default_enable in run-tests
nikic and others added 16 commits November 9, 2020 10:19
* PHP-8.0:
  Fixed bug #80334
All other leftovers of this feature have been dropped in PHP 8,
so we should remove the property as well.

Closes GH-6407.
* PHP-8.0:
  Remove embedded property from mysqli_driver
I made some mistakes on this code, which meant that not everything which
should be tested was actually being tested.
There is no meaningful difference between these and UCS-{2,4}. They are
just a little bit more lax about passing errors silently. They also have
no known use.

Alias to UCS-{2,4} in case someone, somewhere is using them.
- Reject otherwise valid kuten codes which don't map to anything in JIS X 0208.
- Handle truncated multi-byte characters as an error.
- Convert Shift-JIS 0x7E to Unicode 0x203E (overline) as recommended by the
  Unicode Consortium, and as iconv does.
- Convert Shift-JIS 0x5C to Unicode 0xA5 (yen sign) as recommended by the
  Unicode Consortium, and as iconv does.
  (NOTE: This will affect PHP scripts which use an internal encoding of
  Shift-JIS! PHP assigns a special meaning to 0x5C, the backslash. For example,
  it is used for escapes in double-quoted strings. Mapping the Shift-JIS yen
  sign to the Unicode yen sign means the yen sign will not be usable for
  C escapes in double-quoted strings. Japanese PHP programmers who want to
  write their source code in Shift-JIS for some strange reason will have to
  use the JIS X 0208 backlash or 'REVERSE SOLIDUS' character for their C
  escapes.)
- Convert Unicode 0x5C (backslash) to Shift-JIS 0x815F (reverse solidus).
- Immediately handle error if first Shift-JIS byte is over 0xEF, rather than
  waiting to see the next byte. (Previously, the value used was 0xFC, which is
  the limit for the 2nd byte and not the 1st byte of a multi-byte character.)
- Don't allow 'control characters' to appear in the middle of a multi-byte
  character.

The test case for bug 47399 is now obsolete. That test assumed that a number
of Shift-JIS byte sequences which don't map to any character were 'valid'
(because the byte values were within the legal ranges).
- Don't allow control characters to appear in the middle of a multi-byte
  character. (A strange feature, or perhaps misfeature, of mbstring which is
  not present in other libraries such as iconv.)
- When checking whether string is valid, reject kuten codes which do not
  map to any character, whether converting from EUC-JP to another encoding,
  or converting another encoding which uses JIS X 0208/0212 charsets to
  EUC-JP.
- Truncated multi-byte characters are treated as an error.
mbstring had an 'identify filter' for almost every supported text encoding
which was used when auto-detecting the most likely encoding for a string.
It would run over the string and set a 'flag' if it saw anything which
did not appear likely to be the encoding in question.

One problem with this scheme was that encodings which merely appeared
less likely to be the correct one were completely rejected, even if there
was no better candidate. Another problem was that the 'identify filters'
had a huge amount of code duplication with the 'conversion filters'.

Eliminate the identify filters. Instead, when auto-detecting text
encoding, use conversion filters to see whether the input string is valid
in candidate encodings or not. At the same type, watch the type of
codepoints which the string decodes to and mark it as less likely if
non-printable characters (ESC, form feed, bell, etc.) or 'private use
area' codepoints are seen.

Interestingly, one old test case in which JIS text was misidentified
as UTF-8 (and this wrong behavior was enshrined in the test) was 'fixed'
and the JIS string is now auto-detected as JIS.
* PHP-7.4:
  Fixed bug #80310: Support for icu4c 68.1.
* PHP-8.0:
  Fixed bug #80310: Support for icu4c 68.1.
And drop the U_DEFINE_TRUE_AND_FALSE flag.
* PHP-8.0:
  Use true/false instead of TRUE/FALSE in intl
* PHP-7.4:
  Fix phi use chain management when renaming variable
* PHP-8.0:
  Fix phi use chain management when renaming variable
…thentication

This will introduce support for LDAP SASL authentication using
SCRAM-SHA-1 and SCRAM-SHA-256, at the moment those methods
are supported only by the enterprise version of MySQL, but are
widely used.
Copy link
Contributor

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! :)

ext/mysqlnd/mysqlnd_auth.c Outdated Show resolved Hide resolved
ext/mysqlnd/mysqlnd_auth.c Outdated Show resolved Hide resolved
@fjanisze fjanisze requested a review from cmb69 November 23, 2020 09:06
Copy link
Contributor

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

I can't really comment on the actual implementation, but I see a few general issues:

  • the code formatting looks rather uncommon; please try to keep the style of the respective source file
  • on Windows, mysqlnd is built statically; with this patch, libsasl would be a hard dependency, what is not desired (cf. PR Add Windows support for caching_sha2_password #5129); not sure how to handle this
  • I'm not sure that targeting PHP-7.4 is appropriate; @derickr may have an opinion on that

@fjanisze
Copy link
Author

Thanks, will make sure to have a deeper look at the formatting. As for the dependency on libsasl I'm not sure what I can do, is that all the libs ought to be always and only statically linked on windows? That might be a short of an issue since I expect to deliver support for ldap kerberos and pure kerberos authentication mechanism as well, those are going to be supported by the latest commercial flavours of MySQL (The user should be given the option to use those meccanism along with mysqlnd) and also have lib dependencies coming along..

Would that work for the community if all those mechanisms are disabled and would require manual building to make them work, so that no hard dependency is introduced in "vanilla" releases? What kind of alternative do I have?

Also, I think I'll submit a pull for master instead of PHP-7.4, one collaborator of mine has recently been asked to move some new code to master instead of 7.4 as well so probably this pull will end up the same way, I think...

@derickr
Copy link
Contributor

derickr commented Nov 23, 2020

I don't think this is appropriate for PHP 7.4.

@fjanisze
Copy link
Author

I've created a new pull request to master: #6447 I'll close this request.

@fjanisze fjanisze closed this Nov 23, 2020
@cmb69
Copy link
Contributor

cmb69 commented Nov 23, 2020

Would that work for the community if all those mechanisms are disabled and would require manual building to make them work, so that no hard dependency is introduced in "vanilla" releases? What kind of alternative do I have?

Yes, something like that should work. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet