Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JPO-ODE Hotfix for Security Headers #545

Merged

Conversation

drewjj
Copy link
Collaborator

@drewjj drewjj commented Apr 19, 2024

PR Details

Description

Adds support for stripping IEEE 1609.2 and 1609.3 headers for both log ingestion and UDP endpoints. The data pipeline workflow has been altered for the log ingestion to merge with the pipeline for the UDP endpoints. This allows the ASN1 to be maintained from both message endpoints within a Kafka topic and include any signed security headers if present.

These changes can be viewed via the updated dataflow diagrams.

Related Issue

N/A

Motivation and Context

This allows for more flexible support of messages with IEEE1609.2 and 1609.3 headers for both log ingestion and UDP forwarding. This pull request also allows for the maintaining of the ASN1 with headers for troubleshooting purposes.

How Has This Been Tested?

This has been tested through unit tests, local Docker deployment and GCP K8s.

Types of changes

  • Defect fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that cause existing functionality to change)

Checklist:

  • I have added any new packages to the sonar-scanner.properties file
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
    ODE Contributing Guide
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@drewjj drewjj requested a review from dan-du-car April 19, 2024 22:38
@@ -72,7 +65,7 @@ public ParserStatus parseFile(BufferedInputStream bis, String fileName) throws F
status = parseStep(bis, getPayloadLength());
if (status != ParserStatus.COMPLETE)
return status;
setPayload(removeHeader(Arrays.copyOf(readBuffer, getPayloadLength())));
setPayload(stripDot3Header(Arrays.copyOf(readBuffer, getPayloadLength())));
Copy link
Contributor

@dan-du-car dan-du-car Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide some explanation of these requirements change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original implementation was the removeHeader function. This needed to remove all headers and only include the payload. We now want to maintain the 1609.2 header if it is a signed header so it will be written into Kafka before it goes to the ADM. We need to be able to record the original header UPER hex so we can utilize it. The signed security header does get removed before the ADM since the ADM doesn't truly support IEEE 1609.2 headers.

// construct odeData
odeData = new OdeAsn1Data(metadata, payload);
String payloadHexString = ((JSONObject)((JSONObject) rawMapJsonObject.get("payload")).get("data")).getString("bytes");
payloadHexString = super.stripDot2Header(payloadHexString);
Copy link
Contributor

@dan-du-car dan-du-car Apr 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we combine the start flags data with this stripDot2Header() behavior into a separate series of classes that are dedicated to remove the Dot2header? Then set this inside Message JSON classes without modifying the message constructor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will add this to our backlog to get this to be more generic. We really are just wanting to get this functionality to be available so CDOT and WYDOT can both utilize this through DockerHub.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drewjj Please address the unit test failure reported by sonarscanner

Copy link
Collaborator Author

@drewjj drewjj May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failure seems to only affect CI. We haven't been able to resolve Travis from failing. Any ideas? @dan-du-car This has been plaguing us since the last release.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GM, Drew. Just discussed with Dan and Sai. We documented the issue here #546 for now and Sai can plan to address this issue with you/Tony in our next sprint in June. For now, we are good with merging the hotfix changes into master, which Dan has reviewed.

@drewjj drewjj changed the base branch from develop to master April 26, 2024 14:43
@dan-du-car
Copy link
Contributor

Thanks for the update.

@SaikrishnaBairamoni SaikrishnaBairamoni merged commit 331a90f into usdot-jpo-ode:master May 23, 2024
1 of 2 checks passed
@drewjj drewjj deleted the hotfix/security_headers branch January 9, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants