openssl @1.0.1k breaks certificate signature verification

[mouse07410 (Mouse)]

Current openssl-1.0.1k_0 port breaks certificate signature verification:

$ openssl version
OpenSSL 1.0.1k 8 Jan 2015
$ openssl verify -CAfile RabbitMQ_Test_CA.pem -verbose RabbitMQ_Test.pem
RabbitMQ_Test.pem: CN = RabbitMQ_Test, C = US
error 7 at 0 depth lookup:certificate signature failure
$ /usr/bin/openssl version
OpenSSL 0.9.8za 5 Jun 2014
$ /usr/bin/openssl verify -CAfile RabbitMQ_Test_CA.pem -verbose RabbitMQ_Test.pem
RabbitMQ_Test.pem: OK
$ 

Needless to say, certificates in question are correct and work fine elsewhere. They also validate just fine on the same machine/platform (Mac OS X 10.9.5 and Xcode-6.1.1) using native openssl port, as shown above. One certificate that demonstrates this problem is attached, together with its CA cert.

This is a critical problem.

[mouse07410 (Mouse)]

Certificate that demonstrates the problem

[mouse07410 (Mouse)]

CA used to create and verify this certificate

[mouse07410 (Mouse)]

Interestingly enough, the CA cert itself validates fine:

$ openssl verify -CAfile RabbitMQ_Test_CA.pem -verbose RabbitMQ_Test_CA.pem
RabbitMQ_Test_CA.pem: OK
$ 

[mouse07410 (Mouse)]

Patch that provides a reasonably secure workaround for this problem

[mouse07410 (Mouse)]

Problem has been tracked to ASN.1 type comparison in "crypto/asn1/a_type.c". It is triggered from 1.0.1k version by the following patch:

diff --git a/crypto/x509/x_all.c b/crypto/x509/x_all.c
index e06602d..fef55f8 100644 (file)
--- a/crypto/x509/x_all.c
+++ b/crypto/x509/x_all.c
@@ -72,6 +72,8 @@
 
 int X509_verify(X509 *a, EVP_PKEY *r)
        {
+       if (X509_ALGOR_cmp(a->sig_alg, a->cert_info->signature))
+               return 0;
        return(ASN1_item_verify(ASN1_ITEM_rptr(X509_CINF),a->sig_alg,
                a->signature,a->cert_info,r));
        }

The above patch invokes "X509_ALGOR_cmp" in "crypto/asn1/x_algor.c", which in turn invokes "ASN1_TYPE_cmp" in "crypto/asn1/a_type.c", where the comparison fails because one parameter is explicit ASN.1 NULL (0x05), while the other one is empty (not present, null-pointer). So ASN.1 NULL fails to compare to NULL, and the entire comparison fails with "not equal".

I think the cause of this problem is somewhere in the certificate decoding piece, where either an explicit ASN.1 NULL gets added when it shouldn't, or the code forgets to add it when it should. Correction: after checking the actual certificate ASN.1 encoding, I found that indeed in all places but one, signature algorithm parameters part is encoded as ASN.1 NULL; and in one place the parameters part is just not present (thus decoded as null pointer). I still maintain that the two are equal. :)

The following is a reasonable secure workaround, until somebody figures why certificate parsing is screwed up:

