Core: SFTP + keypair mode doesn't work any more on stable10

[PVince81]

Steps

1. Setup stable10
2. Setup a system-wide SFTP mount with "RSA public key" mode
3. Copy the given key into "~/ssh/.authorized_keys"
4. Type in the user name and other stuff, wait for refresh

Expected

Green light

Actual

Red.

Version

stable10 1f0a3b077bdeba38ec5231b926cdb281e26b1f06

Log is full of:

{"reqId":"nGG4Y8OWrYTttMWrtzN1","level":3,"time":"2017-08-14T12:28:32+00:00","remoteAddr":"127.0.0.1","user":"admin","app":"PHP","method":"GET","url":"\/owncloudee\/ocs\/v2.php\/apps\/notifications\/api\/v1\/notifications?format=json","message":"pack(): Type H: illegal hex digit - at \/srv\/www\/htdocs\/owncloudee\/lib\/composer\/phpseclib\/phpseclib\/phpseclib\/Crypt\/RSA.php#1081"}

[PVince81]

0  phpseclib\Crypt\RSA->_parseKey() /srv/www/htdocs/owncloud/lib/composer/phpseclib/phpseclib/phpseclib/Crypt/RSA.php:1081
1  phpseclib\Crypt\RSA->loadKey() /srv/www/htdocs/owncloud/lib/composer/phpseclib/phpseclib/phpseclib/Crypt/RSA.php:1565
2  OCA\Files_External\Lib\Auth\PublicKey\RSA->manipulateStorageConfig() /srv/www/htdocs/owncloud/apps/files_external/lib/Lib/Auth/PublicKey/RSA.php:63
3  OCA\Files_External\Controller\GlobalStoragesController->manipulateStorageConfig() /srv/www/htdocs/owncloud/apps/files_external/lib/Controller/StoragesController.php:228
4  OCA\Files_External\Controller\GlobalStoragesController->updateStorageStatus() /srv/www/htdocs/owncloud/apps/files_external/lib/Controller/StoragesController.php:245
5  OCA\Files_External\Controller\GlobalStoragesController->update() /srv/www/htdocs/owncloud/apps/files_external/lib/Controller/GlobalStoragesController.php:187
6  call_user_func_array:{/srv/www/htdocs/owncloud/lib/private/AppFramework/Http/Dispatcher.php:159}() /srv/www/htdocs/owncloud/lib/private/AppFramework/Http/Dispatcher.php:159
7  OC\AppFramework\Http\Dispatcher->executeController() /srv/www/htdocs/owncloud/lib/private/AppFramework/Http/Dispatcher.php:159
8  OC\AppFramework\Http\Dispatcher->dispatch() /srv/www/htdocs/owncloud/lib/private/AppFramework/Http/Dispatcher.php:89
9  OC\AppFramework\App::main() /srv/www/htdocs/owncloud/lib/private/AppFramework/App.php:98
10 OC\AppFramework\Routing\RouteActionHandler->__invoke() /srv/www/htdocs/owncloud/lib/private/AppFramework/Routing/RouteActionHandler.php:46
11 call_user_func:{/srv/www/htdocs/owncloud/lib/private/Route/Router.php:299}() /srv/www/htdocs/owncloud/lib/private/Route/Router.php:299
12 OC\Route\Router->match() /srv/www/htdocs/owncloud/lib/private/Route/Router.php:299
13 OC::handleRequest() /srv/www/htdocs/owncloud/lib/base.php:930
14 {main}          /srv/www/htdocs/owncloud/index.php:56

It looks like phpseclib has trouble parsing keys

[PVince81]

Here is an example key that fails:

$matches                         = (array[3]);
  $matches[0]                    = (string[892]) 'DEK-Info: DES-EDE3-CBC,0920080F8133E3CD2epqrFq8zuQpKk0bHothHWaVbDbdLz03liANnx1Sqbb9Q6mhFPsovjJ9BPF921LwnTCMSCT4A97d5qYcloINwg0UBK5TJNs1mRLrRs/GkKb1JYuS8LEJRsn/diUbi7Az7FkuJ1W+0m7/WzWbSHYVAhI6ihySCOBrCKejoiqKd9EOnBO1U5RIim5Z73+3dhAs5QZlavqQzEfORxmyV0MK8rmYse99bkM8n+9qpLhcTQoymeZgeHb/ilN5xvdJlk3vWqkUqWDj6V7L+PJMAfAkHaIcd98N1t8ewThBOixnLGQhQmxdJwgtm6P2/eD4+R54hZi4mpRW9rpHXiu6H6vd2p2AnLV9oR7ZrPocKnKxFFVfIbWtYZdFi0pxPdKTinPkb4ha/ZSbgVIAPYIHSnpxbjk6A2ly8PMoZBAbfaBNVcvuzpO9wYa7HOSATH8qzZf26pKo0yqFf+V5wWP0gVNNqYpyMKNRcR2fpJn2APoSoq1pL1gxF3RrSRxG5yDzl2taRzxQ5taJkJ8X1ByUkk2kkW2MBjOUlvXgtFDz6Sk+7eLXh4oL1dXH3ek5HVe0Q5aDtSynAAiKVnbiYytYIll8yUJTo4XIpU3M9MfMwrfa1XLk5zVdSQmpnvNdekJRXOXkiLshE0N++68WodqDUjqR/M0YT9bYFj5ymoDIYlRFOKq8utxvAZ5kGs3c55bLPXpCJWiBx3ga3vIKfslBf+ljSYcz1KiYR37A7oC2QvgllappNXCEt9CK+xKG3NyKDPc/Svgk3BVQ5uzZDr4jxBdV34RKasckeBXuYgi4zktRnaEc5wcBRMwnJQ==-----END RSA PRIVATE KEY-----';
  $matches[1]                    = (string[12]) 'DES-EDE3-CBC';
  $matches[2]                    = (string[869]) '0920080F8133E3CD2epqrFq8zuQpKk0bHothHWaVbDbdLz03liANnx1Sqbb9Q6mhFPsovjJ9BPF921LwnTCMSCT4A97d5qYcloINwg0UBK5TJNs1mRLrRs/GkKb1JYuS8LEJRsn/diUbi7Az7FkuJ1W+0m7/WzWbSHYVAhI6ihySCOBrCKejoiqKd9EOnBO1U5RIim5Z73+3dhAs5QZlavqQzEfORxmyV0MK8rmYse99bkM8n+9qpLhcTQoymeZgeHb/ilN5xvdJlk3vWqkUqWDj6V7L+PJMAfAkHaIcd98N1t8ewThBOixnLGQhQmxdJwgtm6P2/eD4+R54hZi4mpRW9rpHXiu6H6vd2p2AnLV9oR7ZrPocKnKxFFVfIbWtYZdFi0pxPdKTinPkb4ha/ZSbgVIAPYIHSnpxbjk6A2ly8PMoZBAbfaBNVcvuzpO9wYa7HOSATH8qzZf26pKo0yqFf+V5wWP0gVNNqYpyMKNRcR2fpJn2APoSoq1pL1gxF3RrSRxG5yDzl2taRzxQ5taJkJ8X1ByUkk2kkW2MBjOUlvXgtFDz6Sk+7eLXh4oL1dXH3ek5HVe0Q5aDtSynAAiKVnbiYytYIll8yUJTo4XIpU3M9MfMwrfa1XLk5zVdSQmpnvNdekJRXOXkiLshE0N++68WodqDUjqR/M0YT9bYFj5ymoDIYlRFOKq8utxvAZ5kGs3c55bLPXpCJWiBx3ga3vIKfslBf+ljSYcz1KiYR37A7oC2QvgllappNXCEt9CK+xKG3NyKDPc/Svgk3BVQ5uzZDr4jxBdV34RKasckeBXuYgi4zktRnaEc5wcBRMwnJQ==-----END RSA PRIVATE KEY-----';

[PVince81]

php7-7.1.6-1.1.x86_64

[PVince81]

Also happening with php5-5.6.31-1.1.x86_64

So it's not a PHP version thing

[PVince81]

Okay, I bisected this and the problem appeared with this commit 9b88fa6c8c0ebaa7d471476821cfc70778620ce9 which seems to strip "\n" and "\r" from the config options.

Not sure if this one is really needed, possibly a hardening. @Peter-Prochaska

I think the private and public keys are stored with newline in the DB, maybe we should convert it to a different format before storing, like base64 ? If we do, we'll also need to convert the old keys to the new format.

[peterprochaska]

Yes, this hardening is necessary. You can inject new headers for the server and create a new request.
So we have to find a new method to store the keys correct in the DB.

[PVince81]

Ok, so here's the plan:

• [ ] modify SFTP storage backend to store the private keys in base64 form (base64 the whole key string)
• [ ] write migration to migrate keys that were stored with the old format to the new format

Not sure yet about the migration. Easiest would be to switch to a new setting name so we can detect whether the old one is here or new one.

[Matan]

I've just upgraded to 10.0.3 using owncloud/server:latest docker and now getting this issue.

{"reqId":"SC9N5EqXeZFut5mX18Ow","level":3,"time":"2017-09-20T08:32:47+00:00","remoteAddr":"172.20.0.1","user":"matan.uberstein","app":"PHP","method":"PUT","url":"\/apps\/files_external\/globalstorages\/1","message":"pack(): Type H: illegal hex digit - at \/var\/www\/owncloud\/lib\/composer\/phpseclib\/phpseclib\/phpseclib\/Crypt\/RSA.php#1081"}

[PVince81]

If you need a quickfix you can revert this commit https://github.com/owncloud/core/commit/9b88fa6c8c0ebaa7d471476821cfc70778620ce9. Note that it might introduce back a little security hole.

[Matan]

I think doing that with the docker is going to quite tricky. When do you think the fix will be released?

[Matan]

I commented out lines 224-226 on lib/private/Files/External/StorageConfig.php inside the running docker. It's working now :+1: It's not ideal, but would do for now. Thanks for the info! :)

[PVince81]

@Matan aiming for 10.0.4 / next month for the clean fix.

[PVince81]

fix is here: https://github.com/owncloud/core/pull/29156

you can test it in 10.0.4beta1

[Matan]

I've upgraded to 10.0.4 and I can confirm the issue is resolved. :1st_place_medal:

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.