XMPP http upload

[pitchum]

The problem

Sharing files with XMPP's http upload mechanism is currently impossible.
This PR is an attempt to address issue #1278

Metronome's configuration is ready for http upload but port 5290 is not reachable.

Solution

http upload requires a dedicated subdomain (I have chosen jabber.thedomain.net instead of upload.thedomain.net to avoid possible conflicts with possible other "upload" things).

This subdomain should be reachable via HTTPS so we need:

• add a DNS entry for that subdomain
• transparently include jabber.thedomain.net as a SAN in the same certificate for thedomain.net
• explicitly define the storage path in metronome's config (the same will be configured in nginx)
• create an nginx config for that subdomain
• silently recreate existing Letsencrypt certificates

PR Status

The PR ready for review.

How to test

Create 2 accounts : alice and bob
Install Dino and configure those accounts.
Happily share pictures between alice and bob.

Validation

• Principle agreement 0/2 :
• Quick review 0/1 :
• Simple test 0/1 :
• Deep review 0/1 :

[zamentur]

What do you think to replace jabber by upload-xmpp ? Is jabber a common name for upload http ?

[colmoneill]

Hi all, just here to show my support, I've been using the XMPP & metronome features of my ynh instance, I would have never been able to configure these myself so wanted to say thanks and looking forwards to the possibility of uploads via ynh xmpp too. Cheers !

[alexAubin]

(Flagging as work needed / test needed until you give the green light 😜 )

[pitchum]

Status of this PR: Proof of concept validated
It works for me, in a VM using ynh-dev.

Some things may need improvement:

• find better location and permissions on the directory shared by metronome and nginx
• cleanup hard-coded values ("jabber." + domain)
• rename jabber.thedomain.net to xmpp-upload.thedomain.net
• check if disabling bosh module does not break something else (apps like movim may rely on this)

[maranda]

I'd suggest using mod_http_upload_external instead of mod_http_upload, although configuration extra care in regards of nginx configuration should be adopted since the recent discovering of vulnerabilities in its FastCGI implementation.

[maranda]

That should remove all the limitations of narrow file sizes, and also meddling too much with forwarding ports as all requests will be handled by nginx directly.

[pitchum]

I've simplified my changes to make this PR easier to review.
I'll propose other PR later.

One important thing I don't know how to handle properly is the renewal of existing letsencrypt certificates. We should find a way to silently drop existing certificates and create a new one including SAN for xmpp-upload.thedomain.net

[pitchum]

I'd suggest using mod_http_upload_external instead of mod_http_upload

@maranda using mod_http_upload_external would need more extra work (implementing a new service handling PUT requests). I have currently no idea how to implement this easily and I don't have enough time to work on this.
My solution is simpler: nginx handles GET requests and forwards PUT requests to metronome.

[maranda]

@maranda using mod_http_upload_external would need more extra work (implementing a new service handling PUT requests). I have currently no idea how to implement this easily and I don't have enough time to work on this.
My solution is simpler: nginx handles GET requests and forwards PUT requests to metronome.

@pitchum you don't have to work on a PUT service because the php script to perform the uploads is already included into Metronome it just has to be configured and it's much less work than messing with forwarding all together. See: https://github.com/maranda/metronome/blob/v3.13.0/plugins/http_upload_external/resources/share.php

[pitchum]

@maranda I still don't see why mod_http_external_upload would be useful here and I won't have time to work on this.
So I suggest you create a separate PR when this one is merged.

I'd like to have http_upload feature working in yunohost ASAP.

[maranda]

@pitchum all it takes is adding the relative configuration to nginx, replacing paths into the supplied share.php, and adding config into the main config file of metronome... That's a few minutes work, but otherwise whatever suits your boat.

[colmoneill]

Hey all, just checking back in to show support. If there is any specific testing a non technically aware person like myself could do, I can spin up a dev RPI for these purposes ?
Let me know how I can help and thanks for pushing this fwd !

[pitchum]

@colmoneill I still have to implement a transparent migration of existing certificates. When it's done I'll describe a testing procedure. I hope to have time to work on this tomorrow.

[pitchum]

@colmoneill I did not have time to implement automatic certificate renewal but I've build a DEB package for testing http_upload though.

To test it connect to your yunohost using SSH and type this:

wget https://gramaton.org/~pitchum/tmp/ynh/yunohost_3.6.5.3+pitchum1_all.deb
dpkg -i yunohost_3.6.5.3+pitchum1_all.deb

Then:

1. make sure you have added xmpp-upload 3600 IN CNAME @ to your DNS zone
2. enforce renewal of certificate for the corresponding domain
3. use some modern XMPP client (Conversations, Gajim or Dino for example) and try to share an image with contacts or in a chatroom

[colmoneill]

Amazing @pitchum thanks so much !

Would you say this is safe to try on my 'production' YNH machine ?
If so, I will try it straight away, if not, I will have to set up a new dev env.

Thanks again, looking forwards to trying this !

All the best !

[pitchum]

@colmoneill

Would you say this is safe to try on my 'production' YNH machine ?

No, do not try this on your production YNH, you are the first one to test this (after myself on a testing VM)

[M5oul]

I manage to have it working! \o/
I also had to add a rule in SSOwat configuration in etc/ssowat/conf.json.persistent:

{
    "skipped_urls": [
        "xmpp-upload.domain.tld"
    ]
}

We should find a clean and permanent rule in SSOwat for that

[colmoneill]

Great to hear this progress, sorry I haven't had time to do any tests myself, might wait for your changes to be merged @M5oul ?

Thanks all !

[colmoneill]

Hey all, thanks for this and the last push to get things together.
It's really interesting to follow the process for someone like myself, with low skill levels. I know it will be another while before this makes it's way into an YNH update, but it will be great when we have it.

Will this process of adding a new subdomain work for people using DynDNS systems ? My production setup is using a NoIP domain name which is already a subdomain similar to the trio made available by noho.st etc

Just asking as it would be a good prep step for me to get a specific domain if this was going to be a requirement for XMPP file sharing to work.

Thanks again !

[pitchum]

I think this PR is ready to be merged.
Here is how to test (DON'T DO THIS ON A PRODUCTION INSTANCE):

apt update
apt dist-upgrade
wget https://gramaton.org/~pitchum/tmp/ynh/pr831/moulinette_3.7.0.1%2Bnmu1_all.deb
wget https://gramaton.org/~pitchum/tmp/ynh/pr831/yunohost_3.8.0~alpha%2Bnmu1_all.deb
dpkg -i moulinette_3.7.0.1+nmu1_all.deb
dpkg -i yunohost_3.8.0~alpha+nmu1_all.deb

Some errors during migrations will appear. Run yunohost diagnosis show --issues to see them all.
In my case it was only "normal" errors:

• "inconsistent version numbers" : I realize that my package versions are weird but it's not important
• missing DNS recors: that was intended, I've added missing records later
• "Domain xxxx.yyy is unreachable through HTTP from outside": this one is wrong, don't know why

[alexAubin]

I pushed a couple of commits, hope it's okay, otherwise I can remove them :

• change xmpp-upload folder from /var/www/xmpp-upload.{domain}/ to /var/xmpp-upload/{domain}, otherwise that would be likely to create a shitload of folder in /var/www/ (where tech-savy user sometimes wander)
• factorize all the ciphers and header stuff in nginx conf into a common security.conf.inc. That's pretty much unrelated to this feature, except that the diff with the new vhost copypasta suggest it to increase readability.

Then I realized that there are probably some tweaks to do, considering that the whole xmpp-upload thing is only supposed to work for the main domain (c.f. the metronome conf ?). I can take care of the required changes if you don't feel like or don't have time to do those.

Othewise, I haven't really tested the PR in terms of actually trying the feature from an XMPP client, but I trust you folks on that point and that's pretty much good for me 👍.

[pitchum]

I followed @alexAubin's suggestions but I haven't tested those changes yet.

(I don't understand all the features of github and I don't know what to do with the message "2 reviews requesting changes")

[kay0u]

I was about to test it, but at almost every command, I get this error:

Traceback (most recent call last):
File "/usr/lib/python2.7/dist-packages/moulinette/actionsmap.py", line 555, in process
fromlist=[func_name],
File "/usr/lib/moulinette/yunohost/tools.py", line 41, in
from yunohost.domain import domain_add, domain_list
File "/usr/lib/moulinette/yunohost/domain.py", line 35, in
import yunohost.certificate
File "/usr/lib/moulinette/yunohost/certificate.py", line 46, in
from yunohost.domain import _get_maindomain
ImportError: cannot import name _get_maindomain

the _get_maindomain function exists... any hint?

[alexAubin]

Looks good to me, thanks for taking care of the changes (also sorry about forgetting the security.conf.inc file !)

Comment has been taken care of

[alexAubin]

Anyway, let's try to move forward with this.

I'm proposing to merge this on Monday afternoon and it'll still be possible to do more tests and fixes after merge anyway

[pitchum]

I've made some tests on a VM and everything looks fine. I think this PR is ready to be merged right now, but I'm OK to wait monday :)

@kay0u the import error is caused by the line "import yunohost.certificate" in domain.py .
That's why we can't import something from domain.py in certificate.py... some kind of infinite loop occurs.
My workaround is to lazy-import the function.

@alexAubin maybe I'm wrong but it looks like the code concerning certificates in domain.py is dead code and can be removed.
I would remove line 35 and lines 300-311 and insert a specific import line 109.

[MaStr]

I just wanted to implement manually this and found this PR.
Please do a proper announcement, if you are rolling out the changes for this, because people issuing their own LE certs may need to adjust their stuff. (I'm affected, because need to run a reverse proxy (having multiple webservers behind nat)).

[alexAubin]

Merging naow, congratz everybody!

---

@alexAubin maybe I'm wrong but it looks like the code concerning certificates in domain.py is dead code and can be removed.
I would remove line 35 and lines 300-311 and insert a specific import line 109.

Eh pretty sure that lines 300-311 are not dead, they are what makes the link between the CLI "yunohost domain cert-status" (and install, renew) and the actual functions ... This was implemented before the subcategory system, otherwise probably would have been directly "yunohost domain certificate status" (though anyway there are still naming/import trickery to do if you want to have the subcategory code in a separate file ...)

But you're right about the lazy loading which might improve performances a bit, I'll do a commit after the merge