0

I have a client server model where the client runs on android. It establishes its tls sockets using the following code:.

(Everything the client does to login and relogin)

public class LoginAsync extends AsyncTask<Boolean, String, Boolean>
protected Boolean doInBackground(Boolean... params)
{
    try
    {
        //only handle 1 login request at a time
        synchronized(loginLock)
        {
            if(tryingLogin)
            {
                Utils.logcat(Const.LOGW, tag, "already trying a login. ignoring request");
                onPostExecute(false);
                return false;
            }
            tryingLogin = true;
        }


        //http://stackoverflow.com/a/34228756
        //check if server is available first before committing to anything
        //  otherwise this process will stall. host not available trips timeout exception
        Socket diag = new Socket();
        diag.connect(new InetSocketAddress(Vars.serverAddress, Vars.commandPort), TIMEOUT);
        diag.close();

        //send login command
        Vars.commandSocket = Utils.mkSocket(Vars.serverAddress, Vars.commandPort, Vars.expectedCertDump);
        String login = Utils.currentTimeSeconds() + "|login|" + uname + "|" + passwd;
        Vars.commandSocket.getOutputStream().write(login.getBytes());

        //read response
        byte[] responseRaw = new byte[Const.BUFFERSIZE];
        int length = Vars.commandSocket.getInputStream().read(responseRaw);

        //on the off chance the socket crapped out right from the get go, now you'll know
        if(length < 0)
        {
            Utils.logcat(Const.LOGE, tag, "Socket closed before a response could be read");
            onPostExecute(false);
            return false;
        }

        //there's actual stuff to process, process it!
        String loginresp = new String(responseRaw, 0, length);
        Utils.logcat(Const.LOGD, tag, loginresp);

        //process login response
        String[] respContents = loginresp.split("\\|");
        if(respContents.length != 4)
        {
            Utils.logcat(Const.LOGW, tag, "Server response imporoperly formatted");
            onPostExecute(false); //not a legitimate server response
            return false;
        }
        if(!(respContents[1].equals("resp") && respContents[2].equals("login")))
        {
            Utils.logcat(Const.LOGW, tag, "Server response CONTENTS imporperly formated");
            onPostExecute(false); //server response doesn't make sense
            return false;
        }
        long ts = Long.valueOf(respContents[0]);
        if(!Utils.validTS(ts))
        {
            Utils.logcat(Const.LOGW, tag, "Server had an unacceptable timestamp");
            onPostExecute(false);
            return false;
        }
        Vars.sessionid = Long.valueOf(respContents[3]);

        //establish media socket
        Vars.mediaSocket = Utils.mkSocket(Vars.serverAddress, Vars.mediaPort, Vars.expectedCertDump);
        String associateMedia = Utils.currentTimeSeconds() + "|" + Vars.sessionid;
        Vars.mediaSocket.getOutputStream().write(associateMedia.getBytes());

        Intent cmdListenerIntent = new Intent(Vars.applicationContext, CmdListener.class);
        Vars.applicationContext.startService(cmdListenerIntent);

        onPostExecute(true);
        return true;
    }
    catch (CertificateException c)
    {
        Utils.logcat(Const.LOGE, tag, "server certificate didn't match the expected");
        onPostExecute(false);
        return false;
    }
    catch (Exception i)
    {
        Utils.dumpException(tag, i);
        onPostExecute(false);
        return false;
    }
}

with the mksocket utility function being:

public static Socket mkSocket(String host, int port, final String expected64) throws CertificateException
{
    TrustManager[] trustOnlyServerCert = new TrustManager[]
    {new X509TrustManager()
            {
                @Override
                public void checkClientTrusted(X509Certificate[] chain, String alg)
                {
                }

                @Override
                public void checkServerTrusted(X509Certificate[] chain, String alg) throws CertificateException
                {
                    //Get the certificate encoded as ascii text. Normally a certificate can be opened
                    //  by a text editor anyways.
                    byte[] serverCertDump = chain[0].getEncoded();
                    String server64 = Base64.encodeToString(serverCertDump, Base64.NO_PADDING & Base64.NO_WRAP);

                    //Trim the expected and presented server ceritificate ascii representations to prevent false
                    //  positive of not matching because of randomly appended new lines or tabs or both.
                    server64 = server64.trim();
                    String expected64Trimmed = expected64.trim();
                    if(!expected64Trimmed.equals(server64))
                    {
                        throw new CertificateException("Server certificate does not match expected one.");
                    }

                }

                @Override
                public X509Certificate[] getAcceptedIssuers()
                {
                    return null;
                }

            }
    };
    try
    {
        SSLContext context;
        context = SSLContext.getInstance("TLSv1.2");
        context.init(new KeyManager[0], trustOnlyServerCert, new SecureRandom());
        SSLSocketFactory mkssl = context.getSocketFactory();
        Socket socket = mkssl.createSocket(host, port);
        socket.setKeepAlive(true);
        return socket;
    }
    catch (Exception e)
    {
        dumpException(tag, e);
        return null;
    }
}

