PR-staging

Success

User tests: Successful: Unsuccessful:

avatar Didldu-Florian
Didldu-Florian
9 Oct 2017

The security file check at the installation could be skipped, if the HTTP status code is not 401 (HTTP authentication required) which means the installation ist http access protected.

I think some web companys like me did work on http access protected development sites. The installation anyway take some steps, so, let's make the life easier and let's skip this if it is not needed.

For this, I have opened this pull request, where the http chek is now implemented.

Summary of Changes

Added HTTP status code check,
for skipping security file if website is http 401 authentication required, which means http access protected.
70173fb

Outsoureced functionality for the checks to two new methods
called checkHostSecurity() and checkSecurityFile(). Did also renamed some names called remoteDBFile to securityFile, because i think it is more understandable. Because we are at the InstallationModelDatabase so think its clear which file and it's more associated to the method names. And some improvements in the check order logic. Hope the community like it ;)

The new methodes are called during initialise()

// Save host checks
if ($this->checkHostSecurity($options) === false)
{
    if ($this->checkSecurityFile() === false)
    {
            return false;
    }
}

Testing Instructions

  • Create a new installation site which is behind an directory protection like http access authentication.
  • Go through the installation steps and you shouldn't see the security file check notification.
  • And check again, but this time without the http directory protection and the security check should work, the notification should be shown as known ;)

Documentation Changes Required

https://docs.joomla.org/J3.x:Secured_procedure_for_installing_Joomla_with_a_remote_database

Changes below in bold italic

English New Documenation Part:

Starting with Joomla! 3.7.4 the Joomla! Security Strike Team (JSST) implemented additional security checks in the install application in order to protect your web hosting accounts from being overtaken by a remote attacker. In case your database is not on the same server as your website and your website is not through a HTTP authentication protected, we require an extra check that makes sure you are the owner of the website.

...

German New Documenation Part:

Beginnend mit Joomla! 3.7.4 hat das Joomla! Security Strike Team (JSST) weitere Sicherheitsprüfungen in die Installations-Anwendung eingeführt, um Ihre Webhosting-Konten vor der Übernahme von einem entfernten Angreifer zu schützen. Im Falle, dass die genutzte Datenbank sich nicht auf dem gleichen Server befindet und die Website nicht durch eine HTTP-Authentifizierung geschützt ist, wird eine separate Prüfung verlangt, um zu versichern, dass Du der Inhaber der Website bist.

...

avatar joomla-cms-bot joomla-cms-bot - change - 9 Oct 2017
Category Installation
avatar Didldu-Florian Didldu-Florian - open - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
Status New Pending
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar brianteeman
brianteeman - comment - 9 Oct 2017

To me it looks like you haven't understood the purpose of the security check and the vulnerability it is protecting against.

avatar Didldu-Florian
Didldu-Florian - comment - 9 Oct 2017

Hi @brianteeman as i know, the security check is there to protect the installation from outside, right?

avatar brianteeman
brianteeman - comment - 9 Oct 2017

No it's much more than that

avatar Didldu-Florian
Didldu-Florian - comment - 9 Oct 2017

Of course @brianteeman , and there are also other checks. But this PR did just effect the dbhost check. And it would be very exotic if we check also the dbhost if the site ist http protected. So, why shouldn't we simplify it on protected installations?

avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian
Didldu-Florian - comment - 9 Oct 2017

Did now also added the changes for the documentation in my PR comment. Documentation in English and German if this help. If wished, we could write it to the official documentation site: https://docs.joomla.org/J3.x:Secured_procedure_for_installing_Joomla_with_a_remote_database

avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 9 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 9 Oct 2017
avatar brianteeman
brianteeman - comment - 10 Oct 2017

@SniperSister one for JSST to review

avatar joomla-cms-bot joomla-cms-bot - change - 11 Oct 2017
Title
Added HTTP status code check feature > for the insatallation #18294
Added HTTP status code check feature > for the installation #18294
avatar franz-wohlkoenig franz-wohlkoenig - change - 11 Oct 2017
Title
Added HTTP status code check feature > for the insatallation #18294
Added HTTP status code check feature > for the installation #18294
avatar joomla-cms-bot joomla-cms-bot - edited - 11 Oct 2017
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 11 Oct 2017

