Skip to content

Potential fix for code scanning alert no. 20: Use of a broken or risky cryptographic algorithm #37

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.

Sign up for GitHub

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

Merged
phrocker merged 2 commits into from
Mar 11, 2025
Merged

Potential fix for code scanning alert no. 20: Use of a broken or risky cryptographic algorithm #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cdf2a28
Potential fix for code scanning alert no. 20: Use of a broken or risk…
phrocker Mar 10, 2025
1be280e
fixup
phrocker Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/main/java/io/sentrius/sso/controllers/api/HostApiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ public ResponseEntity<ObjectNode> connectSSHServer(HttpServletRequest request, H

sessionMetadata = terminalSessionMetadataService.createSession(sessionMetadata);

var encryptedSessionId = cryptoService.encrypt(connectedSystem.getSession().getId().toString());
var encryptedSessionId = cryptoService.encrypt(connectedSystem.getSession().getId().toString().trim());

log.info("returning " + encryptedSessionId);
log.info("returning {} from {}", encryptedSessionId, connectedSystem.getSession().getId().toString().trim());

node.put("sessionId", encryptedSessionId);

Expand Down
6 changes: 2 additions & 4 deletions api/src/main/java/io/sentrius/sso/websocket/ChatWSHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,9 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
return;
}
// Decrypt the session ID
// var sessionIdStr = cryptoService.decrypt(sessionId);
// var sessionIdLong = Long.parseLong(sessionIdStr);
var lookupId = sessionId + "==";
var sessionIdStr = cryptoService.decrypt(sessionId);
// Retrieve ConnectedSystem from your persistent map using the session ID
var sys = sessionTrackingService.getEncryptedConnectedSession(lookupId);
var sys = sessionTrackingService.getEncryptedConnectedSession(sessionIdStr);
if (null != sys ) {
log.info("oh");
// Get the user's session and handle trigger if present
Expand Down
25 changes: 13 additions & 12 deletions api/src/main/java/io/sentrius/sso/websocket/TerminalWSHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void afterConnectionEstablished(WebSocketSession session) throws Exceptio
if (sessionId != null) {
// Store the WebSocket session using the session ID from the query parameter
sessions.put(sessionId, session);
log.info("New connection established, session ID: " + sessionId);
log.debug("New connection established, session ID: " + sessionId);
sshListenerService.startListeningToSshServer(sessionId, session);
} else {
log.trace("Session ID not found in query parameters.");
Expand All @@ -75,14 +75,14 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)

// Extract query parameters from the URI again if needed
URI uri = session.getUri();
log.trace("got message {}", uri);
log.debug("got message {}", uri);
try {
if (uri != null) {
Map<String, String> queryParams = parseQueryParams(uri.getQuery());
String sessionId = queryParams.get("sessionId");

if (sessionId != null) {
log.trace("Received message from session ID: " + sessionId);
log.debug("Received message from session ID: " + sessionId);
// Handle the message (e.g., process or respond)


Expand All @@ -91,11 +91,10 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
Session.TerminalMessage auditLog =
Session.TerminalMessage.parseFrom(messageBytes);
// Decrypt the session ID
// var sessionIdStr = cryptoService.decrypt(sessionId);
// var sessionIdLong = Long.parseLong(sessionIdStr);
var lookupId = sessionId + "==";
var sessionIdStr = cryptoService.decrypt(sessionId);
var lookupId = sessionId; // + "==";
// Retrieve ConnectedSystem from your persistent map using the session ID
var sys = sessionTrackingService.getEncryptedConnectedSession(lookupId);
var sys = sessionTrackingService.getEncryptedConnectedSession(sessionIdStr);
if (null != sys ) {
boolean allNoAction = true;
log.debug("**** Processing message for session ID: {} with {} actions", sessionId,
Expand Down Expand Up @@ -125,14 +124,16 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
}
}
if (allNoAction && sys.getSessionStartupActions().size() > 0) {
log.info("**** Setting NO_ACTION Trigger");
log.debug("**** Setting NO_ACTION Trigger");
var noActionTrigger = new Trigger(TriggerAction.NO_ACTION, "");
sessionTrackingService.addSystemTrigger(sys, noActionTrigger);
sys.getTerminalAuditor().setSessionTrigger(noActionTrigger);
}

// Get the user's session and handle trigger if present
sshListenerService.processTerminalMessage(sys, auditLog);
} else {
log.debug("No session found for session ID: {}", sessionId);
}
} else {
log.trace("Session ID not found in query parameters for message handling.");
Expand All @@ -153,10 +154,10 @@ public void afterConnectionClosed(WebSocketSession session, org.springframework.

if (sessionId != null) {
// Remove the session when connection is closed
var lookupId = sessionId + "==";
var sys = sessionTrackingService.getEncryptedConnectedSession(lookupId);
var sessionIdStr = cryptoService.decrypt(sessionId);
var sys = sessionTrackingService.getEncryptedConnectedSession(sessionIdStr);
if (null != sys){
log.info("**** Closing session for {}", sys.getSession());
log.debug("**** Closing session for {}", sys.getSession());
terminalSessionMetadataService.getSessionBySessionLog(sys.getSession()).ifPresent(sessionMetadata -> {
sessionMetadata.setEndTime(new Timestamp(System.currentTimeMillis()));
sessionMetadata.setSessionStatus("CLOSED");
Expand All @@ -167,7 +168,7 @@ public void afterConnectionClosed(WebSocketSession session, org.springframework.
sessions.remove(sessionId);
sshListenerService.removeSession(sessionId);

log.info("Connection closed, session ID: " + sessionId);
log.debug("Connection closed, session ID: {}", sessionId);
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions api/src/main/resources/static/node/js/sockjs-client/sockjs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5227,6 +5227,3 @@ module.exports = Url;

},{"querystringify":58,"requires-port":59}]},{},[1])(1)
});


//# sourceMappingURL=sockjs.js.map
2 changes: 1 addition & 1 deletion build-images-gcp.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ if $update_sentrius_keycloak; then
fi

if $update_sentrius_agent; then
cp java-agents/target/java-agents-*.jar docker/sentrius-agent/agent.jar
cp java-agents/target/java-agent-*.jar docker/sentrius-agent/agent.jar
SENTRIUS_AGENT_VERSION=$(increment_patch_version $SENTRIUS_AGENT_VERSION)
build_and_push_image "sentrius-agent" "$SENTRIUS_AGENT_VERSION" "./docker/sentrius-agent"
rm docker/sentrius-agent/agent.jar
Expand Down
2 changes: 1 addition & 1 deletion build-images-local.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ if $update_sentrius_keycloak; then
fi

if $update_sentrius_agent; then
cp java-agents/target/java-agents-*.jar docker/sentrius-agent/agent.jar
cp java-agents/target/java-agent-*.jar docker/sentrius-agent/agent.jar
SENTRIUS_AGENT_VERSION=$(increment_patch_version $SENTRIUS_AGENT_VERSION)
build_image "sentrius-agent" "$SENTRIUS_AGENT_VERSION" "./docker/sentrius-agent"
rm docker/sentrius-agent/agent.jar
Expand Down
46 changes: 38 additions & 8 deletions core/src/main/java/io/sentrius/sso/core/security/service/CryptoService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package io.sentrius.sso.core.security.service;

import javax.crypto.Cipher;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.SecretKeySpec;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.security.GeneralSecurityException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.Base64;
import com.jcraft.jsch.JSch;
import com.jcraft.jsch.JSchException;
Expand All @@ -30,7 +32,7 @@ public class CryptoService {
final ApplicationKeyRepository applicationKeyRepository;
private final byte[] key;

private static final String CIPHER_INSTANCE = "AES/ECB/PKCS5Padding";
private static final String CIPHER_INSTANCE = "AES/GCM/NoPadding";
private static final String CRYPT_ALGORITHM = "AES";
private static final String HASH_ALGORITHM = "SHA-256";
private final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
Expand Down Expand Up @@ -65,25 +67,53 @@ public String hash(String str, String salt) throws NoSuchAlgorithmException {

public String encrypt(String str) throws GeneralSecurityException {
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
byte[] iv = new byte[12];
new SecureRandom().nextBytes(iv);
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);
byte[] encVal = cipher.doFinal(str.getBytes(StandardCharsets.UTF_8));
return Base64.getEncoder().encodeToString(encVal);
byte[] encryptedIvAndText = new byte[iv.length + encVal.length];
System.arraycopy(iv, 0, encryptedIvAndText, 0, iv.length);
System.arraycopy(encVal, 0, encryptedIvAndText, iv.length, encVal.length);
return Base64.getEncoder().encodeToString(encryptedIvAndText);
}

public String encrypt(byte [] bytes) throws GeneralSecurityException {
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
byte[] iv = new byte[12];
new SecureRandom().nextBytes(iv);
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);
byte[] encVal = cipher.doFinal(bytes);
return Base64.getEncoder().encodeToString(encVal);
byte[] encryptedIvAndText = new byte[iv.length + encVal.length];
System.arraycopy(iv, 0, encryptedIvAndText, 0, iv.length);
System.arraycopy(encVal, 0, encryptedIvAndText, iv.length, encVal.length);
return Base64.getEncoder().encodeToString(encryptedIvAndText);
}

public String decrypt(String encryptedStr) throws GeneralSecurityException {
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");

// Decode Base64
byte[] decodedVal = Base64.getDecoder().decode(encryptedStr);
return new String(cipher.doFinal(decodedVal), StandardCharsets.UTF_8);

// Extract IV (first 12 bytes)
byte[] iv = Arrays.copyOfRange(decodedVal, 0, 12);

// Extract actual ciphertext (rest of the bytes)
byte[] cipherText = Arrays.copyOfRange(decodedVal, 12, decodedVal.length);

// Ensure we use the same IV for decryption
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);

// Decrypt the text
byte[] decryptedBytes = cipher.doFinal(cipherText);

return new String(decryptedBytes, StandardCharsets.UTF_8);
}


public String encodePassword(String password) throws NoSuchAlgorithmException {
return encoder.encode(password);
}
Expand Down
16 changes: 4 additions & 12 deletions core/src/main/java/io/sentrius/sso/core/services/terminal/SessionTrackingService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,8 @@ public List<ConnectedSystem> getConnectedSession() {
*/
public void removeUserSession(ConnectedSystem connectedSystem) {
userConnectionMap.remove(connectedSystem.getSession().getId());
try {
userConnectionMapEncrypted.remove(
cryptoService.encrypt(connectedSystem.getSession().getId().toString()) );
} catch (GeneralSecurityException e) {
throw new RuntimeException(e);
}
userConnectionMapEncrypted.remove(
connectedSystem.getSession().getId().toString());
UserSessionsOutput userSessionsOutput = userSessionsOutputMap.get(connectedSystem.getSession().getId());
if (userSessionsOutput != null) {
userSessionsOutput.getSessionOutputMap().clear();
Expand Down Expand Up @@ -107,12 +103,8 @@ public void addOutput(SessionOutput sessionOutput) {
userSessionsOutputMap.put(sessionOutput.getSessionId(), new UserSessionsOutput());
userSessionsOutput = userSessionsOutputMap.get(sessionOutput.getSessionId());
userConnectionMap.put(sessionOutput.getSessionId(), sessionOutput.getConnectedSystem());
try {
userConnectionMapEncrypted.put(cryptoService.encrypt(sessionOutput.getSessionId().toString()),
sessionOutput.getConnectedSystem());
} catch (GeneralSecurityException e) {
throw new RuntimeException(e);
}
userConnectionMapEncrypted.put(sessionOutput.getSessionId().toString().trim(),
sessionOutput.getConnectedSystem());
}
else {
if (userSessionsOutput.getSessionOutputMap().containsKey(sessionOutput.getSessionId())) {
Expand Down