Projects tigase _server server-core Issues #945
Adding VHost via ad-hoc doesn't work on AWS (#945)
Closed
wojciech.kapcia@tigase.net opened 6 years ago

Only main IP is checked, which in case of AWS is the internal IP. We need to check both internal and external IP in case we are verifying DNS.

I think that server admins should have option to skip validation -- %kobit , %andrzej.wojcik ?

Artur Hefczyc commented 6 years ago

Wojciech Kapcia wrote:

I think that server admins should have option to skip validation -- %kobit , %andrzej.wojcik ?

Yes, the super admin should be able to skip the validation and as far as I remember that was how I implemented it, unless my memory is not good enough.

Andrzej Wójcik (Tigase) commented 6 years ago

But there is a property in config.tdsl which allows you to disable DNS verification, so do we really need to have separate "feature" for super admin? isn't property enough? Also for AWS, this check needs to be permanently disabled, so there is no point to allowing adding domain without verification only for super admin.

I would rather suggest allowing to configure the server to pass ELB domain so that we could verify if domain which we try to add is resolving to the same domain which we have set for ELB. That would be better.

Just a note - the same issue we will have on sure.im for hosted domains if I'm correct, so plain users need to be able to add domains there and not only super admins.

wojciech.kapcia@tigase.net commented 6 years ago

Andrzej Wójcik wrote:

But there is a property in config.tdsl which allows you to disable DNS verification, so do we really need to have separate "feature" for super admin? isn't property enough?

This requires restarting the server, which may be itsy-bitsy inconvenient, when you have (even small) number of users. Being able, as an admin, to modify it is simply convenient.

Also for AWS, this check needs to be permanently disabled, so there is no point to allowing adding domain without verification only for super admin.

I would rather suggest allowing to configure the server to pass ELB domain so that we could verify if domain which we try to add is resolving to the same domain which we have set for ELB. That would be better.

Yup, this is the gist of the issue actually - checking the most external/user facing/returned-by-DNS IP.

Just a note - the same issue we will have on sure.im for hosted domains if I'm correct, so plain users need to be able to add domains there and not only super admins.

This actually came after working on the sure.im instance. And IMHO disabling verification for everyone is not a good idea (I guess the verification itself was implemented in the first place to make sure that users will be only to add their own domains, but only the ones that are valid). But it would be still handy to be able (as an admin) not be bothered with the verification.

After glancing over the sources there is a check for isAdmin() but it doesn't seem to work so super-admin requests are still going through validation.

Andrzej Wójcik (Tigase) commented 6 years ago

I've modified logic in ad-hoc scripts, so that server admin may add/modify vhosts with invalid settings.

Moverover, I've introduced the new setting for vhost named installation-dns-address which can be set with DNS address pointing to load balancer/proxy DNS address. With this property set, Tigase will validate vhosts DNS settings with DNS entries returned for name set in this new property.

wojciech.kapcia@tigase.net commented 6 years ago

Skiping validation for admin accounts works now.

As for checking for correctnes - it would be good to add this information to documentation, i.e. how to configure it:

'vhost-man' {
    vhostRepository () {
        instance () {
            'installation-dns-address' = '…'
        }
    }

While configuring our installation, it occured to me, that we also have dns-srv-def-addr which could possible be used as a shorthand instead of vhostRepository/instance/installation-dns-addres? Or even instead?

Andrzej Wójcik (Tigase) commented 6 years ago

I've added documentation for 'installation-dns-address'.

I'm against usage of dns-srv-def-addr even as a shorthand. It is possible to set installation-dns-address even in root context of the configuration file and it will for so there is no point for shorthand and full name of the property is more meaningful than the proposed shorthand.

Moreover, their behavior is slightly different. installation-dns-address makes sure that SRV, A or CNAME of the domain added as a vhost resolves to either this DNS name or IP addresses to which this domain name resolves to. While dns-srv-def-addr checks only if it resolves to this particular name. So new property adds some flexibility to DNS setup but it also has stricter checking of DNS entries to for domain - it makes sure that it post only to this installation and not to other installation as well (ie. by 2 SRV records pointing to two distinct XMPP installations).

With that in mind I would consider deprecation of dns-srv-def-addr property.

wojciech.kapcia@tigase.net commented 6 years ago

I think we can leave it for now. Thank you for adding documentation bit.

issue 1 of 1
Type
Bug
Priority
Normal
Assignee
RedmineID
7477
Version
tigase-server-8.0.0
Spent time
15h
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#945
Please wait...
Page is in error, reload to recover