Bug 232133 - mail/libdomainkeys: Fix OpenSSL Build
Summary: mail/libdomainkeys: Fix OpenSSL Build
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Tobias Kortkamp
URL:
Keywords:
Depends on:
Blocks: 231931
  Show dependency treegraph
 
Reported: 2018-10-09 22:26 UTC by Nathan
Modified: 2018-11-05 21:11 UTC (History)
0 users

See Also:


Attachments
Fix OpenSSL 1.1.x build (1.53 KB, patch)
2018-10-09 22:26 UTC, Nathan
ndowens04: maintainer-approval+
Details | Diff
Fix OpenSSL 1.1.x build (5.89 KB, patch)
2018-10-10 23:21 UTC, Nathan
no flags Details | Diff
OpenSSL patch (1.04 KB, patch)
2018-10-12 13:53 UTC, Nathan
no flags Details | Diff
OpenSSL patch (10.36 KB, patch)
2018-10-12 13:57 UTC, Nathan
no flags Details | Diff
OpenSSL patch (6.09 KB, patch)
2018-10-12 21:41 UTC, Nathan
no flags Details | Diff
libdomainkeys.diff v6 (6.58 KB, patch)
2018-10-13 04:04 UTC, Tobias Kortkamp
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nathan 2018-10-09 22:26:17 UTC
Created attachment 197982 [details]
Fix OpenSSL 1.1.x build

mail/libdomainkeys: Fix OpenSSL build
 
 PR:             231931
 Submitted by:   Nathan <ndowens@yahoo.com>
 Approved by:    portmgr (unmaintained port)
Comment 1 Tobias Kortkamp freebsd_committer 2018-10-10 10:01:24 UTC
+-  EVP_MD_CTX mdctx;   /* the hash */
++  EVP_MD_CTX *mdctx;   /* the hash */

This is not ok.  You cannot just make things a pointer to fix "field
has incomplete type" style errors.

I have not tested on a branch with OpenSSL 1.1.1 but it breaks
releases with older OpenSSL versions and produces warnings like
the following:

domainkeys.c:1749:22: warning: incompatible pointer types passing 'EVP_MD_CTX **' (aka 'struct env_md_ctx_st **') to parameter of type 'EVP_MD_CTX *' (aka 'struct env_md_ctx_st *'); remove & [-Wincompatible-pointer-types]
    EVP_DigestUpdate(&dk->mdctx, dk->hash_buff, dk->hash_buff_len);
                     ^~~~~~~~~~
/usr/include/openssl/evp.h:595:34: note: passing argument to parameter 'ctx' here
int EVP_DigestUpdate(EVP_MD_CTX *ctx, const void *d, size_t cnt);
                                 ^
domainkeys.c:1952:27: warning: incompatible pointer types passing 'EVP_MD_CTX **' (aka 'struct env_md_ctx_st **') to parameter of type 'EVP_MD_CTX *' (aka 'struct env_md_ctx_st *'); remove & [-Wincompatible-pointer-types]
      i = EVP_VerifyFinal(&dk->mdctx, md_value, md_len, publickey);
                          ^~~~~~~~~~
/usr/include/openssl/evp.h:654:33: note: passing argument to parameter 'ctx' here
int EVP_VerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sigbuf,
                                ^
domainkeys.c:2061:21: warning: incompatible pointer types passing 'EVP_MD_CTX **' (aka 'struct env_md_ctx_st **') to parameter of type 'EVP_MD_CTX *' (aka 'struct env_md_ctx_st *'); remove & [-Wincompatible-pointer-types]
      EVP_SignFinal(&dk->mdctx, sig, &siglen, pkey);
                    ^~~~~~~~~~

While the build succeeds these warnings should not be ignored and
break the runtime (tested via libdomainkeys test suite).
Comment 2 Nathan 2018-10-10 23:21:00 UTC
Created attachment 198020 [details]
Fix OpenSSL 1.1.x build

Tobik how's this patch. I got rid of all the EVP errors. What's left is errors that was there before patch
Comment 3 Tobias Kortkamp freebsd_committer 2018-10-11 05:13:54 UTC
Comment on attachment 198020 [details]
Fix OpenSSL 1.1.x build

It's a step in the right direction.

The test suite still does not run correctly.  I think the problem
is that mdctx is still uninitialized and never allocated any memory.
You're freeing it in dk_free(), but it's never allocated in dk_init()
(AFAICT this should happen with EVP_MD_CTX_create or EVP_MD_CTX_new).

Btw, here's how you can run the test suite:

