F30 Self-Contained Change proposal: libcrypt.so.1 (compatibility library for POSIX): Let encrypt, encrypt_r, setkey, setkey_r, and fcrypt return ENOSYS instead of performing any real operation

[Ben Cotton]

Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
and fcrypt from the libxcrypt.so.1 compatibility library and let those
functions set "errno" to "ENOSYS" when invoked.


== Owner ==
* Name: [[User:besser82 | Björn Esser]] <besser82(a)fedoraproject.org&gt;

== Detailed Description ==
In [[Changes/FullyRemoveDeprecatedAndUnsafeFunctionsFromLibcrypt | the
system-wide change: "Fully remove deprecated and unsafe functions from
libcrypt"]] we propose to remove the named functions from the system
default so-version of the crypt library, which is a pre-requirement
for this change.  It basically is a follow-up announcing to remove the
real functionality from those functions in the POSIX-compatibility
version of the crypt library as well, and replace them with fully
POSIX-compilant stubs, that properly indicate those functions are not
supported.

This change was basically filed, because it may require additional
documentation apart from the system-wide change, that is a
pre-requirement for this one.


== Benefit to Fedora ==
Third-party applications, which still use / require these unsafe
functions cannot use them anymore, which is the key benefit of this
change for our users.


== Scope ==
* Proposal owners:  Implement the needed changes in the
libxcrypt-compat package.
* Other developers: N/A (not a System Wide Change)
* Release engineering: N/A, as this is a follow-up of an already
evaluated system-wide change.
* Trademark approval: N/A (not needed for this Change)


== Upgrade/compatibility impact ==
N/A (not a System Wide Change)


== How To Test ==
N/A (not a System Wide Change)


== User Experience ==
No impact, as nothing in the distribution uses those functions.
Third-party applications that are distributed in pre-compiled / binary
form only, may have a reduced set of features.  Since POSIX explicitly
states those functions shall set "errno" to "ENOSYS", if the
functionality is not available nor implemented, third-party
application that start to malfunction are broken by design.


== Dependencies ==
N/A (not a System Wide Change)


== Contingency Plan ==
Revert the change and rebuild libxcrypt.


== Documentation ==
The version of the libcrypt.so.1 library included with Fedora 30 for
POSIX-compatibility has entirely removed the functionality of the
encrypt, encrypt_r, setkey, setkey_r, and fcrypt functions, while
keeping fully binary compatibility with existing (third-party)
applications possibly still using those funtions.  If such an
application attemps to call one of these functions, the corresponding
function will indicate that it is not supported by the system in a
POSIX-compliant way.

All existing binary executables linked against glibc's libcrypt should
work unmodified with this version of the libcrypt.so.1 library
supplied by the libxcrypt-compat package.


== Release Notes ==
See the paragraph about documentation above.


-- 
Ben Cotton
Fedora Program Manager
TZ=America/Indiana/Indianapolis

[Florian Weimer]

* Ben Cotton:

>  Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
>  and fcrypt from the libxcrypt.so.1 compatibility library and let those
>  functions set "errno" to "ENOSYS" when invoked. 
encrypt rewrites its argument in place, so this will leave the argument
unencrypted.  This does not seem a good idea, even if it's just DES.

Thanks,
Florian

[Simo Sorce]

On Tue, 2019-01-15 at 10:39 +0100, Florian Weimer wrote:
>  * Ben Cotton:
>  
>  > Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
>  > and fcrypt from the libxcrypt.so.1 compatibility library and let those
>  > functions set "errno" to "ENOSYS" when invoked.
>  
>  encrypt rewrites its argument in place, so this will leave the argument
>  unencrypted.  This does not seem a good idea, even if it's just DES. 
Maybe encrypt with AES and return an error anyway ?

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

[Tom Hughes]

On 15/01/2019 13:42, Simo Sorce wrote:
>  On Tue, 2019-01-15 at 10:39 +0100, Florian Weimer wrote:
> > * Ben Cotton:
> >
> >> Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
> >> and fcrypt from the libxcrypt.so.1 compatibility library and let those
> >> functions set "errno" to "ENOSYS" when invoked.
> >
> > encrypt rewrites its argument in place, so this will leave the argument
> > unencrypted.  This does not seem a good idea, even if it's just DES.
>  
>  Maybe encrypt with AES and return an error anyway ? 
Or just fill the buffer with some constant - zero or 0xdeadbeef
or whatever = and return an error.

Tom

-- 
Tom Hughes (tom(a)compton.nu)
http://compton.nu/

[Florian Weimer]

* Simo Sorce:

>  On Tue, 2019-01-15 at 10:39 +0100, Florian Weimer wrote:
> > * Ben Cotton:
> > 
> > > Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
> > > and fcrypt from the libxcrypt.so.1 compatibility library and let those
> > > functions set "errno" to "ENOSYS" when invoked.
> > 
> > encrypt rewrites its argument in place, so this will leave the argument
> > unencrypted.  This does not seem a good idea, even if it's just DES.
>
>  Maybe encrypt with AES and return an error anyway ? 
It's still only got a 56-bit key.  AES would only make dictionary
attacks easier because there are more efficient AES implementations than
DES implementations.

Maybe the stub implementation should just overwrite the argument with
zeros.

Thanks,
Florian

[Simo Sorce]

On Tue, 2019-01-15 at 14:51 +0100, Florian Weimer wrote:
>  * Simo Sorce:
>  
>  > On Tue, 2019-01-15 at 10:39 +0100, Florian Weimer wrote:
>  > > * Ben Cotton:
>  > > 
>  > > > Remove real functionality from encrypt, encrypt_r, setkey, setkey_r,
>  > > > and fcrypt from the libxcrypt.so.1 compatibility library and let those
>  > > > functions set "errno" to "ENOSYS" when invoked.
>  > > 
>  > > encrypt rewrites its argument in place, so this will leave the argument
>  > > unencrypted.  This does not seem a good idea, even if it's just DES.
>  > 
>  > Maybe encrypt with AES and return an error anyway ?
>  
>  It's still only got a 56-bit key.  AES would only make dictionary
>  attacks easier because there are more efficient AES implementations than
>  DES implementations. 
You could use a random key, but yeah if you need to simply make it
inoperable just overwrite with random.

>  Maybe the stub implementation should just overwrite the argument
> with
>  zeros. 
I wouldn't overwrite with zeros because then it is clear the encryption
failed and if it is used in non-orthodox ways could give an attacker a
way to exploit the zeroing.

(for example if someone uses it to encrypt a password, instead of
hashing it and then compare to some stored value, then zeroing might be
a bad choice as all invocations will always return the same value and
would always compare "right")

Simo.

-- 
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc

[Florian Weimer]

* Simo Sorce:

> > Maybe the stub implementation should just overwrite the argument
> with
> > zeros.
>
>  I wouldn't overwrite with zeros because then it is clear the encryption
>  failed and if it is used in non-orthodox ways could give an attacker a
>  way to exploit the zeroing.
>
>  (for example if someone uses it to encrypt a password, instead of
>  hashing it and then compare to some stored value, then zeroing might be
>  a bad choice as all invocations will always return the same value and
>  would always compare "right") 
That's a fair point.  Overwriting with random data seems better.
(There's precedent for doing that on decryption failures, too.)

Thanks,
Florian

[Björn 'besser82' Esser]

Am Dienstag, den 15.01.2019, 15:20 +0100 schrieb Florian Weimer:
>  * Simo Sorce:
>  
>  > > Maybe the stub implementation should just overwrite the argument
>  > > with
>  > > zeros.
>  > 
>  > I wouldn't overwrite with zeros because then it is clear the
>  > encryption
>  > failed and if it is used in non-orthodox ways could give an attacker
>  > a
>  > way to exploit the zeroing.
>  > 
>  > (for example if someone uses it to encrypt a password, instead of
>  > hashing it and then compare to some stored value, then zeroing might
>  > be
>  > a bad choice as all invocations will always return the same value
>  > and
>  > would always compare "right")
>  
>  That's a fair point.  Overwriting with random data seems better.
>  (There's precedent for doing that on decryption failures, too.)
>  
>  Thanks,
>  Florian 

Thanks for the thoughts and a easy solution, guys!  I've updated the
description and documentation of the proposal accordingly:

>  The encrypt{,_r} function will - for security reasons - additionally
>  overwrite the data-block argument with random data. 
Björn

[Code complete, needs testing] Re: F30 Self-Contained Change proposal: libcrypt.so.1 (compatibility library for POSIX): Let encrypt, encrypt_r, setkey, setkey_r, and fcrypt return ENOSYS instead of performing any real operation

[Björn 'besser82' Esser]

The change [1] has been successfully implemented and is code complete,
pending QA / testing.


You will need to have libxcrypt-compat >= 4.4.3-3.fc30 and some prebuilt
binary linking at least one of these functions installed for testing
this:

  * encrypt{,_r}
  * setkey{,_r}
  * fcrypt


Please test this change and report problems / bugs related to this
change on Bugzilla against the libxcrypt package of the Fedora
component.  Your bug should also block rhbz# 1670735, so I can track
everything much easier in one place.

Björn aka besser82


[1]  
https://fedoraproject.org/wiki/Changes/libcrypt_so_1_Let_encrypt_encrypt_...