merging in changes from master

.gitlab-ci.yml

@@ -1,6 +1,29 @@
 image: java:8-jdk
+stages:
+  - build
+  - test
+  - deploy
+
 before_script:
   - export GRADLE_USER_HOME=`pwd`/.gradle
 
-test:
+certs:
-  script: "./buildwrapper.sh"
+  stage: build
+  script: "./certs-gen.sh"
+
+compile:
+  stage: build
+  script: "./gradlew clean assemble"
+
+tests:
+  stage: test
+  script:
+    - "./certs-gen.sh" 
+    - "./gradlew test"
+
+package: 
+  stage: deploy
+  script:
+    - "./certs-gen.sh"
+    - "./gradlew build" 
+    - "./package.sh"

README.md

@@ -48,3 +48,13 @@ At this point you can copy the staging/client or staging/server folders to any e
 # Testing
 Running the gradle test task, or the buildwrapper will run all junit tests.
 Currently that includes a test of certificate based authentication (Mutual TLS), tests for the thread safe process control module, and tests ensuring that only one connection can access a processes information at a time.
+
+# Contributing
+Many issues are marked great-first-issue for the sake of first time contributors.
+If you are a more experienced contributor I encourage you to start on a different issue.
+
+### Code Standards
+Java contributions will be held to the following standard
+https://www.oracle.com/technetwork/java/codeconvtoc-136057.html
+
+Scala contributions are welcome as well, it is on the roadmap to refactor all this code to Scala.

build.gradle

@@ -26,13 +26,10 @@ plugins {
 def grpcVersion = '1.20.0' 
 
 repositories {
-    //maven{ url "https://maven-central.storage-download.googleapis.com/repos/central/data/" }
-    //mavenLocal()
     mavenCentral()
 }
 
 dependencies {
-    // This dependency is found on compile classpath of this component and consumers.
     implementation 'com.google.guava:guava:27.0.1-jre'
 
     // Use JUnit test framework

certs-gen.sh

@@ -1,8 +1,18 @@
+ 
 #!/bin/sh
+pwd
+
+# get CNs
+read -p "Enter Server CN (default: localhost): " SRVNAME
+read -p "Enter Client CN (default: localhost): " CLTNAME
+if [ -z "$SRVNAME" ]; then
+    SRVNAME=localhost
+fi
+
+if [ -z "$CLTNAME" ]; then
+    CLTNAME=localhost
+fi
 
-<<<<<<< Updated upstream
-read -p "Enter Server CN (localhost or address): " SRVNAME
-read -p "Enter Client CN (localhost or address): " CLTNAME
 
 SERVER_CA_CN=jobserv-server-ca
 SERVER_PATH=resources/server
@@ -24,6 +34,15 @@ rm -rf staging
 # Get passwords for CAs
 read -p "Enter Server CA Passphrase: " SRVCAPASS
 read -p "Enter Client CA Passphrase: " CLTCAPASS
+if [ -z "$SRVCAPASS" ]; then
+    SRVCAPASS=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 13)
+    echo "[*] Server CA Password is: " $SRVCAPASS
+fi
+
+if [ -z "$CLTCAPASS" ]; then
+    CLTCAPASS=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 13)
+    echo "[*] Client CA Password is: " $CLTCAPASS
+fi
 
 # Generate CA Keys
 echo "[+] Generating Server CA Key"
@@ -78,63 +97,3 @@ openssl pkcs8 -topk8 -nocrypt -in $CLIENT_PATH/private.key -out $CLIENT_PATH/pri
 openssl pkcs8 -topk8 -nocrypt -in $SERVER_PATH/private.key -out $SERVER_PATH/private.pem
 openssl pkcs8 -topk8 -nocrypt -in $TEST_PATH/private.key -out $TEST_PATH/private.pem
 