1. Go to ${WRKSRC}
2. Compile dktest: cc -o dktest dktest.c -I. libdomainkeys.a -lcrypto
3. Run the test suite: ./test
Comment 4 Nathan 2018-10-11 05:15:12 UTC
(In reply to Tobias Kortkamp from comment #3)
Thanks for the reply. When I go on lunch I’ll fix it
Comment 5 Nathan 2018-10-12 04:24:07 UTC
(In reply to Tobias Kortkamp from comment #3)
Good news. I found a patch that works. Only error I get is no pub key available. I just got to adjust it where it works on base and devel version of OpenSSL
Comment 6 Nathan 2018-10-12 13:53:21 UTC
Created attachment 198062 [details]
OpenSSL patch

OK this one should be fine. I tested in openssl base and openssl 1.1 and both give the same error at the end

Also have to modify the test compile a bit:
cc -o dktest dktest.c -I. libdomainkeys.a -lssl -lcrypto
Comment 7 Nathan 2018-10-12 13:57:53 UTC
Created attachment 198064 [details]
OpenSSL patch

Forgot to svn add patch
Comment 8 Tobias Kortkamp freebsd_committer 2018-10-12 14:38:05 UTC
Comment on attachment 198064 [details]
OpenSSL patch

Where did you find the patch?  It has many bogus changes like:

+ static DK_STAT dkstore_char(DK *dk, char ch)
+ {
+-  if (dk->headerlen >= dk->headermax)
++  if (dk->headerlen < dk->headermax)
+   {

This seems like an unrelated change?

+@@ -924,18 +967,26 @@ static void dkhash(DK *dk, const unsigned char *ptr)
+   }
+   else
+   {
+-    while (dk->state >= 2)
++    while (dk->state < 2)

Hmm?

+-  if (snprintf(header_list,sizeof(header_list),":%s:",dk->headers) >= sizeof(header_list))
++  if (snprintf(header_list,sizeof(header_list),":%s:",dk->headers) < sizeof(header_list))
+   {
+     //header list is too large for buffer

Woah...  What's up with these extra changes?


+-  if (st >= (sizeof errors) / (sizeof errors[0]))
++  if (st < (sizeof errors) / (sizeof errors[0]))
+   {

Definitely wrong...
Comment 10 Tobias Kortkamp freebsd_committer 2018-10-12 18:30:00 UTC
(In reply to Nathan from comment #9)
Ok, but the patch from PLD Linux has none of the < to > flips etc.

Can you amend your patch and remove them?  I think it's ok otherwise.
Comment 11 Nathan 2018-10-12 18:33:49 UTC
(In reply to Tobias Kortkamp from comment #10)
Are you referring to the OpenSSL version part of the code?  If so I put them there to not break OpenSSL base
Comment 12 Tobias Kortkamp freebsd_committer 2018-10-12 18:38:06 UTC
(In reply to Nathan from comment #11)
No, I refer to the changes like the ones mentioned in comment #8.
Comment 13 Nathan 2018-10-12 21:41:17 UTC
Created attachment 198073 [details]
OpenSSL patch
Comment 14 Tobias Kortkamp freebsd_committer 2018-10-13 04:04:26 UTC
Created attachment 198085 [details]
libdomainkeys.diff v6

Thank you.

The patch was missing a few bits from the PLD Linux patch.  I've
added them back in.  I also removed the HAVE_ERR_REMOVE_STATE
changes.  They do not appear to be needed and neither OpenSSL 1.1.1
nor 1.0.2p nor the build itself define it.

The test suite now passes with OpenSSL 1.1.1 and 1.0.2p (on 12.0-ALPHA9
and 12.0-ALPHA6 respectively).
Comment 15 Tobias Kortkamp freebsd_committer 2018-11-05 21:08:43 UTC
Let's land this.
Comment 16 commit-hook freebsd_committer 2018-11-05 21:10:26 UTC
A commit references this bug:

Author: tobik
Date: Mon Nov  5 21:09:42 UTC 2018
New revision: 484222
URL: https://svnweb.freebsd.org/changeset/ports/484222

Log:
  mail/libdomainkeys: Unbreak with OpenSSL 1.1.1 [1]

  - Hook up the test suite
  - Stop installing the test script which is useless beyond port
    testing, and calls a dktest binary that is never installed
    or built normally
  - Cleanup/remove post-patch

  PR:		232133
  Submitted by:	Nathan <ndowens@yahoo.com> [1] (based on)
  Obtained from:	PLD Linux [1] (based on)

Changes:
  head/mail/libdomainkeys/Makefile
  head/mail/libdomainkeys/files/Makefile
  head/mail/libdomainkeys/files/patch-domainkeys.c
  head/mail/libdomainkeys/pkg-plist
Comment 17 Tobias Kortkamp freebsd_committer 2018-11-05 21:11:00 UTC
Thanks!