Skip to content

Commit f1008b7

Browse files
authored
Merge pull request #37 from SentriusLLC/alert-autofix-20
Potential fix for code scanning alert no. 20: Use of a broken or risky cryptographic algorithm
2 parents 143dc98 + 1be280e commit f1008b7

File tree

8 files changed

+61
-43
lines changed

8 files changed

+61
-43
lines changed

api/src/main/java/io/sentrius/sso/controllers/api/HostApiController.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,9 @@ public ResponseEntity<ObjectNode> connectSSHServer(HttpServletRequest request, H
238238

239239
sessionMetadata = terminalSessionMetadataService.createSession(sessionMetadata);
240240

241-
var encryptedSessionId = cryptoService.encrypt(connectedSystem.getSession().getId().toString());
241+
var encryptedSessionId = cryptoService.encrypt(connectedSystem.getSession().getId().toString().trim());
242242

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

245245
node.put("sessionId", encryptedSessionId);
246246

api/src/main/java/io/sentrius/sso/websocket/ChatWSHandler.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,9 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
8989
return;
9090
}
9191
// Decrypt the session ID
92-
// var sessionIdStr = cryptoService.decrypt(sessionId);
93-
// var sessionIdLong = Long.parseLong(sessionIdStr);
94-
var lookupId = sessionId + "==";
92+
var sessionIdStr = cryptoService.decrypt(sessionId);
9593
// Retrieve ConnectedSystem from your persistent map using the session ID
96-
var sys = sessionTrackingService.getEncryptedConnectedSession(lookupId);
94+
var sys = sessionTrackingService.getEncryptedConnectedSession(sessionIdStr);
9795
if (null != sys ) {
9896
log.info("oh");
9997
// Get the user's session and handle trigger if present

api/src/main/java/io/sentrius/sso/websocket/TerminalWSHandler.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void afterConnectionEstablished(WebSocketSession session) throws Exceptio
5757
if (sessionId != null) {
5858
// Store the WebSocket session using the session ID from the query parameter
5959
sessions.put(sessionId, session);
60-
log.info("New connection established, session ID: " + sessionId);
60+
log.debug("New connection established, session ID: " + sessionId);
6161
sshListenerService.startListeningToSshServer(sessionId, session);
6262
} else {
6363
log.trace("Session ID not found in query parameters.");
@@ -75,14 +75,14 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
7575

7676
// Extract query parameters from the URI again if needed
7777
URI uri = session.getUri();
78-
log.trace("got message {}", uri);
78+
log.debug("got message {}", uri);
7979
try {
8080
if (uri != null) {
8181
Map<String, String> queryParams = parseQueryParams(uri.getQuery());
8282
String sessionId = queryParams.get("sessionId");
8383

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

8888

@@ -91,11 +91,10 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
9191
Session.TerminalMessage auditLog =
9292
Session.TerminalMessage.parseFrom(messageBytes);
9393
// Decrypt the session ID
94-
// var sessionIdStr = cryptoService.decrypt(sessionId);
95-
// var sessionIdLong = Long.parseLong(sessionIdStr);
96-
var lookupId = sessionId + "==";
94+
var sessionIdStr = cryptoService.decrypt(sessionId);
95+
var lookupId = sessionId; // + "==";
9796
// Retrieve ConnectedSystem from your persistent map using the session ID
98-
var sys = sessionTrackingService.getEncryptedConnectedSession(lookupId);
97+
var sys = sessionTrackingService.getEncryptedConnectedSession(sessionIdStr);
9998
if (null != sys ) {
10099
boolean allNoAction = true;
101100
log.debug("**** Processing message for session ID: {} with {} actions", sessionId,
@@ -125,14 +124,16 @@ protected void handleTextMessage(WebSocketSession session, TextMessage message)
125124
}
126125
}
127126
if (allNoAction && sys.getSessionStartupActions().size() > 0) {
128-
log.info("**** Setting NO_ACTION Trigger");
127+
log.debug("**** Setting NO_ACTION Trigger");
129128
var noActionTrigger = new Trigger(TriggerAction.NO_ACTION, "");
130129
sessionTrackingService.addSystemTrigger(sys, noActionTrigger);
131130
sys.getTerminalAuditor().setSessionTrigger(noActionTrigger);
132131
}
133132

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

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

170-
log.info("Connection closed, session ID: " + sessionId);
171+
log.debug("Connection closed, session ID: {}", sessionId);
171172
}
172173
}
173174
}

api/src/main/resources/static/node/js/sockjs-client/sockjs.js

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5227,6 +5227,3 @@ module.exports = Url;
52275227

52285228
},{"querystringify":58,"requires-port":59}]},{},[1])(1)
52295229
});
5230-
5231-
5232-
//# sourceMappingURL=sockjs.js.map

