Vanilla Textpattern 4.8.4 hiccup

gomedia · 2021-01-08 02:17:46

Bloke wrote #328067:

Out of curiosity …

Sorry, to dash your hopes but neither had any effect. Same failure in browser, no Diags panel, and nothing extra in error_log.

Would doing a curl_getinfo($ch); before the curl_exec() help – that way you could check some stuff in advance? I get this:

array (
  'url' => 'https://textpattern.com/version.json',
  'content_type' => NULL,
  'http_code' => 0,
  'header_size' => 0,
  'request_size' => 0,
  'filetime' => 0,
  'ssl_verify_result' => 0,
  'redirect_count' => 0,
  'total_time' => 0,
  'namelookup_time' => 0,
  'connect_time' => 0,
  'pretransfer_time' => 0,
  'size_upload' => 0,
  'size_download' => 0,
  'speed_download' => 0,
  'speed_upload' => 0,
  'download_content_length' => -1,
  'upload_content_length' => -1,
  'starttransfer_time' => 0,
  'redirect_time' => 0,
  'redirect_url' => '',
  'primary_ip' => '',
  'certinfo' => 
  array (
  ),
  'primary_port' => 0,
  'local_ip' => '',
  'local_port' => 0,
)

Hopefully, the values of ssl_verify_result or certinfo provide something useful.

Bloke · 2021-01-08 12:05:14

gomedia wrote #328083:

Hopefully, the values of ssl_verify_result or certinfo provide something useful.

Thanks for testing, and good call on that function. I’ll check those values on a working system and see if we can leverage the results so we can come to less of a screeching halt.

gaekwad · 2021-01-08 13:11:24

I think I’ve got it. The penny just dropped. Adi, I think you’ve explained something that’s been bugging me on and off for a few years. Thank you!

Looking at the PHP 5.6.21 + OpenSSL 0.9.8 stack:

(
    [0] => tcp
    [1] => udp
    [2] => unix
    [3] => udg
    [4] => ssl
    [5] => sslv3
    [6] => sslv2
    [7] => tls
    [8] => tlsv1.0
)

…that tops out at TLS v1.0, which textpattern.com isn’t supporting (and won’t, at least on my watch), so the connection to https://textpattern.com/version.json will fail. TLS v1.0 is long-since known as being insecure and has been deprecated, along with SSL v2 and v3 (also on the list above).

The PHP 7.3 instance will work because it uses TLS v1.2, which we do use. We currently run textpattern.com on TLS v1.2 and v1.3, and have a legacy set of ciphers to cover the vast majority of browsers & OpenSSL / curl instances without opening ourselves up to compromise with known-broken stuff.

More later, just looking into some stuff.

gaekwad · 2021-01-08 13:25:39

Worst case scenario, someone’s running the oldest Textpattern-supported PHP for whatever reason, currently PHP 5.5 on the Textpattern 4.8 branch. The PHP OpenSSL page says (emphasis mine):

In order to use the OpenSSL functions you need to install the OpenSSL library. PHP 5 requires at least OpenSSL >= 0.9.6. However later PHP 5 versions have some compilation issues and should be used at least with OpenSSL >= 0.9.8 which is also a minimal version for PHP 7.0. Other versions (PHP >= 7.1.0) require OpenSSL >= 1.0.1.

So, PHP 5 needs OpenSSL 0.9.8 (released 2005, support ended 2015) or later, PHP 7 needs OpenSSL 1.0.1 (released 2012, support ended 2016) or later. TLS v1.2 (the oldest we use that’s currently considered ‘safe’) was introduced in OpenSSL 1.0.1, so PHP running with OpenSSL 1.0.1 or an off-brand remix with TLS v1.2 support should work just fine.

I have no desire to run broken crypto, that will cause us problems – do we need to consider a fallback to GitHub if someone is using bad TLS? Or perhaps update system requirements to include “PHP with TLS v1.2 support (i.e. OpenSSL 1.0.1 or newer)”, sort of thing?

Bloke · 2021-01-08 13:35:59

I’d rather detect the broken crypto stack and bail out slightly more gracefully than we do now, if possible.

Adding the security requirements to the sys reqs page works for me.

If we can detect it, there’s also the possibility of introducing the check very early on – in something like janitor() where we check the PHP version and then (somehow) check that TLS is up-to-spec. We could die with a message that it won’t run right there before going any further.

Will that cause issues with localhost dev though? People who don’t develop in an https environment? If so, we can keep the crypto stack checks at the point of access – namely in our (forthcoming) “connect-to-some-external-stream” class. So each time we try and access either the .json version file, or some third-party stuff we rely on, such as theme or plugin update checks, we’ll verify at that point, and abort the call from within the class if we can’t do it for whatever reason.

The latter sounds safest. But I’m not sure this’ll be ready for 4.8.5 unless we delay a bit longer. If I can hack in the check on the Diagnostics panel as a testbed, we can at least test the waters and see if this is a viable solution for when we do it properly in a class (in 4.9.0).

Last edited by Bloke (2021-01-08 13:38:08)

