right now, the site_exists() check uses the port to see if the site already exists. ie. that allows example.com:80 and example.com:443 to be created separatly.

however, if client A owns example.com:80, client B shouldn't be allowed to create example.com:443 (and vice-versa), so we have a new security issue over here... this should be fixed.

CommentFileSizeAuthor
#8 570980_hosting_vhosts_multiports.patch8.55 KBmig5
#8 570980_provision_vhosts_multiports.patch8.62 KBmig5
#2 allow_domain.png81.38 KBmig5

Comments

anarcat’s picture

anarcat commented
Status: Active » Fixed

I have changed the API. Now we use hook_domain_allow() to check all potential hooks for errors (all must return true). More information is available to the hooks so they can validate against users and such.

alias and site hooks were fixed so that they take clients into considerations.

to test this, create "sitea" on port 80 for client A. then try to create sitea on port 443 for client B. this should yield a warning and fail. then try to create sitea on port 443 for client A, this should work.

eventually, all this will be useful to control domain-level permissions when DNS kicks in.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Needs work
StatusFileSize
new allow_domain.png81.38 KB

Some strange stuff happening here.. after pulling latest from git and running all updates:

I add site a and I see strange stuff both in the drupal_set_message and errors beneath it:

    * client set: 1
    * SELECT COUNT(n.nid) FROM {node} n JOIN {hosting_site} h ON n.nid = h.nid WHERE type='site' AND title='%s' AND h.status <> %d AND ( h.client <> %d OR h.port = %d ) : 1
    * Site siteaa.home.mig5.net has been created.

user warning: Column count doesn't match value count at row 1 query: INSERT INTO hosting_site (vid, nid, client, db_server, platform, profile, language, last_cron, status, verified, port, `ssl`, ssl_redirect) VALUES (157, 140, 1, 2, 5, 86, 'en', 0, 0, 0, 80, 0) in /var/aegir/drupal-6.13/profiles/hostmaster/modules/hosting/site/hosting_site.module on line 419.

Screenshot attached for context.

Edit: So we're not inserting any value into ssl_redirect column (missing a variable substitution probably). I think i'll remove the drupal_set_message in hosting_site_allow_domain() too as it looks useful only for debugging yeah?

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Fixed

Fixed in cvs.

P.S, site creation on different ports for only the same client works as advertised.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Needs work

Rahh creating the site the second time on port 443 fails for fairly obvious reasons:

Task starts processing
Running: php /var/aegir/drush/drush.php --root='/var/aegir/drupal-6.13' 'provision' 'install' 'siteaaa.home.mig5.net' --backend
Drush bootstrap phase : _drush_bootstrap_drush()
Drush bootstrap phase : _drush_bootstrap_drupal_root()
Loading drushrc "/var/aegir/drupal-6.13/drushrc.php" into "drupal" scope.
Initialized Drupal 6.13 root directory at /var/aegir/drupal-6.13
Found command: provision install
Initializing drush commandfile: provision_apache
Undefined index: base_url
Initializing drush commandfile: provision_drupal
Initializing drush commandfile: provision_mysql
Undefined index: db_url
Including /var/aegir/.drush/provision/web_server/install.provision.inc
Including /var/aegir/.drush/provision/platform/install.provision.inc
Including /var/aegir/.drush/provision/db_server/install.provision.inc
mkdir(): File exists
Could not create sites/siteaaa.home.mig5.net
Changed permissions of sites/siteaaa.home.mig5.net to 0755
Changed permissions of sites/siteaaa.home.mig5.net/files to 2770
Changed permissions of sites/siteaaa.home.mig5.net/files/tmp to 2770
Changed permissions of sites/siteaaa.home.mig5.net/files/images to 2770
Changed permissions of sites/siteaaa.home.mig5.net/files/pictures to 2770
Changed permissions of sites/siteaaa.home.mig5.net/themes to 0755
Changed permissions of sites/siteaaa.home.mig5.net/modules to 0755
Changed ownership of sites/siteaaa.home.mig5.net/files
Changed group ownership of sites/siteaaa.home.mig5.net/files
Changed ownership of sites/siteaaa.home.mig5.net/files/tmp
Changed group ownership of sites/siteaaa.home.mig5.net/files/tmp
Changed ownership of sites/siteaaa.home.mig5.net/files/images
Changed group ownership of sites/siteaaa.home.mig5.net/files/images
Changed ownership of sites/siteaaa.home.mig5.net/files/pictures
Changed group ownership of sites/siteaaa.home.mig5.net/files/pictures
An error occurred at function : drush_provision_drupal_pre_provision_install
Command dispatch complete
Removing task from hosting queue
An error occurred at function : drush_hosting_hosting_task
Changes for drush_hosting_hosting_task module have been rolled back.
Command dispatch complete