$ diff -uw crypto/asn1/a_type.c.~1~ crypto/asn1/a_type.c
--- crypto/asn1/a_type.c.~1~	2015-01-15 09:43:14.000000000 -0500
+++ crypto/asn1/a_type.c	2015-01-17 15:12:17.000000000 -0500
@@ -117,7 +117,22 @@
 	{
 	int result = -1;
 
-	if (!a || !b || a->type != b->type) return -1;
+	if (!a || !b) {
+	  if (!a && !b) /* both types are empty (null) */
+	    return 0;
+	  /* one is null, the other is maybe ASN.1 NULL (explicit) */
+	  if (a && !b) { 
+	    if (a->type == V_ASN1_NULL)
+	      return 0;
+	  } 
+	  if (b && !a) {
+	    if (b->type == V_ASN1_NULL)
+	      return 0;
+	  }
+	  return -1; /* the non-null (present) type isn't ASN.1 NULL */
+	}
+
+	if (a->type != b->type) return -1;
 
 	switch (a->type)
 		{

I've also attached it as a patch.

For what it's worth - the same problem (and I suspect caused by the same misunderstanding leading to the same bug) is present in the latest beta (development) versions of Botan library ​https://botan.randombit.net, starting with 1.11.12.

[mouse07410 (Mouse)]

Running a wonderful program "dumpasn1", I found that indeed most algorithms are encoded with an explicit ASN.1 NULL for parameters, but the first occurrence is encoded with no parameters at all.

Therefore my workaround probably is a real patch for this issue.

P.S. Surprisingly, this issue is not triggered on the CA cert that exhibits the same disparity in signature algorithm parameters encoding.

[mouse07410 (Mouse)]

Dump of ASN.1 encoding of a cert, illustrating problem: look at data at offset 20, and compare it with offsets 223 and 628.

[neverpanic (Clemens Lang)]

As the reporter you are automatically Cc'd, so you don't need to Cc yourself.

This isn't a problem with MacPorts' build of OpenSSL, but OpenSSL itself. It is being tracked by the OpenSSL developers in ​http://rt.openssl.org/Ticket/Display.html?id=3665&user=guest&pass=guest. I don't feel comfortable applying a patch that changes certificate verification without analysis and approval of the OpenSSL developers.

We will patch this when upstream commits or releases a new version with a fix.

[mouse07410 (Mouse)]

Yeah, that's fair.

In the meanwhile, is there a way for me to package my patch in such a way that my Macports would pick it and apply to my build, while we all are waiting for the OpenSSL developers to weigh in? Because regardless of what & when they decide, I need this patch for as long as I'm using Mac OS X. (And I've dealt enough with cryptography to know that my patch is safe, so my computers will run it, even if others won't.)

[neverpanic (Clemens Lang)]

Sure, you can put the file in $(port dir openssl)/files and add its filename to the patchfiles directive (creating it if it doesn't exist) in $(port file openssl). Then, re-install openssl using sudo port -fsn upgrade openssl.

[mouse07410 (Mouse)]

Thank you! Did the steps you mentioned, and got openssl-1.0.1k re-installed with my patch. A few things:

• Since I have GMP port installed, I decided to add GMP dependency to my openssl. So I added "port:gmp" to "depends_lib". However the build did not seem to pick the gmp library ("port upgrade" would fail) until I explicitly added "-lgmp" to "configure.args". That sounds wrong?
• The exact command that worked for me was sudo port -n upgrade -s --force openssl, and after that it had problems activating openssl (my fault, I think), so I had to activate it by force sudo port -f activate openssl. After that everything seemed fine.

In the meanwhile, OpenSSL developers are discussing the best course of action. :-)

[neverpanic (Clemens Lang)]

Replying to uri@…:

Since I have GMP port installed, I decided to add GMP dependency to my openssl. So I added "port:gmp" to "depends_lib". However the build did not seem to pick the gmp library ("port upgrade" would fail) until I explicitly added "-lgmp" to "configure.args". That sounds wrong?

No, that sounds exactly right. If adding the dependency was all it took to make openssl link against gmp, all users who built from source and have gmp installed would end up with openssl linked against it, but the dependency relation was not necessary picked up by MacPorts. This would lead to incorrect dependency information, breakage when later uninstalling gmp and non-reproducible builds (because they depend on your selection of ports), which we try to avoid at all cost.

[mouse07410 (Mouse)]

I've modified openssl Portfile to add more variants (gmp, JPAKE, etc).

Would it be possible to incorporate those additions into the mainstream? I'd love it.

Thanks!

[mouse07410 (Mouse)]

Also, a strange (for me :) thing: after that successful upgrade of openssl (in a few hours), I did

sudo port selfupdate; sudo port upgrade outdated

which also succeeded. But when I looked at "$(port file openssl)" I see that it reverted to the original version! And my patch file that I put in "$(port dir openssl)" is also gone. Did I do something wrong? Is there a way to preserve my modification to Portfile, and my additions to "$(port dir openssl)"?

P.S. I still hope you'd accept my enhanced Portfile with variants. ;)

[mouse07410 (Mouse)]

Updated patch for signature algorithm mismatch and ASN.1 type comparison bug.

[mouse07410 (Mouse)]

Oh, and based upon discussion here ​http://rt.openssl.org/Ticket/Display.html?id=3665#txn-50911 I've changed the patch. Steve Henson correctly pointed out that to change ASN1_TYPE_cmp() may not be appropriate, as there could be cases when null pointer (absent list) means something different from list being NULL.

To address that comment, I've made sure the workaround applies only to the case when two algorithms are compared, based on RFC 5754 and 5280 that state that AlgorithmIdentifier parameter list can be absent or represented as ASN.1 NULL - and implementations MUST accept both cases.

So please find attached my updated patch "patch-null-absent.diff". I'm also posting it upstream.

Thanks!

[neverpanic (Clemens Lang)]

If you are using rsync to sync your ports tree (which is the default) your changes will be reverted on selfupdate. Workarounds are

• Using SVN wiki:howto/SyncingWithSVN
• Using a local ports tree with a copy of OpenSSL that shadows ours: ​http://guide.macports.org/#development.local-repositories

Whether we should add more variants to openssl is essentially the maintainer's decision, but I wouldn't be opposed to it unless these variants somehow break API or ABI.

As with your previous patch, I'm hesitant to pull it into MacPorts' OpenSSL without upstream approval.

[mouse07410 (Mouse)]

Replying to cal@…:

If you are using rsync to sync your ports tree (which is the default) your changes will be reverted on selfupdate.

:-) I was certain that I was doing it to myself!

Workarounds are

• Using SVN wiki:howto/SyncingWithSVN
• Using a local ports tree with a copy of OpenSSL that shadows ours: ​http://guide.macports.org/#development.local-repositories

I thought that the best way is to add a local port tree, as shown in the URL you kindly provided.

This is what I've created:

$ ls -FR ~/ports
PortIndex		PortIndex.quick		devel/

/Users/ur20980/ports/devel:
openssl/

/Users/ur20980/ports/devel/openssl:
Portfile	files/

/Users/ur20980/ports/devel/openssl/files:
patch-null-absent.diff

I've created index with "portindex", like the Web page told. However when I try to do "sudo port selfupdate", I'm getting this:

$ sudo port selfupdate
Password:
--->  Updating MacPorts base sources using rsync
MacPorts base version 2.3.3 installed,
MacPorts base version 2.3.3 downloaded.
--->  Updating the ports tree
Error: updating PortIndex for file://Users/ur20980/ports failed
--->  MacPorts base is already the latest version

The ports tree has been updated. To upgrade your installed ports, you should run
  port upgrade outdated

Whether we should add more variants to openssl is essentially the maintainer's decision, but I wouldn't be opposed to it unless these variants somehow break API or ABI.

I'm pretty sure they don't break anything, because they just apply certain OpenSSL configuration options (and they don't seem to interfere with anything on my machine :).

Perhaps you could point me at a person that I should ask about this? Is it mww@…?

As with your previous patch, I'm hesitant to pull it into MacPorts' OpenSSL without upstream approval.

Yes, I understand and appreciate your position.

But they surely do take their time, especially considering the obviousness of the issue (there was also a bug in ASN.1 type comparison function - a one-liner that I fixed along the way :).

[larryv (Lawrence Velázquez)]

This ticket has gone almost completely off-topic. Please use macports-dev to continue discussing local ports trees or new openssl variants (which I am opposed to adding, without more convincing reasons).

[neverpanic (Clemens Lang)]

Replying to uri@…:

I've created index with "portindex", like the Web page told. However when I try to do "sudo port selfupdate", I'm getting this:

$ sudo port selfupdate
Password:
--->  Updating MacPorts base sources using rsync
MacPorts base version 2.3.3 installed,
MacPorts base version 2.3.3 downloaded.
--->  Updating the ports tree
Error: updating PortIndex for file://Users/ur20980/ports failed
--->  MacPorts base is already the latest version

The ports tree has been updated. To upgrade your installed ports, you should run
  port upgrade outdated

Edit your sources.conf and add [nosync] to the line adding your local repository. If it is also marked as [default] separate the options using a comma: [default,nosync].

Perhaps you could point me at a person that I should ask about this? Is it mww@…?

Yes. He should already get an email for each comment in this ticket, but he hasn't been very active recently, which is why I've taken care of all pressing issues with openssl in the last few months. Nevertheless, go ahead and email him, or open a new ticket with a patch and assign it to him/put him on cc.

But they surely do take their time, especially considering the obviousness of the issue (there was also a bug in ASN.1 type comparison function - a one-liner that I fixed along the way :).

Yeah, I know. Nonetheless, I'd like to avoid patching security-relevant stuff, even if the issue is obvious. We're trying to avoid replicating some of the disasters Debian created when patching OpenSSL ;-)

[mouse07410 (Mouse)]

Replying to cal@…:

Error: updating PortIndex for file://Users/ur20980/ports failed

Edit your sources.conf and add [nosync] to the line adding your local repository. If it is also marked as [default] separate the options using a comma: [default,nosync].

Thank you - adding [nosync] and a missing 3rd slash fixed the day!

Perhaps you could point me at a person that I should ask about this? Is it mww@…?

Yes. He should already get an email for each comment in this ticket, but he hasn't been very active recently, which is why I've taken care of all pressing issues with openssl in the last few months. Nevertheless, go ahead and email him, or open a new ticket with a patch and assign it to him/put him on cc.

Great! I'm polishing the Portfile now. Hitting a strange obstacle:

--->  Updating database of binaries
--->  Scanning binaries for linking errors
--->  Found 62 broken file(s), matching files to ports   
--->  Found 3 broken port(s), determining rebuild order
--->  Rebuilding in order
     openssl @1.0.1k +universal
     curl @7.40.0 +ssl+universal
     gnome-vfs @2.24.4 +universal
--->  Computing dependencies for openssl
--->  Cleaning openssl
--->  Computing dependencies for curl
--->  Cleaning curl
--->  Computing dependencies for gnome-vfs
--->  Cleaning gnome-vfs
--->  Scanning binaries for linking errors
--->  Found 62 broken file(s), matching files to ports
--->  Found 3 broken port(s), determining rebuild order
--->  Rebuilding in order
     openssl @1.0.1k +universal
     curl @7.40.0 +ssl+universal
     gnome-vfs @2.24.4 +universal
--->  Computing dependencies for openssl
--->  Fetching distfiles for openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://ykf.ca.distfiles.macports.org/MacPorts/mpdistfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://distfiles.macports.org/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://mse.uk.distfiles.macports.org/sites/distfiles.macports.org/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://nue.de.distfiles.macports.org/macports/distfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://fco.it.distfiles.macports.org/mirrors/macports-distfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://sea.us.distfiles.macports.org/macports/distfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://lil.fr.distfiles.macports.org/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://www.openssl.org/source/
--->  Attempting to fetch patch-Makefile.org.diff from http://her.gr.distfiles.macports.org/mirrors/macports/mpdistfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://cjj.kr.distfiles.macports.org/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://aarnet.au.distfiles.macports.org/pub/macports/mpdistfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://jog.id.distfiles.macports.org/macports/mpdistfiles/openssl
--->  Attempting to fetch patch-Makefile.org.diff from http://svn.macports.org/repository/macports/distfiles/openssl
Error: org.macports.fetch for port openssl returned: fetch failed
Please see the log file for port openssl for details:
    /opt/local/var/macports/logs/_Users_uri_ports_devel_openssl/openssl/main.log
Error: Unable to upgrade port: 1
Error rebuilding openssl
    while executing
"error "Error rebuilding $portname""
    (procedure "revupgrade_scanandrebuild" line 395)
    invoked from within
"revupgrade_scanandrebuild broken_port_counts $opts"
    (procedure "macports::revupgrade" line 5)
    invoked from within
"macports::revupgrade $opts"
    (procedure "action_revupgrade" line 2)
    invoked from within
"action_revupgrade $action $portlist $opts"
    (procedure "action_upgrade" line 25)
    invoked from within
"$action_proc $action $portlist [array get global_options]"
    (procedure "process_cmd" line 103)
    invoked from within
"process_cmd $remaining_args"
    invoked from within
"if { [llength $remaining_args] > 0 } {

    # If there are remaining arguments, process those as a command
    set exit_status [process_cmd $remaining..."
    (file "/opt/local/bin/port" line 5268)
$ sudo port upgrade outdated
Nothing to upgrade.

It looks like "port" cannot fetch openssl patch files? I did not copy to my local ports directory anything, except for my (modified) Portile, and my additional patch:

$ ls -RF ~/ports
PortIndex		PortIndex.quick		devel/

/Users/uri/ports/devel:
openssl/

/Users/uri/ports/devel/openssl:
Portfile	files/

/Users/uri/ports/devel/openssl/files:
patch-null-absent.diff
$ 

Am I doing something wrong???

But they surely do take their time, especially considering the obviousness of the issue (there was also a bug in ASN.1 type comparison function - a one-liner that I fixed along the way :).

Yeah, I know. Nonetheless, I'd like to avoid patching security-relevant stuff, even if the issue is obvious. We're trying to avoid replicating some of the disasters Debian created when patching OpenSSL ;-)

:-) :-) I'm with you here. Wouldn't want my name associated with such a "sensation" either. :-)

[neverpanic (Clemens Lang)]

Replying to uri@…:

Great! I'm polishing the Portfile now. Hitting a strange obstacle:

Please open a new ticket about this and discuss it there.

I did not copy to my local ports directory anything, except for my (modified) Portile, and my additional patch:

You need to copy the original files/ directory as well.

[mouse07410 (Mouse)]

Replying to cal@…:

Replying to uri@…:

Great! I'm polishing the Portfile now. Hitting a strange obstacle:

Please open a new ticket about this and discuss it there.

As a matter of fact, unnecessary - your answer below solved it.

I did not copy to my local ports directory anything, except for my (modified) Portile, and my additional patch:

You need to copy the original files/ directory as well.

I will be opening a new ticket with Portfile patch/update. I guess I'll label it as feature request?

[neverpanic (Clemens Lang)]

Upstream rejected the patch, so we aren't going to carry it against their decision.