Analyze security

.github/codeql/filters.yml

@@ -0,0 +1,12 @@
+query-filters:
+  # The application uses non-browser clients. Yes, there is swagger interface,
+  # but is's used only for testing/tuning.
+  #
+  # From https://docs.spring.io/spring-security/reference/features/exploits/csrf.html
+  # "If you are creating a service that is used only by non-browser clients,
+  #  you likely want to disable CSRF protection."
+  - exclude: 
+      id: java/spring-disabled-csrf-protection
+  - exclude: 
+      id: java/error-message-exposure
+

.github/workflows/codeql.yml

@@ -9,78 +9,106 @@
 # the `language` matrix defined below to confirm you have the correct set of
 # supported CodeQL languages.
 #
-name: "CodeQL"
+name: "CodeQL Advanced"
 
 on:
   push:
-    branches: [ "master" ]
+    # Branch analyze-security will be removed once the PR is merged.
+    branches: [ "master", "analyze-security" ]
   pull_request:
-    # The branches below must be a subset of the branches above
     branches: [ "master" ]
   schedule:
     - cron: '28 3 * * 1'
 
 jobs:
   analyze:
-    name: Analyze
+    name: Analyze (${{ matrix.language }})
     # Runner size impacts CodeQL analysis time. To learn more, please see:
     #   - https://gh.io/recommended-hardware-resources-for-running-codeql
     #   - https://gh.io/supported-runners-and-hardware-resources
-    #   - https://gh.io/using-larger-runners
-    # Consider using larger runners for possible analysis time improvements.
+    #   - https://gh.io/using-larger-runners (GitHub.com only)
+    # Consider using larger runners or machines with greater resources for possible analysis time improvements.
     runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }}
     timeout-minutes: ${{ (matrix.language == 'swift' && 120) || 360 }}
     permissions:
+      # required for all workflows
+      security-events: write
+
+      # required to fetch internal or private CodeQL packs
+      packages: read
+
+      # only required for workflows in private repositories
       actions: read
       contents: read
       security-events: write
 
     strategy:
       fail-fast: false
       matrix:
-        language: [ 'java-kotlin' ]
-        # CodeQL supports [ 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift' ]
-        # Use only 'java-kotlin' to analyze code written in Java, Kotlin or both
-        # Use only 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both
-        # Learn more about CodeQL language support at https://aka.ms/codeql-docs/language-support
-
+        include:
+        - language: actions
+          build-mode: none
+        - language: java-kotlin
+          build-mode: none # This mode only analyzes Java. Set this to 'autobuild' or 'manual' to analyze Kotlin too.
+        # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'swift'
+        # Use `c-cpp` to analyze code written in C, C++ or both
+        # Use 'java-kotlin' to analyze code written in Java, Kotlin or both
+        # Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both
+        # To learn more about changing the languages that are analyzed or customizing the build mode for your analysis,
+        # see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning.
+        # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how
+        # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages
     steps:
     - name: Checkout repository
       uses: actions/checkout@v4
 
     - name: set up jdk 17
       uses: actions/setup-java@v4
       with:
-         distribution: 'temurin'
-         java-version: '17'
-         
+        distribution: 'temurin'
+        java-version: '17'
+
     - name: checkout code
       uses: actions/checkout@v4
 
+    # Add any setup steps before running the `github/codeql-action/init` action.
+    # This includes steps like installing compilers or runtimes (`actions/setup-node`
+    # or others). This is typically only required for manual builds.
+    # - name: Setup runtime (example)
+    #   uses: actions/setup-example@v1
+
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
       uses: github/codeql-action/init@v3
       with:
         languages: ${{ matrix.language }}
+        build-mode: ${{ matrix.build-mode }}
         # If you wish to specify custom queries, you can do so here or in a config file.
         # By default, queries listed here will override any specified in a config file.
         # Prefix the list here with "+" to use these queries and those in the config file.
 
         # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
         # queries: security-extended,security-and-quality
+        config-file: .github/codeql/filters.yml
 
