Builtin OpenSSL is outdated

[est31]

The version of the builtin openssl is 1.0.1h, which fortunately isn't affected by heartbleed anymore, but still outdated. The most recent version is 1.0.1p.

For the longer term, I think that it would be great if openssl could be updated to the most recent version for every stable release of godot.

[akien-mga]

Marking it as a bug as we are therefore likely exposed to all the security vulnerabilities announced since 1.0.1h. See e.g.:
http://advisories.mageia.org/MGASA-2015-0022.html
http://advisories.mageia.org/MGASA-2015-0111.html
http://advisories.mageia.org/MGASA-2015-0246.html
http://advisories.mageia.org/MGASA-2015-0274.html

Note that there is also a 1.0.2 branch now, with 1.0.2d being the latest release (July 2015).

Maintaining a built-in dependency such as OpenSSL will always be a pain though if we try to ensure that security vulnerabilities are patched in a timely manner (especially if we need to provide patch releases of past versions to address those issues).

It could be of course that many of those vulnerabilities are not exposed to Godot's API, but that's still bad press after the recent press exposure of things like heartbleed.

[reduz]

argh updating openssl and getting it to compile on every single platform is
such a giant pain in the ass

On Fri, Nov 13, 2015 at 8:35 AM, Rémi Verschelde notifications@github.com
wrote:

Marking it as a bug as we are therefore likely exposed to all the security
vulnerabilities announced since 1.0.1h. See e.g.:
http://advisories.mageia.org/MGASA-2015-0022.html
http://advisories.mageia.org/MGASA-2015-0111.html
http://advisories.mageia.org/MGASA-2015-0246.html
http://advisories.mageia.org/MGASA-2015-0274.html

Note that there is also a 1.0.2 branch now, with 1.0.2d being the latest
release (July 2015).

Maintaining a built-in dependency such as OpenSSL will always be a pain
though if we try to ensure that security vulnerabilities are patched in a
timely manner (especially if we need to provide patch releases of past
versions to address those issues).

It could be of course that many of those vulnerabilities are not exposed
to Godot's API, but that's still bad press after the recent press exposure
of things like heartbleed.

—
Reply to this email directly or view it on GitHub
#2780 (comment).

[reduz]

let's wait to see how progress go with mbedtls

[reduz]

well, seems progress with mbedtls stalled, and will not accept a PR using libcurl so I will have to tackle this myself in 2.1

[est31]

argh updating openssl and getting it to compile on every single platform is such a giant pain in the ass

You can try to get changes you do to the openssl source code merged upstream, that way you don't have to repeat them every time you update.

Also note that the Openssl 1.0.1 line will cease to be supported with security updates on Dec 31 this year, so best to switch to 1.0.2: https://www.openssl.org/policies/releasestrat.html

[slapin]

Please use git submodules against https://github.com/openssl/openssl
That will remove some of maintenance pain.

On Sat, Mar 5, 2016 at 7:10 AM, est31 notifications@github.com wrote:

argh updating openssl and getting it to compile on every single platform
is such a giant pain in the ass

You can try to get changes you do to the openssl source code merged
upstream, that way you don't have to repeat them every time you update.

Also note that the Openssl 1.0.1 line will cease to be supported with
security updates on Dec 31 this year, so best to switch to 1.0.2:
https://www.openssl.org/policies/releasestrat.html

—
Reply to this email directly or view it on GitHub
#2780 (comment).

[slapin]

But again, I think dynamic solution would be preferred for distros. The
static way should only be used for binaries on site.
Distro maintainers should have a way to build against their system openssl.
This allows forgetting about openssl update problems
for distros. I'd also be very happy having separate binaries for distros
(fedora + everythiing else), as fedora prefers to be very different from
others in more than one aspect.
Also I think that dlopen-based solution should be researched as possible
way to overcome library issues.

On Sat, Mar 5, 2016 at 7:29 AM, Sergey Lapin slapinid@gmail.com wrote:

Please use git submodules against https://github.com/openssl/openssl
That will remove some of maintenance pain.

On Sat, Mar 5, 2016 at 7:10 AM, est31 notifications@github.com wrote:

argh updating openssl and getting it to compile on every single platform
is such a giant pain in the ass

You can try to get changes you do to the openssl source code merged
upstream, that way you don't have to repeat them every time you update.

Also note that the Openssl 1.0.1 line will cease to be supported with
security updates on Dec 31 this year, so best to switch to 1.0.2:
https://www.openssl.org/policies/releasestrat.html

—
Reply to this email directly or view it on GitHub
#2780 (comment)
.

[akien-mga]

Distro maintainers should have a way to build against their system openssl. This allows forgetting about openssl update problems for distros.

That's the case already. If you don't specify openssl=builtin, it will use the system-wide version on Linux systems.

[xodene]

Is there any progress for this issue? I have a Godot game in Google Play and I was sent an email by Google Play that the OpenSSL version, which I assume Godot uses, is vulnerable to the Logjam attack.

Google Play has a deadline for APKs with this version of OpenSSL set for July 2016, therefore all current Godot APKs are affected by this issue. Is it possible to compile without OpenSSL as a temporary fix until OpenSSL is upgraded?

[akien-mga]

You can compile the templates with openssl=no as a workaround. But indeed, updating the builtin openssl version is high priority.

[akien-mga]

Apparently all Google Play Developers are getting a notification that their Godot games are vulnerable to logjam attacks and CVE-2015-3194. While this is likely not true per se (OpenSSL is vulnerable, but I don't think the very small usage we're making of it exposes the mentioned vulnerabilities, though I haven't checked myself), we need to fix this ASAP to silence those warnings.

[akien-mga]

I've had a very quick look at updating our builtin version to 1.0.1s (current version in the 1.0.1 branch that we are on right now), it does not seem to build out of the box as I get this error:

In file included from drivers/builtin_openssl2/crypto/armcap.c:8:0:
drivers/builtin_openssl2/crypto/arm_arch.h:35:6: error: #error "unsupported ARM architecture"
 #    error "unsupported ARM architecture"
      ^

I also seem to get conflicts between the system-wide headers and the ones in builtin_openssl2 when trying to build with openssl=builtin. So... needs more work :)

I'd be glad if someone feeling confident about it would give it a go ;)

[beocat]

Looks like the error comes from this file: [https://github.com/openssl/openssl/blob/OpenSSL_1_0_1-stable/crypto/arm_arch.h]
And happens because something (ARM_ARCH?) is'nt defined?

[reduz]

check the openssl config file (it's somewhere inside the openssl dir), this
one needs to be kept

On Fri, Apr 1, 2016 at 6:41 AM, beocat notifications@github.com wrote:

Looks like the error comes from this file: [
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1-stable/crypto/arm_arch.h
]
And happens because something (ARM_ARCH?) is'nt defined?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#2780 (comment)

[est31]

@akien-mga perhaps try to make a diff or something to the official release tarball the currently used openssl comes from, then you know which modifications happened, and you perhaps have a base on which to get it work again.

[akien-mga]

Yeah that's a good idea, I'll have a look at it over the week-end.

[akien-mga]

Sadly the git history is not really useful as the openssl customizations have often been done in huge commits together with other stuff, so it's difficult to update the current version in a way that Godot can work with out of the box.

Here's the history for whoever is interested:

• https://github.com/godotengine/godot/commits/master/drivers/builtin_openssl
• https://github.com/godotengine/godot/commits/master/drivers/builtin_openssl2

Basically the 1.0.1h version was imported, with the main header for each crypto component split from crypto/<component>/<component>.h to openssl/<component>.h. Makefiles were removed, so one would have to ignore them when diffing the two versions. Then some custom changes were made to the bundled ssl code, so they might need to be reapplied once the builtin version has been updated.

[akien-mga]

@reduz Is there a practical reason for moving those headers to openssl instead of leaving them at their upstream location? That makes maintenance much more bothersome.

[dresch86]

@reduz I haven't forgotten about the mbedTLS route. I actually have it working in a clunky fashion, but 2 other important projects sidetracked me from finishing. I finished one of those projects and the other is almost done.

[beocat]

@akien-mga Have you got an answer on why the the headers have been moved to openssl folder? I agree that it makes the maintenance harder. It would be nice to know what the thought behind this moving-of-files was.

[rofl0r]

yeah i agree that it would be much better using mbedtls (but not in-tree) or maintaining a patchset (for windows, since all linux distros (and probably mac) already ship openssl) against latest openssl release tarball.
so on windows, the build process could fetch the latest *ssl sources, patch them, and build them.
an option would be to use libressl instead of openssl, which reportedly builds for all platform without issues.

[rofl0r]

the "fix" is only temporal (until the next CVE) - so it does not really solve the issue.
additionally importing several KLOC of changes in a single commit could as well be a security issue - who can tell if one of the diff blobs is not actually a new backdoor (like this alerj78/lucky7coin#1 ) ?

[vnen]

Reopening this as it needs more discussion. @rofl0r made a good point. Also, it's not building on Windows, so that's one more problem.

[est31]

I don't know whether this is clear to everybody, but the commit adds lots of windows blobs. This might also be the reason for windows builds failing. They should be removed. Its just weird that this wasn't catched by the gitignore.

I've done a diff -r comparison from the drivers/builtin_openssl2 directory as of commit 925aa08 to an extracted download of the official release tarball: http://pastebin.com/jSiVX7Xq

There are no differing files, only files which exist in one directory, but not in the other.

The object files are obviously wrong, I don't know about the other stuff. But for example why add mips3.s to crypto/bn/asm?

[akien-mga]

What Windows blobs? Are you sure they're not from your own compilation of the windows template? I don't have any such blob:

[akien@cauldron godot.git (master)]$ find -name "*windows*o"
./drivers/windows/semaphore_windows.x11.tools.64.o
./drivers/windows/shell_windows.x11.tools.64.o
./drivers/windows/thread_windows.x11.tools.64.o
./drivers/windows/file_access_windows.x11.tools.64.o
./drivers/windows/dir_access_windows.x11.tools.64.o
./drivers/windows/mutex_windows.x11.tools.64.o

[akien-mga]

Thanks for checking that it's the same contents as the upstream tarball, I trust @mrezai's contributions but double-checking is never a bad idea, especially for sensitive drivers like openssl :)

[akien-mga]

But for example why add mips3.s to crypto/bn/asm?

No idea, but gitk shows that it was added by 6789480 and not modified further, so I guess it's no longer in OpenSSL 1.0.1s and @mrezai forgot to remove it.

[akien-mga]

Here's @est31's list of differences with the windows compiled files and Makefiles removed: http://pastebin.com/rYunpGmy

[est31]

Ah yeah, may be my own artifact.

[akien-mga]

OpenSSL 1.0.2h will be released on May 3rd: https://mta.openssl.org/pipermail/openssl-announce/2016-April/000069.html

@mrezai If you're around to handle the update in the following days, it would be awesome. Then I'll prepare a 2.0.3 release to, among other bug fixes, reenable openssl support.

[mrezai]

@akien-mga I'll update :)

[akien-mga]

Closing as fixed by #4275, #4329 and #4540.

The discussion on how to get rid of security-critical code in our source tree can continue in #4267.