gaekwad · 2021-01-08 14:02:25

Bloke wrote #328089:

I’d rather detect the broken crypto stack and bail out slightly more gracefully than we do now, if possible.

+1. I was curious what raw.githubusercontent.com runs in its TLS stack, they currently support TLS v1.0 and v1.1:

It’s a trivial thing to maintain an identical version.json on GitHub, but there are privacy / access considerations for people who are anti-GitHub, of course.

Adding the security requirements to the sys reqs page works for me.

I’ll draft something for review.

If we can detect it, there’s also the possibility of introducing the check very early on – in something like janitor() where we check the PHP version and then (somehow) check that TLS is up-to-spec. We could die with a message that it won’t run right there before going any further.

Will that cause issues with localhost dev though? People who don’t develop in an https environment?

As I understand it, OPENSSL_VERSION_NUMBER is a PHP constant with the version string, would that work? A non-dramatic failure message would be sufficient.

But I’m not sure this’ll be ready for 4.8.5 unless we delay a bit longer. If I can hack in the check on the Diagnostics panel as a testbed, we can at least test the waters and see if this is a viable solution for when we do it properly in a class (in 4.9.0).

+1.

Bloke · 2021-01-08 14:39:39

We could flip-flop to GitHub as fallback if we can get the file_get_contents() to die gracefully enough that we can catch it and failover.

Thing is, there are a few things at play here:

Adi’s server wouldn’t connect via curl_exec() so we need some way to check this code pathway is going to work, without it exploding in a shower of 1s and 0s.
Adi’s server then chokes if file_get_contents() is used too, so even if we manage to detect that cURL will fail, falling back on file_get_contents() will also die in this case. So we need to verify this pathway too (in case curl isn’t installed or it doesn’t meet the crypto requirements).
Relying on the OPENSSL_VERSION_NUMBER constant looks like it won’t work, since Adi’s server reports “OpenSSL 0.9.8z” which meets the minimum spec (if I’ve read your post correctly, Pete).
curl_version() doesn’t return a reliable number. Parsing out the version number from the text isn’t my idea of futureproof fun. Viz…

My Mac, with curl compiled into PHP:

array (
  'ssl_version_number' => 0,
  'ssl_version' => 'OpenSSL/1.0.2o',
...
)

OPENSSL_VERSION_NUMBER: 268443903
OPENSSL_VERSION_TEXT: OpenSSL 1.0.2o 27 Mar 2018

My server without curl compiled in:

OPENSSL_VERSION_NUMBER: 269488255
OPENSSL_VERSION_TEXT: OpenSSL 1.1.1g 21 Apr 2020

So, not sure what to make of all that.

Ideas?

Last edited by Bloke (2021-01-08 14:55:06)

Bloke · 2021-01-08 14:48:04

P.S. curl_getinfo($ch); on my localhost (Mac) returns:

array (
  'url' => 'https://textpattern.com/version.json',
  'content_type' => NULL,
  'http_code' => 0,
  'header_size' => 0,
  'request_size' => 0,
  'filetime' => -1,
  'ssl_verify_result' => 0,
  'redirect_count' => 0,
  'total_time' => 0.0,
  'namelookup_time' => 0.0,
  'connect_time' => 0.0,
  'pretransfer_time' => 0.0,
  'size_upload' => 0.0,
  'size_download' => 0.0,
  'speed_download' => 0.0,
  'speed_upload' => 0.0,
  'download_content_length' => -1.0,
  'upload_content_length' => -1.0,
  'starttransfer_time' => 0.0,
  'redirect_time' => 0.0,
  'redirect_url' => '',
  'primary_ip' => '',
  'certinfo' => 
  array (
  ),
  'primary_port' => 0,
  'local_ip' => '',
  'local_port' => 0,
)

Can’t see anything of use there. Any other threads we can clutch at?

Last edited by Bloke (2021-01-08 14:55:33)

Bloke · 2021-01-08 14:52:03

gaekwad wrote #328086:

I think I’ve got it. Looking at the PHP 5.6.21 + OpenSSL 0.9.8 stack: [8] => tlsv1.0

Missed this post, sorry. I wonder if we can check that stream_get_transports() has a tls value in the array that we support and bail if not. That might be simplest all round, if it’s reliable.

EDIT: This is what I see on my server:

array (
  0 => 'tcp',
  1 => 'udp',
  2 => 'unix',
  3 => 'udg',
  4 => 'ssl',
  5 => 'tls',
  6 => 'tlsv1.0',
  7 => 'tlsv1.1',
  8 => 'tlsv1.2',
)

and localhost (Mac):

array (
  0 => 'tcp',
  1 => 'udp',
  2 => 'unix',
  3 => 'udg',
  4 => 'ssl',
  5 => 'sslv3',
  6 => 'tls',
  7 => 'tlsv1.0',
  8 => 'tlsv1.1',
  9 => 'tlsv1.2',
)

So it does seem as if detecting whether tlsv1.2 is in the array might be a way forward. If not, just give up and say ‘version check not available’ and move on.