-
-    # Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift).
-    # If this step fails, then you should remove it and run the build manually (see below)
-
+    # If the analyze step fails for one of the languages you are analyzing with
+    # "We were unable to automatically build your code", modify the matrix above
+    # to set the build mode to "manual" for that language. Then modify this step
+    # to build your code.
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun
-
-    #   If the Autobuild fails above, remove it and uncomment the following three lines.
-    #   modify them (or add more) to build your code if your project, please refer to the EXAMPLE below for guidance.
+    - if: matrix.build-mode == 'manual'
+      shell: bash
+      run: |
+        echo 'If you are using a "manual" build mode for one or more of the' \
+          'languages you are analyzing, replace this with the commands to build' \
+          'your code, for example:'
+        echo '  make bootstrap'
+        echo '  make release'
+        exit 1
 
     - run: |
-         mvn clean package -DskipTests
+          mvn clean package -DskipTests
 
     - name: Perform CodeQL Analysis
       uses: github/codeql-action/analyze@v3

.github/workflows/main.yml

@@ -2,6 +2,10 @@
 
 name: CI
 
+permissions:
+  contents: read
+  pull-requests: write
+
 # Controls when the workflow will run
 on:
   # Triggers the workflow on push or pull request events but only for the master branch

CHANGELOG.md

@@ -1,3 +1,5 @@
+## 2.1.12
+- Security fixes applied.
 
 ## 2.1.11
 - Enable to change log level for publish at runtime

pom.xml

@@ -9,7 +9,7 @@
         <version>2.0.15</version>
     </parent>
     <properties>
-        <eiffel-remrem-publish.version>2.1.11</eiffel-remrem-publish.version>
+        <eiffel-remrem-publish.version>2.1.12</eiffel-remrem-publish.version>
         <eiffel-remrem-semantics.version>2.4.2</eiffel-remrem-semantics.version>
     </properties>
     <artifactId>eiffel-remrem-publish</artifactId>

publish-common/src/main/java/com/ericsson/eiffel/remrem/publish/service/EventTemplateHandler.java

@@ -15,6 +15,8 @@
 package com.ericsson.eiffel.remrem.publish.service;
 
 import ch.qos.logback.classic.Logger;
+import com.ericsson.eiffel.remrem.semantics.EiffelEventType;
+import com.ericsson.eiffel.semantics.events.Event;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.JsonParseException;
@@ -31,6 +33,8 @@
 import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider;
 import org.apache.commons.io.IOUtils;
 import org.slf4j.LoggerFactory;
