Bug 219790 - Unable to build bhyve with error in rfb.c with LibreSSL in base
Summary: Unable to build bhyve with error in rfb.c with LibreSSL in base
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Marcelo Araujo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-04 23:08 UTC by Shirkdog
Modified: 2017-07-03 08:08 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Shirkdog 2017-06-04 23:08:27 UTC
Trying to build CURRENT as of 6/4/2017 12:00

Noticed parallel builds failing, so I removed my "make -j8" and found I was still getting the following errors while building:

/usr/src/usr.sbin/bhyve/rfb.c:835:25: error: expected expression
                DES_set_key((C_Block *)keystr, &ks);
                                      ^
/usr/src/usr.sbin/bhyve/rfb.c:835:16: error: use of undeclared identifier 'C_Block'
                DES_set_key((C_Block *)keystr, &ks);
                             ^
/usr/src/usr.sbin/bhyve/rfb.c:836:29: error: expected expression
                DES_ecb_encrypt((C_Block *)challenge,
                                          ^
/usr/src/usr.sbin/bhyve/rfb.c:836:20: error: use of undeclared identifier 'C_Block'
                DES_ecb_encrypt((C_Block *)challenge,
                                 ^
/usr/src/usr.sbin/bhyve/rfb.c:837:15: error: expected expression
                                (C_Block *)crypt_expected, &ks, DES_ENCRYPT);
                                          ^
/usr/src/usr.sbin/bhyve/rfb.c:837:6: error: use of undeclared identifier 'C_Block'
                                (C_Block *)crypt_expected, &ks, DES_ENCRYPT);
                                 ^