corrected Typo in Title.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18295.

avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar zero-24
zero-24 - comment - 11 Oct 2017

@Didldu-Florian To me the idea looks ok but can we move that checks (all three) to a separate private method? That would keep the code simpler to understand as this checks should not all placed in the initialise method so we keep that small and simple to read. Thanks.

avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
Labels Added: PR-staging
avatar Didldu-Florian
Didldu-Florian - comment - 11 Oct 2017

@zero-24 I'm glad you're okay with this. Did outsoureced the functionality for the checks to two new methods called checkHostSecurity() and checkSecurityFile(). Did also renamed some names called remoteDBFile to securityFile, because i think it is more understandable. Because we are at the InstallationModelDatabase so think its clear which file and it's more associated to the method names. And some improvements in the check order logic. Hope the community like it ;)

The new methodes are called during initialise()

// Save host checks
if ($this->checkHostSecurity($options) == false)
{
        $this->checkSecurityFile();
}

Hope it is more clear

avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar Didldu-Florian Didldu-Florian - change - 11 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 11 Oct 2017
avatar zero-24
zero-24 - comment - 12 Oct 2017

This does not work as expected. The check can be skipped. The method initialise needs to return false in case of an error / not matching file ;)

avatar Didldu-Florian
Didldu-Florian - comment - 12 Oct 2017

@zero-24 sorry, can't repoduce your concerns. Did tested again and it works. By the, way, the method initialise returns JDatabaseDriver or false on failure. Same as it worked before my PR.
Can you describe your concern in more detail?

Please try hardset $statusCode = 401; or $statusCode = 200; at line 216 and be sure db_host is not localhost.

HTTP 200 >The securityFileCheck will be required.
HTTP 401 >The securityFileCheck will be skipped.

avatar zero-24
zero-24 - comment - 12 Oct 2017

@Didldu-Florian yes that part is working (401 aka .htaccess)

but when the check should be done you can just hit "install" and it starts doing things. (even that it stop working on another issue)

Basically that check in the initialise method should make sure that the validation fails and the user can't go any step forward. For that reason initialise needs to return false when the checks did not match on a host where we require to check it ;)

avatar Didldu-Florian
Didldu-Florian - comment - 12 Oct 2017

@zero-24 Oh dear! You are right, that was lost during outsourcing methods. My fault.
Did fixed it with c0a90dd Now the logic is same as before on return.

avatar Didldu-Florian Didldu-Florian - change - 12 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 12 Oct 2017
avatar zero-24
zero-24 - comment - 12 Oct 2017

@Didldu-Florian it it just allowed to return false in case there is a error ;)

Else you can't move forward as the method return void just before we try to setup the database ;)

// Save host checks
if ($this->checkHostSecurity($options) === false)
{
        if ($this->checkSecurityFile() === false)
        {
                return false;
        }
}

This should do the trick 😄

avatar Didldu-Florian
Didldu-Florian - comment - 12 Oct 2017

@zero-24 Thanks, done ;)

avatar zero-24
zero-24 - comment - 12 Oct 2017

I have just added the missing= for the typesafe check. Looks good from here than. (the issue @Quy mention should be fixed too)

avatar Didldu-Florian
Didldu-Florian - comment - 12 Oct 2017

@zero-24, @Quy __DEPLOY_VERSION__ added ;)

avatar Didldu-Florian Didldu-Florian - change - 12 Oct 2017
The description was changed
avatar Didldu-Florian Didldu-Florian - edited - 12 Oct 2017
avatar zero-24
zero-24 - comment - 13 Oct 2017

👍 thanks @Didldu-Florian

avatar Quy Quy - change - 27 Oct 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-10-27 14:17:28
Closed_By Quy
avatar Quy Quy - close - 27 Oct 2019
avatar Quy
Quy - comment - 27 Oct 2019

Replaced by PR #26840


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/18295.

Add a Comment

Login with GitHub to post a comment