Refactored JobServClient into multiple modules

src/main/java/JobServ/JobServClient.java

@@ -8,321 +8,54 @@
 
 package JobServ;
 
-import io.grpc.ManagedChannel;
-import io.grpc.ManagedChannelBuilder;
-import io.grpc.StatusRuntimeException;
 import io.grpc.netty.GrpcSslContexts;
-import io.grpc.netty.NettyChannelBuilder;
+import io.grpc.ManagedChannel;
+import java.util.InputMismatchException;
+import io.grpc.ManagedChannelBuilder;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
-
 import javax.net.ssl.SSLException;
-import java.io.File;
-import java.util.InputMismatchException;
-import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+import io.grpc.netty.NettyChannelBuilder;
 import java.util.Scanner;
+import java.io.File;
 
 /*
  * The JobServClient class extends the gRPC stub code
  * Additionally, it plugs a command line interface into the API code.
  */
 public class JobServClient {
+    private final String serversideTimeoutErrorMessage = "Timeout locking process control on server\n"+
+        "Server could be under heavy load\nConsider trying again.";
 
-    /*
-     * The client should not use the same logging module as the server.
-     * In a more robust product the server logging module will take advantage of system level
-     * log aggregators such as journalctl, which the client should not be writing to on the users system
-     */
-    private static final Logger logger = Logger.getLogger(JobServClient.class.getName());
-
-    private final ManagedChannel channel;
-
-    /*
-     * blockingStub is used when the client needs to block until the server responds
-     * the client doesnt nessesarily need to support asynchronously firing off commands
-     * in this shell-like interface it would be disconcerting to get multiple returns out of order
-     */
-    private final ShellServerGrpc.ShellServerBlockingStub blockingStub;
+    private JobServClientAPIConnector api;
+    private String[] programArgs;
 
     /*
      * Constructor
-     * Spawns a new blockingStub for network operations with the server
+     * takes program arguments and an api connector object
      */
-    public JobServClient(ManagedChannel channel) {
-        this.channel = channel;
-        blockingStub = ShellServerGrpc.newBlockingStub(this.channel);
+    public JobServClient(String[] args, JobServClientAPIConnector api) {
+        this.programArgs = args;
+        this.api = api;
     }
 
-    /*
-     * shutdown()
-     * Gets called when you press cntrl+c
-     * takes at most 5 seconds to close its connection
-     */
-    public void shutdown() throws InterruptedException {
-        channel.shutdown().awaitTermination(5, TimeUnit.SECONDS);
-    }
-
-    /*
-     * getProcessInfo()
-     * sends the server a request for output from the process identified by 'pid'
-     * returns process output as string
-     */
-    public String getProcessOutput(int pid) {
-        logger.info("[+] requesting output");
-
-        PIDMessage request = PIDMessage.newBuilder()
-            .setPid(pid)
-            .build();
-        OutputMessage response;
-
-        try {
-            // blocking network operation
-            response = blockingStub.getOutput(request);
-        } catch (StatusRuntimeException e) {
-            logger.log(Level.WARNING, "(API Failure) Request for output failed: " + e.getStatus());
-            return "<Error connecting to API>";
-        }
-
-        return response.getOutput();
-    }
-
-    /*
-     * sendNewJobMessage()
-     * sends a shell command to the api server
-     * returns new pid of job
-     * or -1 if server failed to create job
-     * or -2 if failed to connect to API
-     */
-    public int sendNewJobMessage(String command) {
-        // thought of escaping this, but the vulnerability is only client side, from client user input.
-        logger.info("[+] Sending command to server");
-
-        NewJobMessage request = NewJobMessage.newBuilder()
-            .setCommand(command)
-            .build();
-        PIDMessage response;
-
-        try {
-            // blocking network operation
-            response = blockingStub.newJob(request);
-        } catch (StatusRuntimeException e) {
-            logger.log(Level.WARNING, "(API Failure) Request for new job failed: " + e.getStatus());
-            return -2;
-        }
-
-        if(response.getPid() == -1) {
-            logger.log(Level.WARNING, "New job creation failed server side!");
-        }
-
-        return response.getPid();
-    }
-
-    /*
-     * getProcessStatus()
-     * requests running status of process pid
-     * returns true if process still running else false
-     */
-    public Boolean getProcessStatus(int pid) {
-        logger.info("[+] Requesting status of a job");
-
-        PIDMessage request = PIDMessage.newBuilder()
-            .setPid(pid)
-            .build();
-        StatusMessage response;
-
-        try {
-            // blocking network operation
-            response = blockingStub.getStatus(request);
-        } catch (StatusRuntimeException e) {
-            logger.log(Level.WARNING, "(API Failure) Request for status failed: " + e.getStatus());
-            return false;
-        }
-
-        return response.getIsRunning();
-    }
-
-    /*
-     * sends PID to server
-     * returns process exit code
-     * returns a 0-255 return code or 256 if still running
-     * or 257 if error in API
-     */
-    public int getProcessReturn(int pid) {
-        logger.info("[+] Requesting return code of a job");
-
-        PIDMessage request = PIDMessage.newBuilder()
-            .setPid(pid)
-            .build();
-        ReturnMessage response;
-
-        try {
-            // blocking network operation
-            response = blockingStub.getReturn(request);
-        } catch (StatusRuntimeException e) {
-            logger.log(Level.WARNING, "(API Failure) Failed to get return code: " + e.getStatus());
-            return 257;
-        }
-
-        return response.getProcessReturnCode();
-    }
-
-    /*
-     * killProcess()
-     * send a PID to be killed, function returns nothing
-     * logs warning if job status comes back still running
-     */
-    public void killProcess(int pid) {
-        logger.info("[+] Killing a job");
-
-        PIDMessage request = PIDMessage.newBuilder()
-            .setPid(pid)
-            .build();
-        StatusMessage response;
-
-        try {
-            // blocking network operation
-            response = blockingStub.killJob(request);
-        } catch (StatusRuntimeException e) {
-            logger.log(Level.WARNING, "(API Failure) Failed to send request: " + e.getStatus());
-            return;
-        }
-
-        if (response.getIsRunning()) {
-            logger.log(Level.WARNING, "[-] Server failed to kill job!");
-        }
-    }
-
-    /*
-     * main()
-     * Client entrypoint
-     * Parses arguments, initializes client, and calls the correct functions
-     */
-    public static void main(String[] args) throws Exception {
-
-        // check args
-        if (args.length < 7) {
-            System.out.println("Usage: $ ./jobserv-client privatekey, cert, truststore, host, port, command, args");
-            System.out.println("Or try $ ./jobserv-client help");
-            outputHelp();
-            return;
-        }
-
-        JobServClient client;
-        try {
-            SslContextBuilder builder = GrpcSslContexts.forClient();
-            builder.trustManager(new File(args[2]));
-            builder.keyManager(new File(args[1]), new File(args[0]));
-
-            ManagedChannel channel = NettyChannelBuilder.forAddress(args[3], Integer.parseInt(args[4]))
-                .sslContext(builder.build())
-                .build();
-            client = new JobServClient(channel);
-
-        // Likely bad port
-        } catch (NumberFormatException e) {
-            System.out.println("Invalid Port");
-            return;
-
-        // bad cert or key format
-        } catch (SSLException e) {
-            System.out.println(e.getMessage());
-            return;
-        }
-
-        // declare pid up here so that multiple switch cases can use it
-        int candidatePid;
-        // parse remaining args
-        switch (args[5]) {
-            case "new":
-                if (args.length < 6) {
-                    System.out.println("Improper formatting, try client --help");
-                    break;
-                }
-
-                String command = "";
-                for (int token = 6; token < args.length; token++) {
-                    command += " " + args[token];
-                }
-
-                int newProcess = client.sendNewJobMessage(command);
-                System.out.printf("Process started, assigned pid is %d\n", newProcess);
-                break;
-
-            case "output":
-                candidatePid = getPidArg(args, 6);
-                if (candidatePid < 0) {
-                    break;
-                }
-
-                String processOutput = client.getProcessOutput(candidatePid);
-                System.out.println(processOutput);
-                break;
-                
-            case "status":
-                candidatePid = getPidArg(args, 6);
-                if (candidatePid < 0) {
-                    break;
-                }
-
-                Boolean processStatus = client.getProcessStatus(candidatePid);
-                System.out.printf("Process is currently running? %b\n", processStatus);
-                break;
-
-            case "kill":
-                candidatePid = getPidArg(args, 6);
-                if (candidatePid < 0) {
-                    break;
-                }
-
-                client.killProcess(candidatePid);
-                System.out.println("End process request recieved!");
-                break;
-
-            case "return":
-                candidatePid = getPidArg(args, 6);
-                if (candidatePid < 0) {
-                    break;
-                }
-
-                int returnCode = client.getProcessReturn(candidatePid);
-
-                if (returnCode == 277) {
-                    System.out.println("Process is still running");
-                    break;
-
-                } else if (returnCode == 278) {
-                    System.out.println("RPC Call error!");
-                    break;
-
-                } else {
-                    System.out.printf("Process Exit Code: %d\n", returnCode);
-                }
-
-            default:
-                System.out.println("Improper command, try 'help'");
-                break;
-            }
-        }
-
     /*
      * getPidArg()
      * reentrant code was found in all commands except newjob
-     * this function pulls the pid argument and wraps around the integer case
+     * this function pulls the pid argument and wraps around the integer cast
      * returns -1 (an invalid PID) if bad index or unparsable int
      */
-    private static int getPidArg(String[] args, int index) {
-        if (args.length < index) {
+    private int getPidArg(int index) {
+        if (this.programArgs.length < index) {
             System.out.println("Improper formatting, try client --help");
             return -1;
         }
 
         try {
-            return Integer.parseInt(args[6]);
+            return Integer.parseInt(this.programArgs[index]);
 
         } catch (InputMismatchException e) {
-            System.out.println(args[6] + " is not a valid int, much less a valid pid");
+            System.out.println(this.programArgs[index] + " is not a valid int, much less a valid pid");
             return -1;
         }
 
@@ -344,4 +77,201 @@ public class JobServClient {
         System.out.println("... kill (pid)");
         System.out.println("Immediately destroys remote process");
     }
+
+    /*
+     * makeNewProcess
+     * makes a new process
+     */
+    public void makeNewProcess() {
+        if (this.programArgs.length < 6) {
+            System.out.println("Improper formatting, try client --help");
+            return;
+        }
+
+        String command = "";
+        for (int token = 6; token < this.programArgs.length; token++) {
+            command += " " + this.programArgs[token];
+        }
+
+        int newProcess = this.api.sendNewJobMessage(command);
+        // TODO: switch here for negative returns
+        System.out.printf("Process started, assigned pid is %d\n", newProcess);
+    }
+
+    /*
+     * getOutput
+     * gets output from a process
+     */
+    public void getOutput() {
+        int candidatePid = this.getPidArg(6);
+        if (candidatePid < 0) {
+            return;
+        }
+
+        String processOutput = this.api.getProcessOutput(candidatePid);
+        System.out.println(processOutput);
+    }
+
+    /*
+     * getStatus
+     * gets the running status of a process
+     */
+    public void getStatus() {
+        int candidatePid = this.getPidArg(6);
+        if (candidatePid < 0) {
+            return;
+        }
+
+        int processStatus = this.api.getProcessStatus(candidatePid);
+        switch(processStatus) {
+        case 0:
+            System.out.println("Process is running");
+            break;
+        case 1:
+            System.out.println("Process is not running");
+            break;
+        case 2:
+            System.out.println("A client killed the process already");
+            break;
+        case 3:
+            System.out.println("Process does not exist");
+            break;
+        case 4:
+            System.out.println(this.serversideTimeoutErrorMessage);
+            break;
+        }
+    }
+
+    /*
+     * killProcess
+     * kills a process
+     */
+    public void killProcess() {
+        int candidatePid = this.getPidArg(6);
+        if (candidatePid < 0) {
+            return;
+        }
+
+        int finalStatus = this.api.killProcess(candidatePid);
+        switch(finalStatus) {
+        case 0:
+            System.out.println("Process is still running");
+            break;
+
+        case 1:
+            System.out.println("Process was killed");
+            break;
+
+        case 2:
+            System.out.println("Process does not exist");
+            break;
+
+        case 3:
+            System.out.println(this.serversideTimeoutErrorMessage);
+            break;
+
+        case 4:
+            // error logged in API Connector
+            break;
+        }
+    }
+
+    /*
+     * getReturn
+     * gets return code from a process
+     */
+    public void getReturn() {
+        int candidatePid = this.getPidArg(6);
+        if (candidatePid < 0) {
+            return;
+        }
+
+        int returnCode = this.api.getProcessReturn(candidatePid);
+
+        switch(returnCode){
+        case 256:
+            System.out.println("Process is still running");
+            break;
+        case 257:
+            System.out.println("Process was killed manually by a client");
+            break;
+        case 258:
+            System.out.println("Process does not exist");
+            break;
+        case 259:
+            System.out.println(this.serversideTimeoutErrorMessage);
+            break;
+        case 260:
+            // error logged in getProcesReturn
+            break;
+        default:
+            System.out.println("Process Exit Code: " + Integer.toString(returnCode));
+        }
+    }
+
+    /*
+     * main()
+     * Client entrypoint
+     * Parses arguments, initializes client, and calls the correct functions
+     */
+    public static void main(String[] args) throws Exception {
+        // check args
+        if (args.length < 7) {
+            System.out.println("Usage: $ ./jobserv-client privatekey, cert, truststore, host, port, command, args");
+            System.out.println("Or try $ ./jobserv-client help");
+            outputHelp();
+            return;
+        }
+
+        JobServClientAPIConnector api;
+        try {
+            SslContextBuilder builder = GrpcSslContexts.forClient();
+            builder.trustManager(new File(args[2]));
+            builder.keyManager(new File(args[1]), new File(args[0]));
+
+            ManagedChannel channel = NettyChannelBuilder.forAddress(args[3], Integer.parseInt(args[4]))
+                .sslContext(builder.build())
+                .build();
+            api = new JobServClientAPIConnector(channel);
+
+        // Likely bad port
+        } catch (NumberFormatException e) {
+            System.out.println("Invalid Port");
+            return;
+
+        // bad cert or key format
+        } catch (SSLException e) {
+            System.out.println(e.getMessage());
+            return;
+        }
+
+        JobServClient client = new JobServClient(args, api);
+
+        // parse remaining args
+        switch (args[5]) {
+            case "new":
+                client.makeNewProcess();
+                break;
+
+            case "output":
+                client.getOutput();
+                break;
+
+            case "status":
+                client.getStatus();
+                break;
+
+            case "kill":
+                client.killProcess();
+                break;
+
+            case "return":
+                client.getReturn();
+                break;
+
+            default:
+                System.out.println("Improper command, try 'help'");
+                break;
+        }
+    }
 }

src/main/java/JobServ/JobServClientAPIConnector.java

@@ -0,0 +1,203 @@
+/*
+ * JobServClientAPIConnector
+ *
+ * v1.0
+ *
+ * May 23, 2019
+ */
+
+package JobServ;
+
+import io.grpc.ManagedChannel;
+import io.grpc.StatusRuntimeException;
+import java.io.File;
+import java.util.concurrent.TimeUnit;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/*
+ * JobServClientAPIConnector
+ * Starts a connection to the API Connector
+ * implements functions that send and recieve frm the API
+ * Refactored into its own module to make the Client interface nicer
+ * and to allow for a veriety of interfaces to be created
+ */
+class JobServClientAPIConnector {
+    private final String serversideTimeoutErrorMessage = "Timeout locking process control on server\n"+
+        "Server could be under heavy load\nConsider trying again.";
+    private final String apiFailureMessage = "Failed while trying to connect to server.";
+
+    /*
+     * The client should not use the same logging module as the server.
+     * In a more robust product the server logging module will take advantage of system level
+     * log aggregators such as journalctl, which the client should not be writing to on the users system
+     */
+    private static final Logger logger = Logger.getLogger(JobServClient.class.getName());
+
+    private final ManagedChannel channel;
+
+    /*
+     * blockingStub is used when the client needs to block until the server responds
+     * the client doesnt nessesarily need to support asynchronously firing off commands
+     * in this shell-like interface it would be disconcerting to get multiple returns out of order
+     */
+    private final ShellServerGrpc.ShellServerBlockingStub blockingStub;
+
+    /*
+     * Constructor
+     * Spawns a new blockingStub for network operations with the server
+     */
+    public JobServClientAPIConnector(ManagedChannel channel) {
+        this.channel = channel;
+        blockingStub = ShellServerGrpc.newBlockingStub(this.channel);
+    }
+
+    /*
+     * shutdown()
+     * Gets called when you press cntrl+c
+     * takes at most 5 seconds to close its connection
+     */
+    public void shutdown() throws InterruptedException {
+        channel.shutdown().awaitTermination(5, TimeUnit.SECONDS);
+    }
+
+    /*
+     * getProcessOutput()
+     * sends the server a request for output from the process identified by 'pid'
+     * returns process output as string
+     */
+    public String getProcessOutput(int pid) {
+        logger.info("[+] requesting output");
+
+        PIDMessage request = PIDMessage.newBuilder()
+            .setPid(pid)
+            .build();
+        OutputMessage response;
+
+        try {
+            // blocking network operation
+            response = blockingStub.getOutput(request);
+
+        } catch (StatusRuntimeException e) {
+            logger.log(Level.WARNING, this.apiFailureMessage + ": " + e.getStatus());
+            return "<Error connecting to API>";
+        }
+
+        return response.getOutput();
+    }
+
+    /*
+     * sendNewJobMessage()
+     * sends a shell command to the api server
+     * returns new pid of job
+     * or -1 if server thread couldnt synchronize before timeout
+     * or -2 if server failed to create job
+     * or -3 if client fails to connect
+     */
+    public int sendNewJobMessage(String command) {
+        // thought of escaping this, but the vulnerability is only client side, from client user input.
+        logger.info("[+] Sending command to server");
+
+        NewJobMessage request = NewJobMessage.newBuilder()
+            .setCommand(command)
+            .build();
+        PIDMessage response;
+
+        try {
+            // blocking network operation
+            response = blockingStub.newJob(request);
+
+        } catch (StatusRuntimeException e) {
+            logger.log(Level.WARNING,  this.apiFailureMessage + ": " + e.getStatus());
+            return -3;
+        }
+
+        return response.getPid();
+    }
+
+    /*
+     * getProcessStatus()
+     * requests running status of process pid
+     * 0: running
+     * 1: not running
+     * 2: killed manually by a client
+     * 3: doesnt exist
+     * 4: couldnt grab lock
+     */
+    public int getProcessStatus(int pid) {
+        logger.info("[+] Requesting status of a job");
+
+        PIDMessage request = PIDMessage.newBuilder()
+            .setPid(pid)
+            .build();
+        StatusMessage response;
+
+        try {
+            // blocking network operation
+            response = blockingStub.getStatus(request);
+
+        } catch (StatusRuntimeException e) {
+            logger.log(Level.WARNING, this.apiFailureMessage + ": " + e.getStatus());
+            return -1;
+        }
+
+        return response.getProcessStatus();
+    }
+
+    /*
+     * sends PID to server
+     * returns process exit code
+     * 0-255: process exit code
+     * 256: process still running
+     * 257: process was killed by a client
+     * 258: process doesnt exist
+     * 259: couldnt grab lock in time
+     * 260: couldnt connect to API
+     */
+    public int getProcessReturn(int pid) {
+        logger.info("[+] Requesting return code of a job");
+
+        PIDMessage request = PIDMessage.newBuilder()
+            .setPid(pid)
+            .build();
+        ReturnMessage response;
+
+        try {
+            // blocking network operation
+            response = blockingStub.getReturn(request);
+        } catch (StatusRuntimeException e) {
+            logger.log(Level.WARNING, this.apiFailureMessage + ": " + e.getStatus());
+            return 260;
+        }
+
+        return response.getProcessReturnCode();
+    }
+
+    /*
+     * killProcess()
+     * send a PID to be killed, function returns process status after kill operation
+     * returns 0 if still running
+     * returns 1 if process was killed
+     * returns 2 if process not found
+     * returns 3 if couldnt grab lock
+     * returns 4 on API failure
+     */
+    public int killProcess(int pid) {
+        logger.info("[+] Killing a job");
+
+        PIDMessage request = PIDMessage.newBuilder()
+            .setPid(pid)
+            .build();
+        StatusMessage response;
+
+        try {
+            // blocking network operation
+            response = blockingStub.killJob(request);
+        } catch (StatusRuntimeException e) {
+            logger.log(Level.WARNING, this.apiFailureMessage + ": " + e.getStatus());
+            return 4;
+        }
+
+        return response.getProcessStatus();
+    }
+}

src/main/java/JobServ/ProcessManager.java

@@ -177,7 +177,7 @@ class ProcessManager {
 
         } catch (TimeoutException e) {
             System.err.println("Timeout getting process output: " + e.getMessage());
-            return "[-] ERROR: Timeout grabbing lock to access process information";
+            return "[-] SERVER: Timeout grabbing lock to access process information";
         }
 
         ProcessController candidate = this.processMap.get(pid);

src/test/java/JobServ/JobServerAuthenticationTest.java

@@ -66,8 +66,8 @@ public class JobServerAuthenticationTest {
     private final String badCert = projectRoot + "resources/test/test.crt";
 
     // badClient uses unauthorized certs
-    private JobServClient goodClient;
-    private JobServClient badClient;
+    private JobServClientAPIConnector goodClient;
+    private JobServClientAPIConnector badClient;
     private JobServServer server;
 
     // was setUp able to use SSL Certs
@@ -120,8 +120,8 @@ public class JobServerAuthenticationTest {
                     .directExecutor()
                     .build();
 
-                goodClient = new JobServClient(goodChannel);
-                badClient = new JobServClient(badChannel);
+                goodClient = new JobServClientAPIConnector(goodChannel);
+                badClient = new JobServClientAPIConnector(badChannel);
                 this.clientSslInitialized = true;
 
             } catch (SSLException e) {
@@ -145,10 +145,10 @@ public class JobServerAuthenticationTest {
         assertEquals(true, clientSslInitialized);
 	
         int result = badClient.sendNewJobMessage("test command");
-        assertEquals(-2, result);
+        assertEquals(-3, result);
 
         result = goodClient.sendNewJobMessage("test command");
-        Boolean assertCondition = result == -2;
+        Boolean assertCondition = result == -3;
         assertEquals(assertCondition, false);
     }
 }