/usr/src/usr.sbin/bhyve/rfb.c:838:29: error: expected expression
                DES_ecb_encrypt((C_Block *)(challenge + PASSWD_LENGTH),
                                          ^
/usr/src/usr.sbin/bhyve/rfb.c:838:20: error: use of undeclared identifier 'C_Block'
                DES_ecb_encrypt((C_Block *)(challenge + PASSWD_LENGTH),
                                 ^
/usr/src/usr.sbin/bhyve/rfb.c:839:15: error: expected expression
                                (C_Block *)(crypt_expected + PASSWD_LENGTH),
                                          ^
/usr/src/usr.sbin/bhyve/rfb.c:839:6: error: use of undeclared identifier 'C_Block'
                                (C_Block *)(crypt_expected + PASSWD_LENGTH),
                                 ^
10 errors generated.
*** Error code 1

This is a HardenedBSD server with LibreSSL in base, however I noticed that in rfb.c if NO_OPENSSL is defined, <openssl/des.h> is included, and there is no C_Block struct in that header.

I noticed these functions expected a const_DES_cblock type. So making the following changes made bhyve build for me:

diff --git a/usr.sbin/bhyve/rfb.c b/usr.sbin/bhyve/rfb.c
index 3eb67314c02..c3c31729b96 100644
--- a/usr.sbin/bhyve/rfb.c
+++ b/usr.sbin/bhyve/rfb.c
@@ -832,11 +832,11 @@ rfb_handle(struct rfb_softc *rc, int cfd)
                memcpy(crypt_expected, challenge, AUTH_LENGTH);

                /* Encrypt the Challenge with DES */
-               DES_set_key((C_Block *)keystr, &ks);
-               DES_ecb_encrypt((C_Block *)challenge,
-                               (C_Block *)crypt_expected, &ks, DES_ENCRYPT);
-               DES_ecb_encrypt((C_Block *)(challenge + PASSWD_LENGTH),
-                               (C_Block *)(crypt_expected + PASSWD_LENGTH),
+               DES_set_key((const_DES_cblock *)keystr, &ks);
+               DES_ecb_encrypt((const_DES_cblock *)challenge,
+                               (const_DES_cblock *)crypt_expected, &ks, DES_ENCRYPT);
+               DES_ecb_encrypt((const_DES_cblock *)(challenge + PASSWD_LENGTH),
+                               (const_DES_cblock *)(crypt_expected + PASSWD_LENGTH),
                                &ks, DES_ENCRYPT);

                if (memcmp(crypt_expected, buf, AUTH_LENGTH) != 0) {


It looks like these changes were added with this commit:


commit fa2245832bde97021dc63f35729cb10228d0204d                                                                                                                                                           
Author:     araujo <araujo@FreeBSD.org>                                                                                                                                                                   AuthorDate: Fri Jun 2 02:35:16 2017 +0000                                                                                                                                                                 
Commit:     araujo <araujo@FreeBSD.org>                                                                                                                                                                   CommitDate: Fri Jun 2 02:35:16 2017 +0000                                                                                                                                                                 
                                                                                                                                                                                                              Add VNC Authentication support based on RFC6143 section 7.2.2.                                                                                                                                        
                                                                                                                                                                                                              Submitted by:   Fabian Freyer <fabian.freyer@physik.tu-berlin.de>                                                                                                                                     
    Reworked by:    myself                                                                                                                                                                                    Reviewed by:    grehan, rgrimes and jilles                                                                                                                                                            
    MFC after:      1 week.
    Relnotes:       Yes.
    Sponsored by:   iXsystems, Inc.
    Differential Revision:  https://reviews.freebsd.org/D10818
Comment 1 Ed Maste freebsd_committer freebsd_triage 2017-06-05 16:59:25 UTC
FreeBSD -CURRENT is building as of r319602 (see e.g. https://ci.freebsd.org/tinderbox/) so please provide more detail of your build environment.

(Not to say there's a problem with the change; it looks like it's probably reasonable - although looking only at the diff you posted I wonder why have casts at all.)
Comment 2 Shirkdog 2017-06-05 18:49:16 UTC
(In reply to Ed Maste from comment #1)

Right, this seems to be an issue with downstream FreeBSD projects with LibreSSL in base (should this not cause an issue in TrueOS?)

I will also defer to others as to whether the cast is necessary, as I matched the function variables being passed in and was happy my world built :)
Comment 3 Fabian Freyer 2017-06-05 23:38:10 UTC
Without having tested it, that change looks reasonable to me.
Comment 4 Shirkdog 2017-06-06 01:28:58 UTC
I did a test by adding "password=notsecurepass" to my bhyve rfb command line and I was prompted with vncviewer to enter the password and it worked. I also verified it will fail if I enter the wrong password.
Comment 5 Marcelo Araujo freebsd_committer freebsd_triage 2017-06-06 03:15:04 UTC
I'm taking it.
Comment 6 commit-hook freebsd_committer freebsd_triage 2017-06-08 04:54:37 UTC
A commit references this bug:

Author: araujo
Date: Thu Jun  8 04:54:16 UTC 2017
New revision: 319682
URL: https://svnweb.freebsd.org/changeset/base/319682

Log:
  Make the VNC authentication build with LibreSSL on HardenedBSD and TrueOS.

  PR:		219790
  Submitted by:	Shirkdog <mshirk@daemon-security.com>
  Reviewed by:	grehan and rgrimes
  MFC after:	4 weeks.
  Sponsored by:	iXsystems, Inc.
  Differential Revision:	https://reviews.freebsd.org/D11071

Changes:
  head/usr.sbin/bhyve/rfb.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2017-07-02 04:15:11 UTC
A commit references this bug:

Author: araujo
Date: Sun Jul  2 04:14:35 UTC 2017
New revision: 320567
URL: https://svnweb.freebsd.org/changeset/base/320567

Log:
  MFC r319487-r319488, r319682, r319968, r319995

  r319487:
  Add VNC Authentication support based on RFC6143 section 7.2.2.

  Submitted by:	Fabian Freyer <fabian.freyer@physik.tu-berlin.de>
  Reworked by:	myself
  Reviewed by:	grehan, rgrimes and jilles
  Relnotes:	Yes.
  Sponsored by:	iXsystems, Inc.
  Differential Revision:	https://reviews.freebsd.org/D10818

  r319488:
  Bump manpage date.

  r319682:
  Make the VNC authentication build with LibreSSL on HardenedBSD and TrueOS.

  PR:		219790
  Submitted by:	Shirkdog <mshirk@daemon-security.com>
  Reviewed by:	grehan and rgrimes
  Sponsored by:	iXsystems, Inc.
  Differential Revision:	https://reviews.freebsd.org/D11071

  r319968:
  Initialize variables and use byteorder(9) instead of aliasing char array
  buf via uint32_t pointer.

  CID:		1375949
  Reported by:	Coverity, cem
  Reviewed by:	cem
  Sponsored by:	iXsystems, Inc.
  Differential Revision:	https://reviews.freebsd.org/D11153

  r319995:
  Check if pthread_create(3) successfully created the thread prior to call
  pthread_join(3). The variable tid is not yet initialized in case
  the authentication fails at early stage, that would lead pthread_join be
  called with an uninitialized variable.

  CID:		1375950
  Reported by:	Coverity, cem
  Reviewed by:	cem
  Sponsored by:	iXsystems, Inc.
  Differential Revision:	https://reviews.freebsd.org/D11150

Changes:
_U  stable/11/
  stable/11/usr.sbin/bhyve/Makefile
  stable/11/usr.sbin/bhyve/bhyve.8
  stable/11/usr.sbin/bhyve/pci_fbuf.c
  stable/11/usr.sbin/bhyve/rfb.c
  stable/11/usr.sbin/bhyve/rfb.h