build-images-gcp.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ if $update_sentrius_keycloak; then
8787
fi
8888

8989
if $update_sentrius_agent; then
90-
cp java-agents/target/java-agents-*.jar docker/sentrius-agent/agent.jar
90+
cp java-agents/target/java-agent-*.jar docker/sentrius-agent/agent.jar
9191
SENTRIUS_AGENT_VERSION=$(increment_patch_version $SENTRIUS_AGENT_VERSION)
9292
build_and_push_image "sentrius-agent" "$SENTRIUS_AGENT_VERSION" "./docker/sentrius-agent"
9393
rm docker/sentrius-agent/agent.jar

build-images-local.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ if $update_sentrius_keycloak; then
8787
fi
8888

8989
if $update_sentrius_agent; then
90-
cp java-agents/target/java-agents-*.jar docker/sentrius-agent/agent.jar
90+
cp java-agents/target/java-agent-*.jar docker/sentrius-agent/agent.jar
9191
SENTRIUS_AGENT_VERSION=$(increment_patch_version $SENTRIUS_AGENT_VERSION)
9292
build_image "sentrius-agent" "$SENTRIUS_AGENT_VERSION" "./docker/sentrius-agent"
9393
rm docker/sentrius-agent/agent.jar

core/src/main/java/io/sentrius/sso/core/security/service/CryptoService.java

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
package io.sentrius.sso.core.security.service;
22

33
import javax.crypto.Cipher;
4+
import javax.crypto.spec.GCMParameterSpec;
45
import javax.crypto.spec.SecretKeySpec;
56
import java.io.IOException;
67
import java.nio.charset.StandardCharsets;
78
import java.security.GeneralSecurityException;
89
import java.security.MessageDigest;
910
import java.security.NoSuchAlgorithmException;
1011
import java.security.SecureRandom;
12+
import java.util.Arrays;
1113
import java.util.Base64;
1214
import com.jcraft.jsch.JSch;
1315
import com.jcraft.jsch.JSchException;
@@ -30,7 +32,7 @@ public class CryptoService {
3032
final ApplicationKeyRepository applicationKeyRepository;
3133
private final byte[] key;
3234

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

6668
public String encrypt(String str) throws GeneralSecurityException {
6769
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
68-
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
70+
byte[] iv = new byte[12];
71+
new SecureRandom().nextBytes(iv);
72+
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
73+
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);
6974
byte[] encVal = cipher.doFinal(str.getBytes(StandardCharsets.UTF_8));
70-
return Base64.getEncoder().encodeToString(encVal);
75+
byte[] encryptedIvAndText = new byte[iv.length + encVal.length];
76+
System.arraycopy(iv, 0, encryptedIvAndText, 0, iv.length);
77+
System.arraycopy(encVal, 0, encryptedIvAndText, iv.length, encVal.length);
78+
return Base64.getEncoder().encodeToString(encryptedIvAndText);
7179
}
7280

7381
public String encrypt(byte [] bytes) throws GeneralSecurityException {
7482
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
75-
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
83+
byte[] iv = new byte[12];
84+
new SecureRandom().nextBytes(iv);
85+
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
86+
cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);
7687
byte[] encVal = cipher.doFinal(bytes);
77-
return Base64.getEncoder().encodeToString(encVal);
88+
byte[] encryptedIvAndText = new byte[iv.length + encVal.length];
89+
System.arraycopy(iv, 0, encryptedIvAndText, 0, iv.length);
90+
System.arraycopy(encVal, 0, encryptedIvAndText, iv.length, encVal.length);
91+
return Base64.getEncoder().encodeToString(encryptedIvAndText);
7892
}
7993

8094
public String decrypt(String encryptedStr) throws GeneralSecurityException {
81-
Cipher cipher = Cipher.getInstance(CIPHER_INSTANCE);
82-
cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM));
95+
Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
96+
97+
// Decode Base64
8398
byte[] decodedVal = Base64.getDecoder().decode(encryptedStr);
84-
return new String(cipher.doFinal(decodedVal), StandardCharsets.UTF_8);
99+
100+
// Extract IV (first 12 bytes)
101+
byte[] iv = Arrays.copyOfRange(decodedVal, 0, 12);
102+
103+
// Extract actual ciphertext (rest of the bytes)
104+
byte[] cipherText = Arrays.copyOfRange(decodedVal, 12, decodedVal.length);
105+
106+
// Ensure we use the same IV for decryption
107+
GCMParameterSpec gcmSpec = new GCMParameterSpec(128, iv);
108+
cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(key, CRYPT_ALGORITHM), gcmSpec);
109+
110+
// Decrypt the text
111+
byte[] decryptedBytes = cipher.doFinal(cipherText);
112+
113+
return new String(decryptedBytes, StandardCharsets.UTF_8);
85114
}
86115

116+
87117
public String encodePassword(String password) throws NoSuchAlgorithmException {
88118
return encoder.encode(password);
89119
}

core/src/main/java/io/sentrius/sso/core/services/terminal/SessionTrackingService.java

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,8 @@ public List<ConnectedSystem> getConnectedSession() {
6868
*/
6969
public void removeUserSession(ConnectedSystem connectedSystem) {
7070
userConnectionMap.remove(connectedSystem.getSession().getId());
71-
try {
72-
userConnectionMapEncrypted.remove(
73-
cryptoService.encrypt(connectedSystem.getSession().getId().toString()) );
74-
} catch (GeneralSecurityException e) {
75-
throw new RuntimeException(e);
76-
}
71+
userConnectionMapEncrypted.remove(
72+
connectedSystem.getSession().getId().toString());
7773
UserSessionsOutput userSessionsOutput = userSessionsOutputMap.get(connectedSystem.getSession().getId());
7874
if (userSessionsOutput != null) {
7975
userSessionsOutput.getSessionOutputMap().clear();
@@ -107,12 +103,8 @@ public void addOutput(SessionOutput sessionOutput) {
107103
userSessionsOutputMap.put(sessionOutput.getSessionId(), new UserSessionsOutput());
108104
userSessionsOutput = userSessionsOutputMap.get(sessionOutput.getSessionId());
109105
userConnectionMap.put(sessionOutput.getSessionId(), sessionOutput.getConnectedSystem());
110-
try {
111-
userConnectionMapEncrypted.put(cryptoService.encrypt(sessionOutput.getSessionId().toString()),
112-
sessionOutput.getConnectedSystem());
113-
} catch (GeneralSecurityException e) {
114-
throw new RuntimeException(e);
115-
}
106+
userConnectionMapEncrypted.put(sessionOutput.getSessionId().toString().trim(),
107+
sessionOutput.getConnectedSystem());
116108
}
117109
else {
118110
if (userSessionsOutput.getSessionOutputMap().containsKey(sessionOutput.getSessionId())) {

0 commit comments

Comments
 (0)