GNOME Bugzilla – Bug 795782
Use openssl API to get the default cert path
Last modified: 2018-05-22 12:05:59 UTC
Created attachment 371646 [details] [review] get cert path from openssl As suggested here: https://github.com/Alexpux/MINGW-packages/pull/3487#issuecomment-386324661 The idea being that if openssl is set up properly glib-openssl will work without any configuration. But if the user is only using glib this might be a regression, I don't know.
Review of attachment 371646 [details] [review]: Is this supposed to work with relocations? ::: tls/openssl/gtlsbackend-openssl.c @@ +190,2 @@ gchar *cert_path; + const char *cert_file; use gchar @@ +193,3 @@ + cert_file = g_getenv (X509_get_default_cert_file_env ()); + + if (!cert_file) let's use == NULL
(In reply to Ignacio Casal Quinteiro (nacho) from comment #1) > Review of attachment 371646 [details] [review] [review]: > > Is this supposed to work with relocations? MSYS2 has patched openssl to be relocatable. In case openssl isn't you'd lose it compared to the current code and you'd have to set the SSL_CERT_FILE env var.
can we try the following? if glib-openssl env var GLIB_OPENSSL_HANDLE_CERT_RELOCATABLE use this else fallback to the approach you took
Created attachment 372285 [details] [review] tls: fall back to the default openssl ca file if none is specified otherwise openssl provides X509_get_default_cert_file_env() which gives the env var to use for configuring the ca file path at runtime and X509_get_default_cert_file() which gives a default path. Assuming openssl is properly configured this makes glib-openssl work without setting any path. One remaining problem on Windows is that while under MSYS2 openssl is patched to be relocatable this is not the case for all Windows openssl users. For that introduce a GLIB_OPENSSL_HANDLE_CERT_RELOCATABLE env var which when set uses a hardcoded relative path, as was the default before.
btw, master currently doesn't build. typos in the meson file, and the OCSP stuff fails to build in current MSYS2.
Review of attachment 372285 [details] [review]: Nitpick to fix and feel free to push. ::: tls/openssl/gtlsbackend-openssl.c @@ +189,2 @@ #ifdef G_OS_WIN32 + if (g_getenv ("GLIB_OPENSSL_HANDLE_CERT_RELOCATABLE") != NULL) for consistency with other env vars use G_TLS_OPENSSL...
(In reply to Christoph Reiter (lazka) from comment #5) > btw, master currently doesn't build. typos in the meson file, and the OCSP > stuff fails to build in current MSYS2. About this, I do not have a newly enough openssl handy. Mind fixing the issues?
Created attachment 372312 [details] [review] meson: fix the build
(In reply to Ignacio Casal Quinteiro (nacho) from comment #7) > (In reply to Christoph Reiter (lazka) from comment #5) > > btw, master currently doesn't build. typos in the meson file, and the OCSP > > stuff fails to build in current MSYS2. > > About this, I do not have a newly enough openssl handy. Mind fixing the > issues? Turns out this is due to some define conflicts between winsock.h and openssl. MSYS2 already has some patches for this so my guess is I just have to do something similar for the ocsp header there. And according to https://github.com/openssl/openssl/issues/1157#issuecomment-231330913 this is solved with 1.1, but msys2 is still at 1.0.
Created attachment 372316 [details] [review] tls: Fix the build with openssl 1.0 on Windows Due to name clashes between the Windows headers and openssl headers the include order matters. In addition the 1.0 headers include the Windows headers themselves which makes things more complicated. According to https://github.com/openssl/openssl/issues/1157#issuecomment-231330913 this is fixed in 1.1 by not including the Windows headers in the openssl headers and requiring the user to include the Windows headers first (but I haven't tested this) To avoid future breakage gather all openssl includes in one file, include the Windows headers first and undef all the clashing macros.
Review of attachment 372312 [details] [review]: Looks good
Review of attachment 372316 [details] [review]: I am missing the meson.build change. Can you change it and push it?
Thanks. Things work again under MSYS2 (sadly MSYS2 has patched openssl to make it relocatable based on the executable and not the ssl lib, so "meson test" fails unless you set SSL_CERT_FILE) On Debian openssl's DEFAULT_CERT_FILE isn't set correctly sadly and only DEFAULT_CERT_DIR points to something valid. Not sure how glib-openssl could use that. So this all doesn't improve things as much as I had hoped, but maybe it did a bit at least :)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib-openssl/issues/3.