Here is the command listener service that gets started on successful login:

    public class CmdListener extends IntentService
protected void onHandleIntent(Intent workIntent)
{
    //  don't want this to catch the login resposne
    Utils.logcat(Const.LOGD, tag, "command listener INTENT SERVICE started");

    while(inputValid)
    {
        String logd = ""; //accumulate all the diagnostic message together to prevent multiple entries of diagnostics in log ui just for cmd listener
        try
        {//the async magic here... it will patiently wait until something comes in

            byte[] rawString = new byte[Const.BUFFERSIZE];
            int length = Vars.commandSocket.getInputStream().read(rawString);
            if(length < 0)
            {
                throw new Exception("input stream read failed");
            }
            String fromServer = new String(rawString, 0, length);
            String[] respContents = fromServer.split("\\|");
            logd = logd +  "Server response raw: " + fromServer + "\n";

            //check for properly formatted command
            if(respContents.length != 4)
            {
                Utils.logcat(Const.LOGW, tag, "invalid server response");
                continue;
            }

            //verify timestamp
            long ts = Long.valueOf(respContents[0]);
            if(!Utils.validTS(ts))
            {
                Utils.logcat(Const.LOGW, tag, "Rejecting server response for bad timestamp");
                continue;
            }

            //just parse and process commands here. not much to see

        }
        catch (IOException e)
        {
            Utils.logcat(Const.LOGE, tag, "Command socket closed...");
            Utils.dumpException(tag, e);
            inputValid = false;
        }
        catch(NumberFormatException n)
        {
            Utils.logcat(Const.LOGE, tag, "string --> # error: ");
        }
        catch(NullPointerException n)
        {
            Utils.logcat(Const.LOGE, tag, "Command socket null pointer exception");
            inputValid = false;
        }
        catch(Exception e)
        {
            Utils.logcat(Const.LOGE, tag, "Other exception");
            inputValid = false;
        }
    }
    //only 1 case where you don't want to restart the command listener: quitting the app.
    //the utils.quit function disables BackgroundManager first before killing the sockets
    //that way when this dies, nobody will answer the command listener dead broadcast

    Utils.logcat(Const.LOGE, tag, "broadcasting dead command listner");
    try
    {
        Intent deadBroadcast = new Intent(Const.BROADCAST_BK_CMDDEAD);
        sendBroadcast(deadBroadcast);
    }
    catch (Exception e)
    {
        Utils.logcat(Const.LOGE, tag, "couldn't broadcast dead command listener... leftover broadacast from java socket stupidities?");
        Utils.dumpException(tag, e);
    }
}

And here is the background manager that signs you in when you switch from wifi to lte, lte to wifi, or when you come out of the subway from nothing to lte:

public class BackgroundManager extends BroadcastReceiver
{
private static final String tag = "BackgroundManager";

@Override
public void onReceive(final Context context, Intent intent)
{
    if(Vars.applicationContext == null)
    {
        //sometimes intents come in when the app is in the process of shutting down so all the contexts won't work.
        //it's shutting down anyways. no point of starting something
        return;
    }

    AlarmManager manager = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);

    if(Vars.uname == null || Vars.passwd == null)
    {
        //if the person hasn't logged in then there's no way to start the command listener
        //  since you won't have a command socket to listen on
        Utils.logcat(Const.LOGW, tag, "user name and password aren't available?");
    }