This will need more work, I guess provision should check to see if the site already exists on a different port too, and if it does, just create the vhost.

Anonymous’s picture

Anonymous (not verified) commented

this is easy enough fixed by adding a if (!_provision_drupal_site_exists(drush_get_option('site_url'))) { conditional around _provision_drupal_create_directories() in platform/provision_drupal.drush.inc

However it's occurred to me that we are gonna need to change how provision decides what to do: if the site already exists on the same port, we actually will want to rip out most of provision's tasks altogether: i.e it should ONLY add the vhost, and not create the database or anything else.

Mig

anarcat’s picture

anarcat commented
Title: do not allow vhosts of the same name on a different port » allow vhosts of the same name on a different port
Category: bug » feature

So I agree this is broken and a real problem. If we actually want this feature, we'll actually need to change the sites path so that different sites can be put on different ports. Otherwise, we'll need to rip out the code to simply disallow two hosts on different port, but that would be unfortunate.

I don't like the idea of having two different sites that share the same settings.php, it's a bit counter-intuitive from the UI perspective at this point... Maybe this also relates to site cloning...

Anonymous’s picture

Anonymous (not verified) commented

So I think we ought to operate on the logic that while one can create two site nodes with the same domain name but different port (as long as the client is the same per the earlier fix), the two site nodes should actually point to the same site but just run on different ports (inferring protocol).

So it's a one-to-many relationship of one site > many ports.

This being the case, the easier way to fix this, rather than mess with Provision a huge amount, is to modify Ports so that instead of radio boxes on the site node, one can select multiple ports. Then on provision of the site, one need only create a vhost per port, and only one actual drupal install is run.

I have a feeling this will impact the 'SSL / redirection' checkboxes, i.e I suspect they will operate on the logic that only one port has been selected, and so thats how it knows what port to use for its vhost tinkering.

I'll focus on this I think, for a while, as it's one of the more major ones, and I have too many half-fixed issues on the go right now :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 570980_provision_vhosts_multiports.patch8.62 KB
new 570980_hosting_vhosts_multiports.patch8.55 KB

Patches attached. I think it's almost there but probably needs some testing / review. It's taken me too long to get this far, not enough time to work on this stuff :(

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

*Sigh* I did it again. The $GLOBALS['base_url'] in provision is a mess. Back to the drawing board.

Anonymous’s picture

Anonymous (not verified) commented

So sadly I've been dropping the ball on this one.

Does anyone have a good idea/decision on how to handle the situation that this feature brings up:

If one enables port 80 and say, port 81, or port 8080, whatever, what should we decide is the global $base_url? This affects the Login onetime reset URL among other things.

Basically since this feature turns the $node->port into an array (well, currently, a string that we explode into an array to fiddle with), we need to decide which to use for things like this.

Maybe we just take the first port in the array? (and of course I take into account if it is not an array at all and only one port)

There was also potentially some issues with the combination of Site Alias redirection and HTTP > HTTPS redirection, though I might've solved it in the last patch.

Thoughts welcome and I will try and prepare another patch, I want to put this thing to bed.

adrian’s picture

adrian commented

ummm....

what about aliases ?

if you need the same site running on multiple ports isn't that an alias ?

Anonymous’s picture

Anonymous (not verified) commented

More or less, the point being a different port means it needs its own < VirtualHost > entry. They can't just be ServerAliases in a :80 vhost, as far as I know anyway.

I have it all pretty much working: the only thing I'm grappling with is handling the many different combinations of multiple ports + site aliases + redirection of those aliases to a main site + SSL + HTTP to HTTPS redirection. We've introduced a lot of new functionality that steps on each others toes in this regard.

At least I have the global base_url sane at the moment. People are free to track my branch in git

adrinux’s picture

adrinux commented

mig5 has just pointed out on #aegir that this issue is connected with my lesser one[1]. So a reminder that Drupal's way of dealing with non-standard ports is to prefix the sites/sitename folder with the port:

sites/example.com
sites/8080.example.com
sites/443.example.com

This is documented in default.settings.php.

I think you've already noticed that right now aegir doesn't do this! It ignores the port when creating the settings folder.

I know this issue is more complicated than that, but it's worth remembering what drupal does.

[1]http://drupal.org/node/600508#comment-2142586

Anonymous’s picture

Anonymous (not verified) commented

Per discussion on IRC, working out what needs to be done to fix multiports here

It all works except some vhost config combinations:

What works:

1) HTTP - Multiple ports
2) HTTP - Multiple ports with Aliases
3) HTTP - Multiple ports with Aliases and Alias redirection
4) HTTP & HTTPS multiple ports
5) HTTP & HTTPS with Aliases

What doesn't work:

1) HTTP & HTTPS with Aliases and Alias redirection - only one tiny thing: the Rewrite rule in the 443 vhost uses http://xxx:443 and should just be https:// with no port. I thought I fixed this but might've lost it from my branch (lost my git repo once)

2) HTTP & HTTPS with HTTP > HTTPS redirection

The 443 vhost has the rewrite rule but missing the ServerName. The non-ssl vhosts are worse:
< VirtualHost *: >
RedirectMatch permanent ^(.*) http://:$1
< /VirtualHost >

3) HTTP & HTTPS with Aliases and Alias redirection & HTTP > HTTPS redirection

Pretty much the same as 2)

Some other cleanup:

1) Since we can create vhosts on multiple ports, including HTTP and HTTPS combinations, we don't need the javascript stuff that automatically disables port 80 when SSL / SSL Redirection is selected. We actually *want* to create the port 80 vhost, probably, and this is just a legacy thing from the idea that the port 80 site already exists and might be a 'separate' site.

2) Make sure that deleting all vhosts works on delete. I think it does, but it's been a while.

Git branches:

http://git.mig5.net/?p=modules/provision/.git;a=shortlog;h=refs/heads/57...

http://git.mig5.net/?p=modules/hosting/.git;a=shortlog;h=refs/heads/5709...

Remember update.php , there's a hook_update in Hosting

adrian’s picture

adrian commented

woah.

whats the status on this ?

Anonymous’s picture

Anonymous (not verified) commented

I lost my branches somehow and I was getting nowhere with it anyway, couldn't match every possible use case properly. Needs redoing perhaps based on the last patches I submitted here.

adrian’s picture

adrian commented

so i'm pretty emphatic that we should have different sites running on the same hostname with different ports.

we have a single namespace in the hosting_context table now, and i'm sure as hell not making the port part of the identifier or the site.

adrian’s picture

adrian commented

I'm pretty sure that this will sidestep all of that :
#836136: Simplify ports support.

The port setting is not something associated with the site, but rather the server configuration.

All the use cases for sites on different ports have also been exclusively :

1. site runs on smething other than port 80 for varnish or other reasons.

2. site runs on a web port and then an ssl port.
2a. Redirect all requests to the web port to ssl.
2b. Handle selective redirection to SSL with something like secure pages.

adrian’s picture

adrian commented
Status: Needs work » Fixed

This is no longer an issue.

We will only allow a vhost to be on a single port, and if they are using the SSL version of the apache service they will be able to run on an additional port.

The ssl setting on the site will be 0 (http only) , 1 (ssl + http) , 2 (ssl only , implies redirect)

You can set both the SSL and the standard HTTP port to whatever you want. But it's a setting on the server.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.