diff --git a/.github/codeql/filters.yml b/.github/codeql/filters.yml new file mode 100644 index 00000000..c5a975d7 --- /dev/null +++ b/.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 + diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 2ae42da8..b74b9b18 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -9,28 +9,35 @@ # 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 @@ -38,12 +45,19 @@ jobs: 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 @@ -51,36 +65,50 @@ jobs: - 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 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index af240e93..a9ac6088 100644 --- a/.github/workflows/main.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 4da45190..cdc6ff88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,5 @@ +## 2.1.12 +- Security fixes applied. ## 2.1.11 - Enable to change log level for publish at runtime diff --git a/pom.xml b/pom.xml index fc4bec5e..e124750d 100644 --- a/pom.xml +++ b/pom.xml @@ -6,10 +6,10 @@ com.github.eiffel-community eiffel-remrem-parent - 2.0.15 + 2.0.18 - 2.1.11 + 2.1.12 2.4.2 eiffel-remrem-publish diff --git a/publish-common/src/main/java/com/ericsson/eiffel/remrem/publish/service/EventTemplateHandler.java b/publish-common/src/main/java/com/ericsson/eiffel/remrem/publish/service/EventTemplateHandler.java index 58b60aff..c598eb7a 100644 --- a/publish-common/src/main/java/com/ericsson/eiffel/remrem/publish/service/EventTemplateHandler.java +++ b/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,6 +62,13 @@ 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){ @@ -65,6 +76,7 @@ public JsonNode eventTemplateParser(String jsonData , String eventName){ JsonNode rootNode = null; try { + validateEventName(eventName); String eventTemplate = accessFileInSemanticJar(EVENT_TEMPLATE_PATH + eventName.toLowerCase() + ".json"); rootNode = mapper.readTree(jsonData); diff --git a/publish-service/pom.xml b/publish-service/pom.xml index 41312542..d9c43de4 100644 --- a/publish-service/pom.xml +++ b/publish-service/pom.xml @@ -150,6 +150,11 @@ + + org.apache.commons + commons-text + 1.13.1 + diff --git a/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/config/SecurityConfig.java b/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/config/SecurityConfig.java index ab4fe376..a9b992ae 100644 --- a/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/config/SecurityConfig.java +++ b/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(); } } diff --git a/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java b/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java index 6024cb3a..bdbe6ec6 100644 --- a/publish-service/src/main/java/com/ericsson/eiffel/remrem/publish/controller/ProducerController.java +++ b/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,7 +346,6 @@ 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) { @@ -324,8 +353,17 @@ public ResponseEntity generateAndPublish(final String msgProtocol, final String 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 entity = new HttpEntity<>(bodyJsonOut, headers); - String generateUrl = generateURLTemplate.getUrl() + "&failIfMultipleFound=" + failIfMultipleFound - + "&failIfNoneFound=" + failIfNoneFound + "&lookupInExternalERs=" + lookupInExternalERs - + "&lookupLimit=" + lookupLimit + "&okToLeaveOutInvalidOptionalFields=" + okToLeaveOutInvalidOptionalFields; - ResponseEntity 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 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); } diff --git a/publish-service/src/test/java/com/ericsson/eiffel/remrem/publish/integrationtest/TestSecurityConfig.java b/publish-service/src/test/java/com/ericsson/eiffel/remrem/publish/integrationtest/TestSecurityConfig.java index 8b1fc7ba..2850eec2 100644 --- a/publish-service/src/test/java/com/ericsson/eiffel/remrem/publish/integrationtest/TestSecurityConfig.java +++ b/publish-service/src/test/java/com/ericsson/eiffel/remrem/publish/integrationtest/TestSecurityConfig.java @@ -37,13 +37,13 @@ protected void configure(AuthenticationManagerBuilder auth) throws Exception { @Override protected void configure(HttpSecurity http) throws Exception { http - .authorizeRequests() - .anyRequest() - .authenticated() - .and() - .httpBasic() - .and() - .csrf() - .disable(); + .authorizeRequests() + .anyRequest() + .authenticated() + .and() + .httpBasic() + .and() + .csrf() + .disable(); } }