    String action = intent.getAction();
    if(action.equals(ConnectivityManager.CONNECTIVITY_ACTION))
    {
        manager.cancel(Vars.pendingRetries);
        new KillSocketsAsync().execute();

        if(Utils.hasInternet())
        {
            //internet reconnected case
            Utils.logcat(Const.LOGD, tag, "internet was reconnected");
            new LoginAsync(Vars.uname, Vars.passwd).execute();
        }
        else
        {
            Utils.logcat(Const.LOGD, tag, "android detected internet loss");
        }
        //command listener does a better of job of figuring when the internet died than android's connectivity manager.
        //android's connectivity manager doesn't always get subway internet loss
    }
    else if (action.equals(Const.BROADCAST_BK_CMDDEAD))
    {
        String loge = "command listener dead received\n";

        //cleanup the pending intents and make sure the old sockets are gone before making new ones
        manager.cancel(Vars.pendingRetries);
        new KillSocketsAsync().execute(); //make sure everything is good and dead

        //all of this just to address the stupid java socket issue where it might just endlessly die/reconnect
        //initialize the quick dead count and timestamp if this is the first time
        long now = System.currentTimeMillis();
        long deadDiff =  now - Vars.lastDead;
        Vars.lastDead = now;
        if(deadDiff < Const.QUICK_DEAD_THRESHOLD)
        {
            Vars.quickDeadCount++;
            loge = loge + "Another quick death (java socket stupidity) occured. Current count: " + Vars.quickDeadCount + "\n";
        }

        //with the latest quick death, was it 1 too many? if so restart the app
        //https://stackoverflow.com/questions/6609414/how-to-programatically-restart-android-app
        if(Vars.quickDeadCount == Const.QUICK_DEAD_MAX)
        {
            loge = loge + "Too many quick deaths (java socket stupidities). Restarting the app\n";
            Utils.logcat(Const.LOGE, tag, loge);
            //self restart, give it a 5 seconds to quit
            Intent selfStart = new Intent(Vars.applicationContext, InitialServer.class);
            int pendingSelfId = 999;
            PendingIntent selfStartPending = PendingIntent.getActivity(Vars.applicationContext, pendingSelfId, selfStart, PendingIntent.FLAG_CANCEL_CURRENT);
            manager.set(AlarmManager.RTC_WAKEUP, System.currentTimeMillis()+Const.RESTART_DELAY, selfStartPending);

            //hopefully 5 seconds will be enough to get out
            Utils.quit();
            return;
        }
        else
        { //app does not need to restart. still record the accumulated error messages
            Utils.logcat(Const.LOGE, tag, loge);
        }

        //if the network is dead then don't bother
        if(!Utils.hasInternet())
        {
            Utils.logcat(Const.LOGD, tag, "No internet detected from commnad listener dead");
            return;
        }

        new LoginAsync(Vars.uname, Vars.passwd).execute();
    }
    else if (action.equals(Const.ALARM_ACTION_RETRY))
    {
        Utils.logcat(Const.LOGD, tag, "login retry received");

        //no point of a retry if there is no internet to try on
        if(!Utils.hasInternet())
        {
            Utils.logcat(Const.LOGD, tag, "no internet for sign in retry");
            manager.cancel(Vars.pendingRetries);
            return;
        }

        new LoginAsync(Vars.uname, Vars.passwd).execute();

    }
    else if(action.equals(Const.BROADCAST_LOGIN_BG))
    {
        boolean ok = intent.getBooleanExtra(Const.BROADCAST_LOGIN_RESULT, false);
        Utils.logcat(Const.LOGD, tag, "got login result of: " + ok);

        Intent loginResult = new Intent(Const.BROADCAST_LOGIN_FG);
        loginResult.putExtra(Const.BROADCAST_LOGIN_RESULT, ok);
        context.sendBroadcast(loginResult);

        if(!ok)
        {
            Utils.setExactWakeup(Const.RETRY_FREQ, Vars.pendingRetries);
        }
    }
}
}

The server is on a select system call to listen to its established sockets. It accepts new sockets using this code (C on Linux)

            incomingCmd = accept(cmdFD, (struct sockaddr *) &cli_addr, &clilen);
        if(incomingCmd < 0)
        {
            string error = "accept system call error";
            postgres->insertLog(DBLog(Utils::millisNow(), TAG_INCOMINGCMD, error, SELF, ERRORLOG, DONTKNOW, relatedKey));
            perror(error.c_str());
            goto skipNewCmd;
        }
        string ip = inet_ntoa(cli_addr.sin_addr);

        //setup ssl connection
        SSL *connssl = SSL_new(sslcontext);
        SSL_set_fd(connssl, incomingCmd);
        returnValue = SSL_accept(connssl);

        //in case something happened before the incoming connection can be made ssl.
        if(returnValue <= 0)
        {
            string error = "Problem initializing new command tls connection from " + ip;
            postgres->insertLog(DBLog(Utils::millisNow(), TAG_INCOMINGCMD, error, SELF, ERRORLOG, ip, relatedKey));
            SSL_shutdown(connssl);
            SSL_free(connssl);
            shutdown(incomingCmd, 2);
            close(incomingCmd);
        }
        else
        {
            //add the new socket descriptor to the client self balancing tree
            string message = "new command socket from " + ip;
            postgres->insertLog(DBLog(Utils::millisNow(), TAG_INCOMINGCMD, message, SELF, INBOUNDLOG, ip, relatedKey));
            clientssl[incomingCmd] = connssl;
            sdinfo[incomingCmd] = SOCKCMD;
            failCount[incomingCmd] = 0;
        }