-echo "[+] initiating gradle build"
-./gradlew clean build
-
-# Ideally this next section would be done with gradle
-# Unfortunately gradle's protobuf distribution plugin does not seem to have facilities to manually include certs 
-# Or to specify seperate client and server tarballs for that matter
-# Definitely more research on gradle should be done, but after JobServ hits MVP
-echo "[+] extracting built code"
-mkdir staging
-mkdir staging/client
-mkdir staging/server
-mkdir staging/test
-
-DIST_TAR=JobServ.tar
-DIST_DIR=JobServ
-if [ -f build/distributions/jobserv.tar ]; then
-    DIST_TAR=jobserv.tar
-    DIST_DIR=jobserv
-fi
-
-tar -xvf build/distributions/$DIST_TAR -C staging/client
-tar -xvf build/distributions/$DIST_TAR -C staging/server
-tar -xvf build/distributions/$DIST_TAR -C staging/test
-
-echo "[+] removing server capabilities from client"
-rm staging/client/$DIST_DIR/bin/jobserv-server staging/client/$DIST_DIR/bin/jobserv-server.bat
-
-echo "[+] removing client capabilities from server"
-rm staging/server/$DIST_DIR/bin/jobserv-client staging/server/$DIST_DIR/bin/jobserv-client.bat
-
-echo "[+] populating certificates"
-cp resources/server/server.crt staging/server/
-cp resources/server/private.pem staging/server/
-cp resources/client/ca.crt staging/server/
-cp resources/client/client.crt staging/client/
-cp resources/client/private.pem staging/client/
-cp resources/server/ca.crt staging/client/
-cp -r resources/* staging/test/
-
-echo "[+] Adding wrapper script for client"
-# This could also be a .desktop file without much more work.
-cat << EOF > staging/client/client
-    ./$DIST_DIR/bin/jobserv-client private.pem client.crt ca.crt \$@
-EOF
-chmod +x staging/client/client
-
-echo "[+] Adding wrapper script for server"
-# This could also be a .desktop file without much more work.
-cat << EOF > staging/server/server
-    ./$DIST_DIR/bin/jobserv-server \$1 server.crt private.pem ca.crt
-EOF
-chmod +x staging/server/server
-
-echo "[+] removing test logs"
-rm JobServ-Server-*
-=======
-./certs-gen.sh
-./gradlew clean build
-./package.sh
->>>>>>> Stashed changes

package.sh

@@ -0,0 +1,54 @@
+#!/bin/sh
+
+# Ideally this next section would be done with gradle
+# Unfortunately gradle's protobuf distribution plugin does not seem to have facilities to manually include certs 
+# Or to specify seperate client and server tarballs for that matter
+# Definitely more research on gradle should be done, but after JobServ hits MVP
+echo "[+] extracting built code"
+mkdir staging
+mkdir staging/client
+mkdir staging/server
+mkdir staging/test
+
+DIST_TAR=JobServ.tar
+DIST_DIR=JobServ
+if [ -f build/distributions/jobserv.tar ]; then
+    DIST_TAR=jobserv.tar
+    DIST_DIR=jobserv
+fi
+
+tar -xvf build/distributions/$DIST_TAR -C staging/client
+tar -xvf build/distributions/$DIST_TAR -C staging/server
+tar -xvf build/distributions/$DIST_TAR -C staging/test
+
+echo "[+] removing server capabilities from client"
+rm staging/client/$DIST_DIR/bin/jobserv-server staging/client/$DIST_DIR/bin/jobserv-server.bat
+
+echo "[+] removing client capabilities from server"
+rm staging/server/$DIST_DIR/bin/jobserv-client staging/server/$DIST_DIR/bin/jobserv-client.bat
+
+echo "[+] populating certificates"
+cp resources/server/server.crt staging/server/
+cp resources/server/private.pem staging/server/
+cp resources/client/ca.crt staging/server/
+cp resources/client/client.crt staging/client/
+cp resources/client/private.pem staging/client/
+cp resources/server/ca.crt staging/client/
+cp -r resources/* staging/test/
+
+echo "[+] Adding wrapper script for client"
+# This could also be a .desktop file without much more work.
+cat << EOF > staging/client/client
+    ./$DIST_DIR/bin/jobserv-client private.pem client.crt ca.crt \$@
+EOF
+chmod +x staging/client/client
+
+echo "[+] Adding wrapper script for server"
+# This could also be a .desktop file without much more work.
+cat << EOF > staging/server/server
+    ./$DIST_DIR/bin/jobserv-server \$1 server.crt private.pem ca.crt
+EOF
+chmod +x staging/server/server
+
+echo "[+] removing test logs"
+rm JobServ-Server-*

src/main/java/JobServ/JobServClientAPIConnector.java

@@ -1,4 +1,4 @@
-b/*
+/*
  * JobServClientAPIConnector
  *
  * v1.0

src/main/java/JobServ/ProcessController.java

@@ -8,11 +8,14 @@
 
 package JobServ;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.BufferedReader;
+import java.io.InputStreamReader;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 
 /*
  * ProcessController
@@ -33,18 +36,23 @@ class ProcessController {
     private BufferedReader reader;
 
     private Process process;
-
     private Boolean killedManually = false;
 
+    private Lock lock;
+    private int lockTimeout; // seconds
+
     /*
      * Constructor
      * Takes a command and spawns it in a new process
      * Redirects IO streams and assigns a fake PID
      */
