hippware / wocky

Server side (Erlang/Elixir) software
13 stars 3 forks source link

XMPP TLS encryption discussion #804

Closed bengtan closed 7 years ago

bengtan commented 7 years ago

The app uses a native macOS/iOS library (https://github.com/robbiehanson/XMPPFramework). I had a look in it's code, in particular, at where it creates the TCP socket. This is hardcoded/'nailed up' and there are no extensible hooks or interfaces so there's no way to interject an existing (web- or ssl-) socket for it to use.

Hence, no support for anything other than a vanilla TCP socket (with in-built support for STARTTLS).

This is one of those cases where it is possible to modify the library to support (web- or ssl-) sockets but requires patching and, probably, maintaining our own fork. Considering this is not our forte, it could take us weeks to do it.

Furthermore, I don't think there's any appetite on the client-side to spend the time on this.

So, I would put this in the 'possible but probably don't want to' basket ... for now.


I came across the idea of terminating XMPP's STARTTLS on a proxy server. It turns out that the initial negotiation/setup of an xmpp connection is fairly boilerplate-ish and doesn't necessarily require a full xml parser nor xmpp engine.

There are some anecdotes of this online.

Someone apparently managed to configure a F5 loadbalancer to terminate XMPP/STARTTLS:

https://community.igniterealtime.org/thread/52069 https://devcentral.f5.com/questions/xmpp-with-starttls

Someone did the same with nginx (not sure whether through config/script or a binary module):

https://robn.io/nginx-xmpp

and then there's this one (this is the least informative of the three):

https://support.radware.com/app/answers/answer_view/a_id/16384/~/using-an-appshape%2B%2B-script-to-protect-xmpp-servers-against-ssl-vulnerabilities

Is this an option worth exploring?

If someone can do it with config/script, I'm sure we can do it too ... so it becomes a question of ... Are the benefits worth the trouble?

toland commented 7 years ago

Someone apparently managed to configure a F5 loadbalancer to terminate XMPP/STARTTLS

I have some experience with F5 devices from my time at Rackspace. The F5s are essentially just Linux boxes running specialized packet filtering and routing software. This means that they can be incredibly flexible and customizable. I don't think that the solution described at those links is generally applicable to other platforms, and I don't think that the F5 BigIP is an appropriate solution for us.

Someone did the same with nginx (not sure whether through config/script or a binary module)

The Nginx work was done on a fork, which is now unmaintained. There are quite a few caveats with that work. Most notably, it only supports plain text auth since the auth is done in Nginx and apparently does not support stream resumption. I do not relish the idea of maintaining a Nginx fork to get this kind of functionality. I don't think the work would be transferable to haproxy (which we are currently using for load balancing) and certainly wouldn't be usable with ELB.

and then there's this one (this is the least informative of the three)

This solution is specific to Radware's product, which is overkill for us at this point.

Is this an option worth exploring?

I don't think so. Even in the cases where it has been made to work, the implementation is partial and there are significant caveats. The effort to get stream initiation and STARTTLS working on the load balancer certainly far outweighs the work to get the SSL certs deployed on the app containers and letting MIM handle STARTTLS.

bengtan commented 7 years ago

I thought of those links as proof-of-concepts that it could be done ... but fair enough, if it's more work than necessary, then let's not go there.

There is one more thing I'm getting Pavel to investigate [1] ... which is whether we can use 'wrapped ssl' (ie. traditional) encryption rather than starttls encryption.

Stay tuned.

[1] https://github.com/hippware/rn-chat/issues/342#issuecomment-310657252

bengtan commented 7 years ago

Some new developments since last posting.

'Old school' SSL connections work. (Aka Direct TLS, 'wrapped ssl')

This has been confirmed by @aksonov by connecting to the Production server.

There is a issue with the Staging server because of a certificate mismatch (#820).

There is agreement that 'old school' ssl is the way to go, so we now have to decide what to do about SSL for the Staging server.

Possibilities (Staging only):

A) Acquire a SSL certificate for staging.dev.tinyrobot.com. Either purchase one outright or move to Amazon LB and get a free cert. B) Disable client-side checking of the server certificate for Staging. C) Relax or modify client-side checking of the server certificate for Staging so it passes. D) Rename the xmpp domain to staging.dev.tinyrobot.com to staging.prod.tinyrobot.com.

A is a quick fix but D is a better fix

B is strongly discouraged by @toland (and @bernardd?)