The problem I'm having is when the client reconnects to the server from an ip address it has used recently, the socket on the client always seems to die after creation. If I retry again, it dies again. The only way to get it to connect is for the android app to kill and restart itself.

Example: on wifi at home with address 192.168.1.101. Connection ok. Switch to LTE on address 24.157.18.90. Reconnects me to the server ok. Come back home and get 192.168.1.101. The socket always dies until the app kills itself. Or if while I'm outside, I loose LTE because I take the subway, when I come out, I get the same problem. Note that each time, it will make a new socket. It will not somehow try to salvage the old one. The socket creation also seems to succeed. It's just as soon as the client wants to do a read on it, java says the socket is closed.

I put all the relevant code in its unobfuscated original form since it's my hobby project. I am out of ideas why this happens.

Armali
  • 18,255
  • 14
  • 57
  • 171
AAccount
  • 49
  • 7
  • Define 'socket dies'. – user207421 Dec 15 '16 at 01:50
  • Do you close the connection on the server side? Do you close any connection? – Iharob Al Asimi Dec 15 '16 at 02:02
  • Socket dies as in, select system call marks it as ready to read but when doing SSL_read it returns 0. It's dead. After the client logs in, the server checks if there were previously opened sockets and it does the whole procedure of ssl_shutdown, ssl_free, free, close on the socket and its openssl object. – AAccount Dec 15 '16 at 04:17
  • Reads returning zero indicates that the peer has closed the connection. Not just that a 'socket dies'. – user207421 Dec 15 '16 at 04:32
  • Is there a reason why the client would close the connection after reconnecting to the server with an ip it had in the past? As soon as the android client realizes it looses internet it automatically closes its sockets. Then when internet comes back, it will try to connect again. – AAccount Dec 15 '16 at 05:10
  • Impossible to say what your client is doing. You haven't posted much of it. – user207421 Dec 15 '16 at 06:26
  • OK, I put everything related to how the android client logs in, the background listener it starts and how it keeps track of the internet state and when to sign you back on. – AAccount Dec 15 '16 at 20:33
  • A side question: _why_ are you explicitly calling `onPostExecute` from `doInBackground`?? – Aleks G Dec 15 '16 at 21:03
  • 1. A Java `read()` returning -1 is not a 'read failure', it is a peer disconnect, and you should close the socket at that point. 2. Instead of littering your code with comments about 'Java socket stupidities' it would be somewhat better to show some evidence that you actually understood what the problem was at each such point. – user207421 Dec 15 '16 at 21:38
  • I am not sure what the problem is. That's why I'm asking. #1 sounds like a good idea. From what little they did teach in University about sockets, I am setting it up and taking it down as described. The rest is just android and command parsing. It's also not like I'm waiting forever to close the sockets as it's done when BackgroundReceiver is notified the listener is dead. Didn't think that small difference in timing was such a big deal but I've been known to make bad assumptions... many times. – AAccount Dec 16 '16 at 02:30

1 Answers1

0
    // http://stackoverflow.com/a/34228756
    //check if server is available first before committing to anything
    //  otherwise this process will stall. host not available trips timeout exception
    Socket diag = new Socket();
    diag.connect(new InetSocketAddress(Vars.serverAddress, Vars.commandPort), TIMEOUT);
    diag.close();

It is caused by these three pointless lines of code. The server gets a connection and an immediate read() result of zero.

There is no value in establishing a connection only to close it and then assume you can open another one. You should use the conection you just established. In general the correct way to establish whether any resource is available is to try to use it in the normal way. Techniques like the above are indistinguishable from attempts to predict the future.

user207421
  • 305,947
  • 44
  • 307
  • 483
  • I've tried removing that part with no change in behavior. I'll permanently remove it if it's that bad. This is my first time trying to make a usable client server program. Barely learned anything in university to make this. Most of this is self taught. – AAccount Dec 16 '16 at 02:22