Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(WIP) matrix-synapse: 0.31.2 -> 0.33.0 #43889

Closed
wants to merge 1 commit into from

Conversation

Ekleog
Copy link
Member

@Ekleog Ekleog commented Jul 21, 2018

Motivation for this change

Backport of #43888. As v0.31.2 appears to no longer be able to receive messages from matrix.org, I guess this is important enough a problem to deserve a backport to release-18.03. However… I don't know what the problem was about, and upstream didn't appear to have any clue about it either, apart from “try upgrading”.

That said, it may break setups, so maybe stability for non-federated setups is more important than being able to talk with :matrix.org? I'll let you, the reader, decide 😝

Tested on my otherwise-18.03-running HS.

cc @Ralith @roblabla

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@Ekleog Ekleog changed the title matrix-synapse: bump to 0.33.0 matrix-synapse: 0.31.2 -> 0.33.0 Jul 21, 2018
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: matrix-synapse

Partial log (click to expand)

stripping (with command strip and flags -S) in /nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/lib  /nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin
patching script interpreter paths in /nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0
checking for references to /build in /nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/move_remote_media_to_new_store.py'...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/synapse_port_db'...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/hash_password'...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/synctl'...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/register_new_matrix_user'...
wrapping `/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0/bin/homeserver'...
/nix/store/0yfsa0d3ar1xh83xz5ci2r0rlkx5snnq-matrix-synapse-0.33.0

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: matrix-synapse

Partial log (click to expand)

stripping (with command strip and flags -S) in /nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/lib  /nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin
patching script interpreter paths in /nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0
checking for references to /build in /nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/hash_password'...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/homeserver'...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/move_remote_media_to_new_store.py'...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/register_new_matrix_user'...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/synapse_port_db'...
wrapping `/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0/bin/synctl'...
/nix/store/np920j4kfsgrrs6cmlnm5bv2fqjgrliv-matrix-synapse-0.33.0

@Ralith
Copy link
Contributor

Ralith commented Jul 21, 2018

My un-upgraded HS is still talking to matrix.org just fine, FWIW. Not opposed to the backport, though, synapse certainly needs all the bugfixing it can get.

@Ekleog
Copy link
Member Author

Ekleog commented Jul 21, 2018

Hmm so now I'm even more at a loss about why matrix.org refused to send me messages before the upgrade…

@Ekleog
Copy link
Member Author

Ekleog commented Jul 21, 2018

@fpletz @vcunat Any guidelines on whether potentially backwards-compatibility-breaking updates should be backported for highly unstable software? (I'd think the 0.* version is enough to allow us to backport, but I'm not the release manager here 😋)

(BTW, I couldn't find again any guidelines for backporting, even though I'm almost sure I have already read some; if you know where they are I could maybe copy them into the NixOS manual?)

@vcunat
Copy link
Member

vcunat commented Jul 23, 2018

I do have a WIP section for the manual. I'll polish it and ping you soon.

@vcunat
Copy link
Member

vcunat commented Jul 23, 2018

For now you can refer to the ## Maintenance workflow section in https://groups.google.com/forum/#!topic/nix-devel/3KxPNwxDV9E

@Ekleog
Copy link
Member Author

Ekleog commented Aug 2, 2018

Hmm, so, trying to evaluate this change against https://groups.google.com/forum/#!topic/nix-devel/3KxPNwxDV9E :

  • Certainly all security patches which aren't major updates. If a
    security patch is a major upgrade, try and find minor updates or patches
    for our current version which fix the same vulnerability. Apply the
    major update to master or staging, and the minor one to stable.

The upstream synapse team considers 0.32.0 as being a security update, because it adds the possibility of black/whitelisting servers, and for that it needs every participating server to be v0.32.0+. I, personally, do not consider this as a security vulnerability.

However, matrix-org/synapse#3470 looks like a DoS to me, merged in 0.32.0 too. And matrix-org/synapse#3546 (merged in 0.33.0) appears to be something that might have legal consequences, IIUC and it means GDPR-erasure wasn't actually well-handled. (unsure for both of these, though)

So we'd have to either update or somehow backport these PRs into 0.31.2.

  • Any updates when the current version on stable is broken. A key
    example of this is Spotify, who regularly breaks their old versions.

This is something I've experimented with synapse, while @Ralith appears not to. Unsure what the correct behaviour is in this case, but I'd consider that synapse being declared unstable by the maintaining team would be a hint that this is likely to be true for synapse.

  • Extremely security-sensitive software, in particular Chrome, Chromium,
    Firefox, Thunderbird, and of course kernel minor bumps.

Not the case here.

  • Bugfix-only updates (minor, maintenance). Generally be cautious about
    these. Backporting is optional if the fixes don't seem important; we
    don't want to risk breakage if there's little to gain.

Not the case here.

  • Optionally, new package additions, as that won't break anything.

Not the case here.


FTR, debian has 0.33.0 in testing (but no matrix-synapse in any stable version). Fedora 28 has 0.31.2, last updated 2 months ago. I think the notable fact though is that they have upgraded from 0.27.3 to 0.31.2 (sorry for the web proxy link, pkgs.fedoraproject.org appears to use an invalid certificate with HSTS from here) during the lifecycle of Fedora 28.