There’s the issue of keeping this up to date as things march onward, but if we put a minimum version in the constants then it’s a prompt that we need to attend to it, like we do with the PHP version.

The only issue I can foresee is that, for now, they list all available versions in this array. In future, they might decide to drop older entries so we can’t do a simple “is our version in the list” but also need to check “is our supported version less than the highest one in the list” which complicates things a tad.

Not sure what happens in PHP 8.0.0 either.

Last edited by Bloke (2021-01-08 15:04:15)

gaekwad · 2021-01-08 15:07:42

Bloke wrote #328092:

OPENSSL_VERSION_NUMBER: 268443903
OPENSSL_VERSION_TEXT: OpenSSL 1.0.2o 27 Mar 2018

So, not sure what to make of all that.

Ideas?

Selectively quoting, juggling a few bits: 268443903 is decimal converted from its internal version string: 0x100020ff. Each OpenSSL revision release in a given branch is greater than the last one, so a < or > check should be straightforward.

Ninja edit: wiki.openssl.org/index.php/Versioning explains it far more succinctly than I could!

Last edited by gaekwad (2021-01-08 15:16:27)

gaekwad · 2021-01-08 15:12:27

gaekwad wrote #328087:

TLS v1.2 (the oldest we use that’s currently considered ‘safe’) was introduced in OpenSSL 1.0.1, so PHP running with OpenSSL 1.0.1 or an off-brand remix with TLS v1.2 support should work just fine.

Check OPENSSL_VERSION_NUMBER for a value equal to or greater than 268439567, which is the decimal flavour of 0x1000100f, which is the oldest non-beta release of OpenSSL 1.0.1.

Bloke · 2021-01-08 15:19:48

gaekwad wrote #328097:

Check OPENSSL_VERSION_NUMBER for a value equal to or greater than 268439567

Okay. Watch the 4.8.5 repo for changes.

Bloke · 2021-01-08 16:03:11

Okay, go test please.

You should just get the standard ‘cannot connect’ if the SSL version doesn’t match our minimum (constant).

Note that you’ll need to comment out the check for $is_dev in txp_diag.php if you want to test this, since the update checks aren’t run for dev environments yet. When we get the infrastructure in place in the version.json file to include the bleeding edge dev version/branch (somehow), we might add this feature.

Oh, and delete the last_update_check pref from the database too, unless you’re ultra patient and want to wait an hour for it to timeout.

Last edited by Bloke (2021-01-08 16:07:25)

phiw13 · 2021-01-09 05:34:46

FWIW, on macOS 11.1: PHP SSL version: LibreSSL 2.8.3 – the Diagnostics panel loads just fine.

(patched TXP-dev install on localhost)

Note that you’ll need to comment out the check for $is_dev in txp_diag.php

Not sure what I need to comment out for further testing ? I can only find instances of a $txp_is_dev in that file (5 in total).

Bloke · 2021-01-09 09:43:22

Sorry, yeah, $txp_is_dev. There’s a chunk of code near the top that does the checkUpdate so just comment out or remove line 196 and its corresponding brace on line 218 to enable the update check. And remove the last_update_check pref if it’s in your txp_prefs table as you only get one shot per hour otherwise.

Textpattern CMS

Textpattern CMS support forum

#16 2021-01-08 02:17:46

Re: Vanilla Textpattern 4.8.4 hiccup

Bloke wrote #328067:

#17 2021-01-08 12:05:14

Re: Vanilla Textpattern 4.8.4 hiccup

gomedia wrote #328083:

#18 2021-01-08 13:11:24

Re: Vanilla Textpattern 4.8.4 hiccup

#19 2021-01-08 13:25:39

Re: Vanilla Textpattern 4.8.4 hiccup

#20 2021-01-08 13:35:59

Re: Vanilla Textpattern 4.8.4 hiccup

#21 2021-01-08 14:02:25

Re: Vanilla Textpattern 4.8.4 hiccup

Bloke wrote #328089:

#22 2021-01-08 14:39:39

Re: Vanilla Textpattern 4.8.4 hiccup

#23 2021-01-08 14:48:04

Re: Vanilla Textpattern 4.8.4 hiccup

#24 2021-01-08 14:52:03

Re: Vanilla Textpattern 4.8.4 hiccup

gaekwad wrote #328086:

#25 2021-01-08 15:07:42

Re: Vanilla Textpattern 4.8.4 hiccup

Bloke wrote #328092:

#26 2021-01-08 15:12:27

Re: Vanilla Textpattern 4.8.4 hiccup

gaekwad wrote #328087:

#27 2021-01-08 15:19:48

Re: Vanilla Textpattern 4.8.4 hiccup

gaekwad wrote #328097:

#28 2021-01-08 16:03:11

Re: Vanilla Textpattern 4.8.4 hiccup

#29 2021-01-09 05:34:46

Re: Vanilla Textpattern 4.8.4 hiccup

#30 2021-01-09 09:43:22

Re: Vanilla Textpattern 4.8.4 hiccup

Board footer