C is so-so.

D is ... needs more thought.

D is the ideal solution but it could get complicated because the domain name staging.dev.tinyrobot.com appears in many many places in the database.

There is some discussion at https://github.com/hippware/wocky/issues/820#issuecomment-312052904

(Furthermore, the Staging client probably needs to be uninstalled and reinstalled.)


For now, let's scope out D and then re-evaluate.

bengtan commented 7 years ago

Quoting from https://github.com/hippware/rn-chat/issues/342#issuecomment-311920742

Ok, I've corrected verification algorithm and now it checks hostname but not JID domain. I will update our custom react-native-xmpp fork, so we could use both 5222/5223 (in case of 5223 'old school' connection will happen)

@aksonov:

You did what?

If I understand you properly, that breaks the spec ... (calms down :) ) although I guess it's a possible short term fix.

Can you point me to the code? Curious to have a look.

bernardd commented 7 years ago

B) Disable client-side checking of the server certificate for Staging. B is strongly discouraged by @toland (and @bernardd?)

I think it's an appropriate course only as a short term fix to allow other work to continue. It's not something we want to do long term.

bengtan commented 7 years ago

Part of the reason for choosing D is the notion that having the server hostname be different from the xmpp domainpart is going to cause us other, unknown, unrelated problems in the future ie. better to fix it now than later.

I've had second thoughts about this notion/assumption.

XMPP explicitly supports mismatching server hostnames and xmpp domainparts. It's even in xmpp's DNS SRV and fail-over support.

So I think this assumption is mostly false.

aksonov commented 7 years ago

@bengtan I did following:

- (void)xmppStream:(XMPPStream *)sender willSecureWithSettings:(NSMutableDictionary *)settings
{
    DDLogVerbose(@"%@: %@", THIS_FILE, THIS_METHOD);

    NSString *expectedCertName = [xmppStream hostName];
    if (expectedCertName)
    {
        settings[(NSString *) kCFStreamSSLPeerName] = expectedCertName;
    }

    if (customCertEvaluation)
    {
        settings[GCDAsyncSocketManuallyEvaluateTrust] = @(YES);
    }
}

Before it uses [JID domain] instead of xmppStream hostName. I don't see any problem with it - it is not part of XMPPFramework and designed to be configurable (willSecureWithSettings). It is inside forked hippware/react-native-xmpp now

aksonov commented 7 years ago

https://github.com/hippware/react-native-xmpp/commit/6d16e40dd0f1bdaa25420a98101c9e5c13fc58a6

bengtan commented 7 years ago

@aksonov:

Oh very good. I also noticed didReceiveTrust() which is even more powerful.

I suspected something like that but it's even better than I thought.

@toland, @bernardd:

react-native-xmpp is a React Native component that bridges React Native and the native XMPPFramework. aksonov maintains it, and we also have a hippware fork.

The component has hooks that allow callback of custom logic to evaluate the SSL certificate. We can implement almost any arbitrary certificate validation logic that we want.

aksonov has modified it so that, if the port is 5223, then use wrapped SSL and validate the certificate against the server hostname instead of the XMPP domainpart. This does not comply with the XMPP RFC but ... it's probably good enough for now ... and may be good enough for a long time too.

This approach is like C (mentioned above) but it's not any less secure since it still does validate the certificate.

I would suggest we don't have to proceed with D anymore ... if we can use this approach (or some variation of it).

bengtan commented 7 years ago

After discussion, it has been decided that wrapped SSL with certificate validation against the server hostname instead of the xmpp domainpart is sufficient for now.

It's not any less secure.

It's not compliant with the XMPP RFC but we can revisit and change it when compliance becomes important.


We are still keeping open the possibility of D:

Rename Staging xmpp domainpart from staging.dev to staging.prod #836 https://github.com/hippware/wocky/issues/836

but it is no longer time sensitive.


So, if we are decided on using wrapped SSL on port 5223, should we close off (unencrypted) port 5222, or just leave it as-is?

bernardd commented 7 years ago

So, if we are decided on using wrapped SSL on port 5223, should we close off (unencrypted) port 5222, or just leave it as-is?

If we're certain the client will support it SSL on 5223, we should absolutely shut off 5222.

bengtan commented 7 years ago

Alright, I think this discussion is finished. Closing but feel free to re-open if anyone wants to raise any concerns.

If we're certain the client will support it SSL on 5223, we should absolutely shut off 5222.

Spun out as #838.