I have checked only Debian and Fedora.

Overall, I personally think the level of stability expected from nixos-stable should be roughly equivalent to the level of stability expected from fedora, given the similar release cycle. And so I'd tend to think that this can make it into stable.

That said, I feel like the above instructions miss something that would be interesting for synapse: the behaviour to follow when upstream declares a package unstable.

  • Should we try to make it stable by backporting all (bug and) security fixes ourselves? Upside: stable. Downside: very time-consuming.
  • Should we just accept that upstream is unstable and propagate potentially-breaking updates into nixos-stable? Upside: much less time-consuming. Downside: potentially breaks stable-channel users' workflow.

For the record, (IIRC it was a bit before 18.03 branch-off) I already had an issue with :matrix.org refusing to talk to synapse, with no logs from my side and no issues with another HS I have access to. When asking on matrix's channel if they had any idea why that would be the case, I was asked my synapse version. When answering (was that 0.24? something like that?), I have been answered “that's ancient, just update, we'll bother trying to look into our logs if it still doesn't work with the latest release” (not exact quotation, but the idea was that).

Overall I think this means there are bugs in synapse even the development team has no idea how they came in (I've been told they never knowingly broke backwards-compatibility), and even for a bug with 0.31.2 they first want bug-reporters to upgrade to the latest version before looking into their logs (and as their HS is the only one causing the issue…)

This little rant being done (for the purpose of showing that a non-updated synapse is likely at least as hard to handle as an update to synapse), I overall think that for synapse, we should backport this update, because not updating appears to be enough to break stuff with synapse. But I'll ask NixOS/rfcs#29 for more general instructions about backporting updates to declared-unstable-by-upstream software :)

@Ekleog Ekleog mentioned this pull request Aug 2, 2018
3 tasks
@Ralith
Copy link
Contributor

Ralith commented Aug 2, 2018

It's probably worth note that the synapse team tries decently hard to maintain backwards compatibility, as real-world synapse deployments inherently must interoperate with a wide variety of clients and servers. As such, breaking updates should be rare.

@Ekleog
Copy link
Member Author

Ekleog commented Aug 2, 2018

Indeed my rant was a bit heavy-handed, especially given between it (9 hours ago) and now, I have just discovered the same issue (that had already hit me twice) just started to appear again, with the latest version. So it's likely just that :matrix.org doesn't like my HS, and unrelated to updates… though the fact that until now updates were enough to fix the issue is weird!

@Ekleog
Copy link
Member Author

Ekleog commented Aug 4, 2018

Note: I've just learned the existence of tests for matrix-synapse, and despite this backport working in production on my server for two weeks, it appears that the NixOS test for matrix-synapse doesn't pass.

This is thus currently not ready for for merge. I'll fix it if the outcome of NixOS/rfcs#29 is that such a backport should be merged :)

@Ekleog Ekleog changed the title matrix-synapse: 0.31.2 -> 0.33.0 (WIP) matrix-synapse: 0.31.2 -> 0.33.0 Aug 4, 2018
@fadenb
Copy link
Contributor

fadenb commented Aug 20, 2018

Note: I've just learned the existence of tests for matrix-synapse, and despite this backport working in production on my server for two weeks, it appears that the NixOS test for matrix-synapse doesn't pass.

I applied your patch on top of f45cefe and the tests seem to pass just fine. What errors did you see?
I did not properly apply it and tested against the old version. I also see errors with the tests.

@Ekleog
Copy link
Member Author

Ekleog commented Aug 20, 2018

On the original branch I had loops of tracebacks for ValueError: Server name 'server_postgres' contains invalid characters (and same for server_sqlite). I stopped the tests after ~30s, from the idea that this error was quite unlikely to be transient.

My internet connection has apparently decided it doesn't like me today so I can't just rebase on latest 18.03, but if it works now, great! This PR is unfortunately still blocked on defining what exactly should be backported, which should be decided in NixOS/rfcs#29 (comment). I personally feel it should, but… :)

@pacien
Copy link
Contributor

pacien commented Sep 6, 2018

Synapse 0.33.3.1 is a critical security update that should probably be back-ported.

@Ekleog
Copy link
Member Author

Ekleog commented Sep 20, 2018

@vcunat

I do have a WIP section for the manual. I'll polish it and ping you soon.

Do you have any news in this domain? :)

@Ekleog
Copy link
Member Author

Ekleog commented Sep 24, 2018

Closing as:

  • 18.03 is close to being EOL'd anyway, so having clear guidelines for upstream-unstable software before EOL seems more and more unlikely
  • 0.33.0 is older than the current version in 18.03 anyway, so this would be useless (following a security fix in synapse 0.33.3.1)

@Ekleog Ekleog closed this Sep 24, 2018
@Ekleog Ekleog mentioned this pull request Sep 24, 2018
2 tasks
@vcunat
Copy link
Member

vcunat commented Oct 6, 2018

In the meantime we had some discussion on a "formal" proposal though perhaps without a clear conclusion so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants