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

Support 8-bit characters in conf_def.h #5778

Closed
mattcaswell opened this issue Mar 28, 2018 · 0 comments
Closed

Support 8-bit characters in conf_def.h #5778

mattcaswell opened this issue Mar 28, 2018 · 0 comments
Labels
branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Milestone

Comments

@mattcaswell
Copy link
Member

mattcaswell commented Mar 28, 2018

This issue was introduced by the removal of a contribution because we could not locate the original author to gain approval for the licence change.

After the code removal the various IS_*() macros in the auto-generated file conf_def.h may incorrectly return true if the supplied character has its most significant bit set. The IS_*() macros should be able to correctly handle 8-bit characters. Note that UTF-8 support is not a requirement.

@mattcaswell mattcaswell added branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch labels Mar 28, 2018
@mattcaswell mattcaswell added this to the 1.1.1 milestone Mar 28, 2018
@levitte levitte removed their assignment Mar 29, 2018
mspncp added a commit to mspncp/openssl that referenced this issue Apr 2, 2018
Fixes openssl#5778, openssl#5840

The various IS_*() macros did not work correctly for 8-bit ASCII
characters with the high bit set, because the CVT(a) preprocessor
macro and'ed the given ASCII value with 0x7F, effectively folding
the high value range 128-255 over the low value range 0-127.
As a consequence, some of the IS_*() erroneously returned TRUE.

This commit fixes the issue by mapping CVT(a) to 127 for all values
a >= 127. This works because the lookup tables CONF_type_default
and CONF_type_win32 have all bits cleared at entry 127, whence
IS_*(a) returns FALSE for all values outside the range 0-127.

The IS_*() macros were also changed to return TRUE or FALSE (1 or 0)
instead of a nonzero or zero value.

Note that with this change, the macro IS_*(c,a) evaluates the 'a'
argument twice, unless CHARSET_EBCDIC is defined. This prohibits the
use of arguments with side effects like IS_*(c, *p++).
mspncp added a commit to mspncp/openssl that referenced this issue Apr 7, 2018
Fixes openssl#5778, openssl#5840

The various IS_*() macros did not work correctly for 8-bit ASCII
characters with the high bit set, because the CVT(a) preprocessor
macro and'ed the given ASCII value with 0x7F, effectively folding
the high value range 128-255 over the low value range 0-127.
As a consequence, some of the IS_*() erroneously returned TRUE.

This commit fixes the issue by adding range checks instead of
cutting off high order bits using a mask. In order avoid multiple
evaluation of macro arguments, most of the implementation was moved
from macros into a static function is_keytype().

Thanks to Румен Петров for reporting and analyzing the UTF-8 parsing
issue openssl#5840.
@levitte levitte closed this as completed in a9b7a06 Apr 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Projects
None yet
2 participants