Wireshark · Wireshark-dev: Re: [Wireshark-dev] Remove our bundled crypto library (in favor of Libgcrypt)?

[Erik de Jong <erikdejong@xxxxxxxxx>]

On Sun, Feb 12, 2017 at 3:38 PM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:

On Sun, Feb 12, 2017 at 02:40:03PM +0100, Pascal Quantin wrote:
> Le 12 févr. 2017 11:12, "Erik de Jong" <erikdejong@xxxxxxxxx> a écrit :
> On Sat, Feb 11, 2017 at 10:38 PM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:
> > (forgot to attach the file lists...)
>
> I'll get to work on the aes_cmac_encrypt_* and crypt_* symbols. Will you
> make a separate change for this on which we'll both work or is it
> additional work on 20030?

You can create a separate change, 20030 is focussed on making Libgcrypt
mandatory but will not rewrite other parts (in order to make review
easier).

Alright! I've removed the md4 and rc4 symbols. Don't feel confident enough to remove the crypt_des_ecb calls because I'm not sure what is happening exactly, so I'm worried it'll break.

Looks like wsutil/aes.h is also used by epan/dissectors/packet-dof.c and epan/crypt/airpdcap_ccmp.c for rijndael_encrypt(), so we ought to put those on the list as well.

 

> > On Sat, Feb 11, 2017 at 10:35:10PM +0100, Peter Wu wrote:
> > > On Sat, Feb 11, 2017 at 09:31:17PM +0100, Erik de Jong wrote:
> > > > On Sat, Feb 11, 2017 at 8:55 PM, Peter Wu <peter@xxxxxxxxxxxxx> wrote:
> > > [..]
> > > > > My original goal was to replace wsutil by an existing crypto library
> > > > > (case 2). Since we Libgcrypt is already used in a lot of places, it
> > > > > seemed natural to replace wsutil by Libgcrypt.
> > > > >
> > > > > When trying to do so, I noticed that having an optional Libgcrypt
> > makes
> > > > > it much harder and hence changeset 20030 was created first to make it
> > > > > mandatory. Once that is in place, we can change the wsutil crypto
> > users
> > > > > to Libgcrypt. I plan to start working on that in the next days, let
> > me
> > > > > know if you want to join this effort :-)
> > > > >
> > > >
> > > > I'd like to help out, please tell me how I can assist in a way that
> > won't
> > > > be counterproductive.
> > >
> > > Thanks!  The following files need to be modified / removed:
> > >
> > >     debian/libwsutil0.symbols
> > >     wsutil/CMakeLists.txt
> > >     wsutil/Makefile.am
> > >     wsutil/aes.c
> > >     wsutil/aes.h
> > >     wsutil/des.c
> > >     wsutil/des.h
> > >     wsutil/md4.c
> > >     wsutil/md4.h
> > >     wsutil/md5.c
> > >     wsutil/md5.h
> > >     wsutil/rc4.c
> > >     wsutil/rc4.h
> > >     wsutil/sha1.c
> > >     wsutil/sha1.h
> > >     wsutil/sha2.c
> > >     wsutil/sha2.h
> > >
> > > The symbols to be removed are:
> > >
> > > - aes_cmac_encrypt_finish@Base 2.1.0
> > > - aes_cmac_encrypt_starts@Base 2.1.0
> > > - aes_cmac_encrypt_update@Base 2.1.0
> > > - crypt_des_ecb@Base 1.12.0~rc1
> > > - crypt_md4@Base 1.12.0~rc1
> > > - crypt_rc4@Base 1.12.0~rc1
> > > - crypt_rc4_init@Base 1.12.0~rc1
> > > - md5_append@Base 1.12.0~rc1
> > > - md5_finish@Base 1.12.0~rc1
> > > - md5_hmac@Base 1.12.0~rc1
> > > - md5_hmac_append@Base 1.12.0~rc1
> > > - md5_hmac_finish@Base 1.12.0~rc1
> > > - md5_hmac_init@Base 1.12.0~rc1
> > > - md5_init@Base 1.12.0~rc1
> > > - sha1_finish@Base 1.12.0~rc1
> > > - sha1_hmac@Base 1.12.0~rc1
> > > - sha1_hmac_finish@Base 1.12.0~rc1
> > > - sha1_hmac_starts@Base 1.12.0~rc1
> > > - sha1_hmac_update@Base 1.12.0~rc1
> > > - sha1_starts@Base 1.12.0~rc1
> > > - sha1_update@Base 1.12.0~rc1
> > > - sha256_finish@Base 2.1.0
> > > - sha256_hmac@Base 2.1.0
> > > - sha256_hmac_finish@Base 2.1.0
> > > - sha256_hmac_starts@Base 2.1.0
> > > - sha256_hmac_update@Base 2.1.0
> > > - sha256_starts@Base 2.1.0
> > > - sha256_update@Base 2.1.0
> > >
> > > Attached are the files that need to be modified (one list for any
> > > occurrence per file, one list with files grouped per function).
> > >
> > > The first three functions were "recently" added (see git logs) and is
> > > only used in airpdcap code.
> > >
> > > Looking at crypt_des_ecb, the users are packet-ntlmssp.c and
> > > packet-dcerpc-netlogin.c. These also use md5 a lot and crypt_rc4.
> > > Do you have any preference for a file/crypto function to tackle?
> > >
> > > Note that the current minimum Libgcrypt version is 1.4.2. For CMAC you
> > > need Libgcrypt 1.6.0 or newer. All other functions have been available
> > > for longer time. References that might be helpful:
> > > https://wiki.wireshark.org/Development/Support_library_version_tracking#Libgcrypt
> > > https://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=blob;f=NEWS
> > > https://gnupg.org/documentation/manuals/gcrypt/
> > >
> >
>
> So we'll have to update the minimum required version to 1.6.0 as well then.
>
> Or we could use conditional compilation against the libgcrypt version like
> what is done in packet-pdcp-lte.c.

Increasing the minimum to 1.6 will drop support for RHEL6 and RHEL7
(especially when Libgcrypt becomes mandatory), so it will be better to
make it conditional and return AIRPDCAP_RET_UNSUCCESS if not available.

--
Kind regards,
Peter Wu
https://lekensteyn.nl
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev@xxxxxxxxxxxxx>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request@wireshark.org?subject=unsubscribe