+
+import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
@@ -58,13 +62,21 @@ public class EventTemplateHandler {
         .mappingProvider(new JacksonMappingProvider(mapper))
         .build();
 
+    // Ensure event name doesn't contain any path-special character or path separator.
+    private void validateEventName(String eventName) {
+        if (eventName.contains("..") || eventName.contains(File.separator)) {
+            throw new IllegalArgumentException("Invalid event name: '" + eventName + "'");
+        }
+    }
+
     // eventTemplateParser
     @JsonInclude(JsonInclude.Include.NON_NULL)
     public JsonNode eventTemplateParser(String jsonData , String eventName){
         JsonNode updatedJson = null;
 
         JsonNode rootNode = null;
         try {
+            validateEventName(eventName);
             String eventTemplate = accessFileInSemanticJar(EVENT_TEMPLATE_PATH + eventName.toLowerCase() + ".json");
 
             rootNode = mapper.readTree(jsonData);

publish-service/pom.xml

@@ -150,6 +150,11 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-text</artifactId>
+            <version>1.13.1</version>
+        </dependency>
     </dependencies>
     <build>
         <plugins>

publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/config/DisabledSecurityConfig.java

@@ -34,6 +34,6 @@
 public class DisabledSecurityConfig extends WebSecurityConfigurerAdapter {
     @Override
     protected void configure(HttpSecurity http) throws Exception {
-        http.authorizeRequests().anyRequest().permitAll().and().csrf().disable();
+        http.authorizeRequests().anyRequest().permitAll().and().csrf();
     }
 }

publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/config/SecurityConfig.java

@@ -30,6 +30,8 @@
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
 
+import static org.apache.catalina.webresources.TomcatURLStreamHandlerFactory.disable;
+
 /**
  * This class is used to enable the ldap authentication based on property
  * activedirectory.publish.enabled = true in properties file.
@@ -111,6 +113,12 @@ protected void configure(HttpSecurity http) throws Exception {
             .authenticationEntryPoint(customAuthenticationEntryPoint)
             .and()
             .csrf()
+            // The application uses non-browser clients. Yes, there is swagger interface,
+            // but is's used only for testing/tuning.
+            //
+            // From https://docs.spring.io/spring-security/reference/features/exploits/csrf.html
+            // "If you are creating a service that is used only by non-browser clients,
+            //  you likely want to disable CSRF protection."
             .disable();
     }
 }

publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java

@@ -14,11 +14,15 @@
 */
 package com.ericsson.eiffel.remrem.publish.controller;
 
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 import com.ericsson.eiffel.remrem.protocol.ValidationResult;
 import com.ericsson.eiffel.remrem.publish.service.*;
 import com.google.gson.*;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -221,6 +225,8 @@ public ResponseEntity send(
         } catch (JsonSyntaxException e) {
             String exceptionMessage = e.getMessage();
             log.error("Cannot parse the following JSON data:\n" + body + "\n\n" + exceptionMessage);
+            // TODO Disable CodeQL rule java/error-message-exposure. The message is sent to user to
+            // TODO show where exactly JSON parser encountered an issue, i.e. line and column.
             return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_FATAL_STATUS,
                     "Invalid JSON data: " + exceptionMessage);
         }
@@ -294,6 +300,30 @@ private boolean eventTypeExists(@NonNull MsgService msgService, String eventType
         return supportedEventTypes != null && supportedEventTypes.contains(eventType);
     }
 
+    /**
+     * Ensure attribute value is properly URL encoded.
+     *
+     * @param attribute Attribute name.
+     * @param value Attribute value. It's converted to string using toString().
+     * @return "&attribute=[URL encoded value]"
+     * @throws UnsupportedEncodingException
+     */
+    private String appendAttributeAndValue(String attribute, Object value)
+            throws UnsupportedEncodingException {
+        return "&" + attribute + "="
+                + URLEncoder.encode(value.toString(), StandardCharsets.UTF_8.toString());
+    }
+
+    /**
+     * Guarantees that given value is not null.
+     *
+     * @param b a value
+     * @return b if non-null, Boolean.FALSE otherwise.
+     */
+    private Boolean ensureValueNonNull(Boolean b) {
+        return b != null ? b : Boolean.FALSE;
+    }
+
     /**
      * This controller provides single RemRem REST API End Point for both RemRem
      * Generate and Publish.
@@ -316,16 +346,24 @@ private boolean eventTypeExists(@NonNull MsgService msgService, String eventType
      *      --data "@inputGenerate_activity_finished.txt"
      *      "http://localhost:8986/generateAndPublish/?mp=eiffelsemantics&msgType=EiffelActivityFinished"
      */
-
     public ResponseEntity generateAndPublish(final String msgProtocol, final String msgType, final String userDomain, final String tag, final String routingKey,
                                              final Boolean parseData, final Boolean failIfMultipleFound, final Boolean failIfNoneFound, final Boolean lookupInExternalERs,
                                              final int lookupLimit, final Boolean okToLeaveOutInvalidOptionalFields, final JsonElement bodyJson) {
         if (isAuthenticationEnabled) {
             logUserName();
         }
 
-        MsgService msgService = PublishUtils.getMessageService(msgProtocol, msgServices);
-        if (msgService == null) {
+        if (StringUtils.isEmpty(msgProtocol)) {
+            return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_FATAL_STATUS, "Value of parameter 'msgProtocol' is empty");
+        }
+
+        if (StringUtils.isEmpty(msgType)) {
+            return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_FATAL_STATUS, "Value of parameter 'msgType' is empty");
+        }
+
+        MsgService msgService;
+        if (StringUtils.isEmpty(msgProtocol) ||
+                ((msgService = PublishUtils.getMessageService(msgProtocol, msgServices)) == null)) {
             return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_ERROR_STATUS,
                     "No protocol service has been found registered");
         }
@@ -338,7 +376,7 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
             if (bodyJsonArray.size() > maxSizeOfInputArray) {
                 return createResponseEntity(HttpStatus.BAD_REQUEST, JSON_ERROR_STATUS,
                         "The number of events in the input array is too high: " + bodyJsonArray.size() + " > "
-                                + maxSizeOfInputArray + "; you can modify the property 'maxSizeOfInputArray' to increase it.");
+                                + maxSizeOfInputArray + "; you can modify the property 'maxSizeOfInpuArray' to increase it.");
             }
             for (JsonElement element : bodyJsonArray) {
                 if (element.isJsonObject()) {
@@ -382,13 +420,17 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
                 bodyJsonOut = bodyJson.toString();
             }
             HttpHeaders headers = new HttpHeaders();
-            headers.setContentType(MediaType.APPLICATION_JSON_UTF8);
+            headers.setContentType(MediaType.APPLICATION_JSON);
             HttpEntity<String> entity = new HttpEntity<>(bodyJsonOut, headers);
-            String generateUrl = generateURLTemplate.getUrl() + "&failIfMultipleFound=" + failIfMultipleFound
-                    + "&failIfNoneFound=" + failIfNoneFound + "&lookupInExternalERs=" + lookupInExternalERs
-                    + "&lookupLimit=" + lookupLimit + "&okToLeaveOutInvalidOptionalFields=" + okToLeaveOutInvalidOptionalFields;
 
-            ResponseEntity<String> response = restTemplate.postForEntity(generateUrl,
+            String generatedUrl = generateURLTemplate.getUrl()
+                + appendAttributeAndValue("failIfMultipleFound", ensureValueNonNull(failIfMultipleFound))
+                + appendAttributeAndValue("failIfNoneFound", ensureValueNonNull(failIfNoneFound))
+                + appendAttributeAndValue("lookupInExternalERs", ensureValueNonNull(lookupInExternalERs))
+                + appendAttributeAndValue("lookupLimit", lookupLimit)
+                + appendAttributeAndValue("okToLeaveOutInvalidOptionalFields", ensureValueNonNull(okToLeaveOutInvalidOptionalFields));
+
+            ResponseEntity<String> response = restTemplate.postForEntity(generatedUrl,
                     entity, String.class, generateURLTemplate.getMap(msgProtocol, msgType));
 
             responseStatus = response.getStatusCode();
@@ -415,10 +457,14 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
             } else {
                 return response;
             }
-        } catch (RemRemPublishException e) {
-            String exceptionMessage = e.getMessage();
-            return createResponseEntity(HttpStatus.NOT_FOUND, JSON_ERROR_STATUS, exceptionMessage);
-        } catch (HttpStatusCodeException e) {
+        }
+        catch (UnsupportedEncodingException e) {
+            return createResponseEntity(HttpStatus.INTERNAL_SERVER_ERROR, JSON_FATAL_STATUS, e.getMessage());
+        }
+        catch (RemRemPublishException e) {
+            return createResponseEntity(HttpStatus.NOT_FOUND, JSON_ERROR_STATUS, e.getMessage());
+        }
+        catch (HttpStatusCodeException e) {
             String responseBody = null;
             String responseMessage = e.getResponseBodyAsString();
             if (bodyJson.isJsonObject()) {
@@ -430,7 +476,9 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String
                     tag, routingKey, okToLeaveOutInvalidOptionalFields);
             return new ResponseEntity<>(responseEvents, HttpStatus.BAD_REQUEST);
         }
-        //Status here is the status returned from generate service, except BAD_REQUEST which already handled above
+
+        // Status here is the status returned from generate service,
+        // except BAD_REQUEST which already handled above.
         return new ResponseEntity<>(responseEvents, responseStatus);
     }