-    public ProcessController(String command) throws IOException {
+    public ProcessController(String command, int lockTimeout) throws IOException {
         this.pid = ProcessController.nextPid;
         ProcessController.nextPid += 1;
 
+        this.lock = new ReentrantLock();
+        this.lockTimeout = lockTimeout;
+
         this.process = Runtime.getRuntime().exec(command);
         this.output = this.process.getOutputStream();
         this.input = this.process.getInputStream();
@@ -54,6 +62,28 @@ class ProcessController {
         JobServServer.logger.write("Job " + String.valueOf(this.pid) + ": " + command);
     }
 
+    /*
+     * getLock()
+     * attempts to get the lock for lockTimeout seconds
+     * or throws exceptions if interrupted
+     */
+    public boolean getLock() throws InterruptedException {
+        return this.lock.tryLock(this.lockTimeout, TimeUnit.SECONDS);
+    }
+
+    /*
+     * releaseLock()
+     * releases lock on process
+     */
+    public void releaseLock() {
+        try {
+            this.lock.unlock();
+
+        } catch (IllegalMonitorStateException e) {
+            JobServServer.logger.write("Thread tried to release a lock it didnt have! " + e.getMessage());
+        }
+    }
+
     /*
      * getPid()
      * returns translated pid of this process

src/main/java/JobServ/ProcessManager.java

@@ -40,7 +40,6 @@ class ProcessManager {
      * processMap
      */
     protected ConcurrentHashMap<Integer, ProcessController> processMap;
-    protected ConcurrentHashMap<Integer, Boolean> lockMap;
     private ExecutorService threadPool = Executors.newCachedThreadPool();
 
     /*
@@ -49,7 +48,6 @@ class ProcessManager {
      */
     public ProcessManager() {
         processMap = new ConcurrentHashMap<Integer, ProcessController>();
-        lockMap = new ConcurrentHashMap<Integer, Boolean>();
         /* TODO: In a long running server over a large period of time
          * It is possible that the streams used to redirect IO in the
          * Processes may become a significant use of resources.
@@ -68,11 +66,9 @@ class ProcessManager {
     public int newProcess(String command) {
 
         try {
-            ProcessController newProc = new ProcessController(command);
+            ProcessController newProc = new ProcessController(command, this.LOCK_TIMEOUT);
             // we dont need to lock the map yet
-            this.lockMap.put(newProc.getPid(), true);
             this.processMap.put(newProc.getPid(), newProc);
-
             this.releaseLock(newProc.getPid());
             return newProc.getPid();
 
@@ -95,14 +91,13 @@ class ProcessManager {
     public int getProcessStatus(int pid) {
         try {
             if(!this.getLock(pid)) {
-                return 3;
+                // lock could not be grabbed before timeout
+                JobServServer.logger.write("Timeout getting process status: " + String.valueOf(pid));
+                return 4;
             }
 
-        } catch (TimeoutException e) {
+        } catch (IndexOutOfBoundsException e) {
-            // lock could not be grabbed before timeout
+            return 3;
-            JobServServer.logger.write("Timeout getting process " +
-                                       String.valueOf(pid) + " status: " + e.getMessage());
-            return 4;
         }
 
         ProcessController candidate = this.processMap.get(pid);
@@ -123,13 +118,12 @@ class ProcessManager {
     public int getProcessReturn(int pid) {
         try {
             if(!this.getLock(pid)) {
-                return 258;
+                JobServServer.logger.write("Timeout getting process return: " + String.valueOf(pid));
+                return 259;
             }
 
-        } catch (TimeoutException e) {
+        } catch (IndexOutOfBoundsException e) {
-            JobServServer.logger.write("Timeout getting process " +
+            return 258;
-                                       String.valueOf(pid) + " return: " + e.getMessage());
-            return 259;
         }
 
         ProcessController candidate = this.processMap.get(pid);
@@ -146,13 +140,12 @@ class ProcessManager {
     public String getProcessOutput(int pid, int lines) {
         try {
             if(!this.getLock(pid)) {
-                return "[-] SERVER: Process not found";
+                JobServServer.logger.write("Timeout getting process output: " + String.valueOf(pid));
+                return "[-] SERVER: Timeout grabbing lock to access process information";
             }
 
-        } catch (TimeoutException e) {
+        } catch (IndexOutOfBoundsException e) {
-            JobServServer.logger.write("Timeout getting process " +
+            return "[-] SERVER: Process not found";
-                                       String.valueOf(pid) + " output: " + e.getMessage());
-            return "[-] SERVER: Timeout grabbing lock to access process information";
         }
 
         ProcessController candidate = this.processMap.get(pid);
@@ -172,13 +165,13 @@ class ProcessManager {
     public int killProcess(int pid) {
         try {
             if(!this.getLock(pid)) {
-                return 2;
+                JobServServer.logger.write("Timeout killing process: " + String.valueOf(pid));
+                return 3;
             }
 
-        } catch (TimeoutException e) {
+        } catch (IndexOutOfBoundsException e) {
-            JobServServer.logger.write("Timeout killing process " +
+
-                                       String.valueOf(pid) + ": " + e.getMessage());
+            return 2;
-            return 3;
         }
 
         ProcessController candidate = this.processMap.get(pid);
@@ -194,45 +187,20 @@ class ProcessManager {
      * Function is synchronized to prevent multiple threads accessing the same lock at once
      * (ConcurrentHashMap will report whatever lock value was last to successfully update)
      */
-    protected synchronized Boolean getLock(int pid) throws TimeoutException {
+    protected synchronized Boolean getLock(int pid) throws IndexOutOfBoundsException {
-        if (!lockMap.containsKey(pid)) {
+        ProcessController candidate = this.processMap.get(pid);
-            return false;
+        if (candidate == null) {
+            throw new IndexOutOfBoundsException();
         }
 
-        Future<Object> future = this.threadPool.submit(
-            new Callable<Object>() {
-                public Object call() {
-                    while(lockMap.get(pid)) {
-                        continue; // spin!
-                    }
-
-                    lockMap.replace(pid, true);
-                    return 1;
-                }
-            });
-
         try {
-            future.get(this.LOCK_TIMEOUT, TimeUnit.SECONDS);
+            Boolean success = candidate.getLock();
+            return success;
 
         } catch (InterruptedException e) {
             JobServServer.logger.write("[!] Couldnt get lock " +
                                        String.valueOf(pid) + ": "+ e.getMessage());
-            future.cancel(true);
-
-	    // in case lock was grabbed after exception
-	    this.releaseLock(pid);
             return false;
-
-        } catch (ExecutionException e) {
-            JobServServer.logger.write("[!] Couldnt get lock " +
-                                       String.valueOf(pid) + ": "+ e.getMessage());
-            future.cancel(true);
-
-	    // in case lock was grabbed after exception
-	    this.releaseLock(pid);
-            return false;
-
-        // cancel the attempt to grab the lock
         }
 
         /*
@@ -247,8 +215,6 @@ class ProcessManager {
          * mediate access to the ProcessManager
          * object for fresh calls as well.
          */
-
-        return true;
     }
 
     /*
@@ -256,7 +222,13 @@ class ProcessManager {
      * releases mutex so other threads can operate on processqueue
      */
     protected void releaseLock(int pid) {
-        this.lockMap.put(pid, false);
+        ProcessController candidate = this.processMap.get(pid);
+        if (candidate == null) {
+            JobServServer.logger.write("Tried to release lock of process that doesnt exist!");
+            return;
+        }
+
+        candidate.releaseLock();
     }
 
     /*

src/test/java/JobServ/ProcessManagerTestImplementation.java

@@ -27,18 +27,10 @@ class ProcessManagerTestImplementation extends ProcessManager {
 
             super.releaseLock(pid);
 
-        } catch (TimeoutException e) {
-            System.err.println("[!!] Long Call wasnt able to grab lock!");
-            return;
-
         } catch (InterruptedException e) {
             super.releaseLock(pid); // this doesnt happen, dont cancel this task
             System.err.println("[3] Released lock: interrupted");
             return;
         }
     }
-
-    public Boolean reportLockState(int pid) {
-        return super.lockMap.get(pid);
-    }
 }