Projects tigase _server server-core Issues #1199
Don't send any packets until stream negotiation is finished (#1199)
Closed
wojciech.kapcia@tigase.net opened 4 years ago

As per comments in servers-319 and Completion of Stream Negotiation specification:

The receiving entity indicates completion of the stream negotiation process by sending to the initiating entity either an empty <features/> element or a <features/> element that contains only voluntary-to-negotiate features. After doing so, the receiving entity MAY send an empty <features/> element (e.g., after negotiation of such voluntary-to-negotiate features) but MUST NOT send additional stream features to the initiating entity (if the receiving entity has new features to offer, preferably limited to mandatory-to-negotiate or security-critical features, it can simply close the stream with a <reset/> stream error (Section 4.9.3.16) and then advertise the new features when the initiating entity reconnects, preferably closing existing streams in a staggered way so that not all of the initiating entities reconnect at once). Once stream negotiation is complete, the initiating entity is cleared to send XML stanzas over the stream for as long as the stream is maintained by both parties.

The initiating entity MUST NOT attempt to send XML stanzas to entities other than itself (i.e., the client's connected resource or any other authenticated resource of the client's account) or the server to which it is connected until stream negotiation has been completed. Even if the initiating entity does attempt to do so, the receiving entity MUST NOT accept such stanzas and MUST close the stream with a <not-authorized/> stream error (Section 4.9.3.12). This rule applies to XML stanzas only (i.e., <message/>, <presence/>, and <iq/> elements qualified by the content namespace) and not to XML elements used for stream negotiation (e.g., elements used to complete TLS negotiation or SASL negotiation).

I would say that stream--stream-feature is complete negotiation cycle, so we MUST wait for the features before transmitting anything.

(I wouldn't enforce strictly the last part about closing the stream/connection)

wojciech.kapcia@tigase.net commented 3 years ago

Implemented: https://github.com/tigase/tigase-server/pull/132

Andrzej, could you please take a look and verify?

wojciech.kapcia@tigase.net commented 3 years ago

merged, to be verified on live deployment.

wojciech.kapcia@tigase.net commented 3 years ago

tigase.org and tigase.im updated, seems to be working fine.

Andrzej Wójcik (Tigase) commented 3 years ago

From our support chat:

[2022-02-16 21:36:04] : Hey, any changes on the sure.im (and related domains) server in the past couple of days? [2022-02-16 21:36:50] : We noticed very frequent s2s drops with xmpp.org, and now it's also been discovered on cheogram.com. Both are running Prosody. [2022-02-16 21:37:44] : I haven't done much digging yet, though Zash looked into the xmpp.org case a little we haven't identified exactly what's happening yet [2022-02-17 10:12:21] : MattJ, there were new builds deployed with some changes in s2s connectivity but the last one with final fix was deployed around 10th [2022-02-17 10:12:40] : Hmm, the issues have definitely been after that

wojciech.kapcia@tigase.net commented 3 years ago

Connection timer wasn't cancelled in incoming connections after authentication/completion negotiation. Fixed.

issue 1 of 1
Type
Bug
Priority
Normal
Assignee
Version
tigase-server-8.2.0
Spent time
14h 30m
Subsystem
s2s
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#1199
Please wait...
Page is in error, reload to recover