Bug 2643

This is the infamous Turkish dotless i confusion and will break stuff wherever we naively tolower a compare a string and compare it.

We lowercase strings in:

auth.c
match.c
misc.c
misc.h
pkcs11.h
readconf.c
ssh.c
sshconnect.c

Given that this is a behaviour change, I propose that we replace the handful of calls with tolower() with one that uses the C locale always. This best matches what openssh <7.3 did.

Created attachment 2908 [details]
use an ASCII tolower replacement for lowercasing strings

Comment on attachment 2908 [details]
use an ASCII tolower replacement for lowercasing strings

>+int
>+tolowerc(int c)
>+{
>+	if (c < (int)'A' || c > (int)'Z')

That's wrong for EBCDIC systems since the alphabet is not contiguous.

>+		return c;
>+	return c - ((int)'A' - (int)'a');
>+}

Does the C standard guarantee that the offset between upper and lowercase characters is constant?  It is in ASCII and EBCDIC.  What else is out there?

Created attachment 2910 [details]
use ctype functions instead of roll-ur-own

Comment on attachment 2910 [details]
use ctype functions instead of roll-ur-own

Not sure I want to go with the approach

Created attachment 2911 [details]
fall back to C locale for LC_CTYPE

I think this is less terrible - it falls back to the C locale for tr* locales, and tries to preserve UTF-8 if the system supports C.UTF-8

Created attachment 2912 [details]
fall back to C locale for LC_CTYPE

Helps if I attach the correct diff...

Thank you for your work Damien Miller.

Do I have to compile ssh with the last patch you wrote in order to test whether patched ssh fixes the bug or not?

Wow, that's really pretty ugly, and I'm not certain that Turkish is the only locale with this problem.

How about a simple char[256] table lookup instead?  That shouldn't be too ugly (still less code than the complicated setlocale fiddling) and it ought to deal with EBCDIC.

(In reply to Numan Demirdöðen from comment #8)

> Do I have to compile ssh with the last patch you wrote in order to
> test whether patched ssh fixes the bug or not?

Yes, you'd need to recompile. I've already reproduced and tested the C locale fix though.

(In reply to Colin Watson from comment #9)
> Wow, that's really pretty ugly, and I'm not certain that Turkish is
> the only locale with this problem.
> 
> How about a simple char[256] table lookup instead?  That shouldn't
> be too ugly (still less code than the complicated setlocale
> fiddling) and it ought to deal with EBCDIC.

I think falling back to C locale is preferable. It matches the behaviour for OpenSSH <7.3 best while giving us a chance at preserving UTF-8 output. I'll tidy up that patch - there are a couple of minor problems with it.

Created attachment 2913 [details]
Cleaned-up fallback diff

Previous fallback diff with a few fixes:

1) Correct precedence order for environment variables

2) Correctly terminate environment search if we find a non-TR locale

3) Check for a couple more variants of UTF-8 (lowercase and without hyphen)

4) Also try POSIX.UTF-8 locale if C.UTF-8 doesn't exist.

5) Stick the while mess in utf8.c where it better belongs. I preferred this rather than doing a xsetlocale, because I expect we'll upstream something like the final version in future.

6) Add a longwinded XXX comment describing how to fix this better in the future :)

Thanks to Ingo Schwarze for valuable feedback.

Patch applied - this will be in openssh-7.4

Close all resolved bugs after release of OpenSSH 7.7.