Skip to content

Commit 6273cf7

Browse files
n1ckl0sk0rtgen1ckl0sk0rtge
authored
Fix/error with deep copy in cipher context translation (#193)
* add test case * update issue test, add edge case to translator, override deepCOpy for rsa and oeap Signed-off-by: Nicklas Körtge <nicklas.koertge1@ibm.com> --------- Signed-off-by: Nicklas Körtge <nicklas.koertge1@ibm.com>
1 parent d37d317 commit 6273cf7

File tree

5 files changed

+276
-4
lines changed

5 files changed

+276
-4
lines changed

‎java/src/test/files/rules/issues/ClientEncryptionJavaDuplicatedRSADetectionTestFile.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import javax.crypto.Cipher;
2+
import javax.crypto.spec.OAEPParameterSpec;
3+
import javax.crypto.spec.PSource;
4+
import java.security.GeneralSecurityException;
5+
import java.security.Key;
6+
import java.security.PrivateKey;
7+
import java.security.PublicKey;
8+
import java.security.spec.MGF1ParameterSpec;
9+
10+
public class ClientEncryptionJavaDuplicatedRSADetectionTestFile {
11+
12+
private static final String ASYMMETRIC_CYPHER = "RSA/ECB/OAEPWith{ALG}AndMGF1Padding";
13+
private static final String SYMMETRIC_KEY_TYPE = "AES";
14+
15+
public static byte[] wrapSecretKey(PublicKey publicKey, Key privateKey, String oaepDigestAlgorithm) throws Exception {
16+
try {
17+
MGF1ParameterSpec mgf1ParameterSpec = new MGF1ParameterSpec(oaepDigestAlgorithm);
18+
String asymmetricCipher = ASYMMETRIC_CYPHER.replace("{ALG}", mgf1ParameterSpec.getDigestAlgorithm());
19+
Cipher cipher = Cipher.getInstance(asymmetricCipher); // Noncompliant {{(PublicKeyEncryption) RSA-OAEP}}
20+
cipher.init(Cipher.WRAP_MODE, publicKey, getOaepParameterSpec(mgf1ParameterSpec));
21+
return cipher.wrap(privateKey);
22+
} catch (GeneralSecurityException e) {
23+
throw e;
24+
}
25+
}
26+
27+
public static Key unwrapSecretKey(PrivateKey decryptionKey, byte[] keyBytes, String oaepDigestAlgorithm) throws Exception {
28+
if (!oaepDigestAlgorithm.contains("-")) {
29+
oaepDigestAlgorithm = oaepDigestAlgorithm.replace("SHA", "SHA-");
30+
}
31+
try {
32+
MGF1ParameterSpec mgf1ParameterSpec = new MGF1ParameterSpec(oaepDigestAlgorithm);
33+
String asymmetricCipher = ASYMMETRIC_CYPHER.replace("{ALG}", mgf1ParameterSpec.getDigestAlgorithm());
34+
Cipher cipher = Cipher.getInstance(asymmetricCipher); // Noncompliant {{(PublicKeyEncryption) RSA-OAEP}}
35+
cipher.init(Cipher.UNWRAP_MODE, decryptionKey, getOaepParameterSpec(mgf1ParameterSpec));
36+
return cipher.unwrap(keyBytes, SYMMETRIC_KEY_TYPE, Cipher.SECRET_KEY);
37+
} catch (GeneralSecurityException e) {
38+
throw e;
39+
}
40+
}
41+
42+
private static OAEPParameterSpec getOaepParameterSpec(MGF1ParameterSpec mgf1ParameterSpec) {
43+
return new OAEPParameterSpec(mgf1ParameterSpec.getDigestAlgorithm(), "MGF1", mgf1ParameterSpec, PSource.PSpecified.DEFAULT);
44+
}
45+
}

‎java/src/test/java/com/ibm/plugin/rules/issues/ClientEncryptionJavaDuplicatedRSADetectionTest.java‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
/*
2+
* SonarQube Cryptography Plugin
3+
* Copyright (C) 2024 IBM
4+
*
5+
* Licensed to the Apache Software Foundation (ASF) under one or more
6+
* contributor license agreements. See the NOTICE file distributed with
7+
* this work for additional information regarding copyright ownership.
8+
* The ASF licenses this file to you under the Apache License, Version 2.0
9+
* (the "License"); you may not use this file except in compliance with
10+
* the License. You may obtain a copy of the License at
11+
*
12+
* http://www.apache.org/licenses/LICENSE-2.0
13+
*
14+
* Unless required by applicable law or agreed to in writing, software
15+
* distributed under the License is distributed on an "AS IS" BASIS,
16+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
17+
* See the License for the specific language governing permissions and
18+
* limitations under the License.
19+
*/
20+
package com.ibm.plugin.rules.issues;
21+
22+
import com.ibm.engine.detection.DetectionStore;
23+
import com.ibm.engine.model.Algorithm;
24+
import com.ibm.engine.model.CipherAction;
25+
import com.ibm.engine.model.IValue;
26+
import com.ibm.engine.model.OperationMode;
27+
import com.ibm.engine.model.context.CipherContext;
28+
import com.ibm.mapper.model.INode;
29+
import com.ibm.mapper.model.KeyLength;
30+
import com.ibm.mapper.model.Mode;
31+
import com.ibm.mapper.model.Oid;
32+
import com.ibm.mapper.model.Padding;
33+
import com.ibm.mapper.model.PublicKeyEncryption;
34+
import com.ibm.mapper.model.functionality.Decapsulate;
35+
import com.ibm.mapper.model.functionality.Encapsulate;
36+
import com.ibm.plugin.TestBase;
37+
import org.junit.jupiter.api.Disabled;
38+
import org.junit.jupiter.api.Test;
39+
import org.sonar.java.checks.verifier.CheckVerifier;
40+
import org.sonar.plugins.java.api.JavaCheck;
41+
import org.sonar.plugins.java.api.JavaFileScannerContext;
42+
import org.sonar.plugins.java.api.semantic.Symbol;
43+
import org.sonar.plugins.java.api.tree.Tree;
44+
45+
import javax.annotation.Nonnull;
46+
import java.util.List;
47+
48+
import static org.assertj.core.api.Assertions.assertThat;
49+
50+
class ClientEncryptionJavaDuplicatedRSADetectionTest extends TestBase {
51+
52+
@Test
53+
@Disabled
54+
void test() {
55+
CheckVerifier.newVerifier()
56+
.onFile(
57+
"src/test/files/rules/issues/ClientEncryptionJavaDuplicatedRSADetectionTestFile.java")
58+
.withChecks(this)
59+
.verifyIssues();
60+
}
61+
62+
@Override
63+
public void asserts(
64+
int findingId,
65+
@Nonnull DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> detectionStore,
66+
@Nonnull List<INode> nodes) {
67+
68+
if (findingId == 0) {
69+
/*
70+
* Detection Store
71+
*/
72+
73+
assertThat(detectionStore.getDetectionValues()).hasSize(1);
74+
assertThat(detectionStore.getDetectionValueContext()).isInstanceOf(CipherContext.class);
75+
IValue<Tree> value0 = detectionStore.getDetectionValues().get(0);
76+
assertThat(value0).isInstanceOf(Algorithm.class);
77+
assertThat(value0.asString()).isEqualTo("RSA/ECB/OAEPWith{ALG}AndMGF1Padding");
78+
79+
DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> store_1 =
80+
getStoreOfValueType(OperationMode.class, detectionStore.getChildren());
81+
assertThat(store_1.getDetectionValues()).hasSize(1);
82+
assertThat(store_1.getDetectionValueContext()).isInstanceOf(CipherContext.class);
83+
IValue<Tree> value0_1 = store_1.getDetectionValues().get(0);
84+
assertThat(value0_1).isInstanceOf(OperationMode.class);
85+
assertThat(value0_1.asString()).isEqualTo("3");
86+
87+
DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> store_2 =
88+
getStoreOfValueType(CipherAction.class, detectionStore.getChildren());
89+
assertThat(store_2.getDetectionValues()).hasSize(1);
90+
assertThat(store_2.getDetectionValueContext()).isInstanceOf(CipherContext.class);
91+
IValue<Tree> value0_2 = store_2.getDetectionValues().get(0);
92+
assertThat(value0_2).isInstanceOf(CipherAction.class);
93+
assertThat(value0_2.asString()).isEqualTo("WRAP");
94+
95+
/*
96+
* Translation
97+
*/
98+
99+
assertThat(nodes).hasSize(1);
100+
101+
// PublicKeyEncryption
102+
INode publicKeyEncryptionNode = nodes.get(0);
103+
assertThat(publicKeyEncryptionNode.getKind()).isEqualTo(PublicKeyEncryption.class);
104+
assertThat(publicKeyEncryptionNode.getChildren()).hasSize(5);
105+
assertThat(publicKeyEncryptionNode.asString()).isEqualTo("RSA-OAEP");
106+
107+
// KeyLength under PublicKeyEncryption
108+
INode keyLengthNode = publicKeyEncryptionNode.getChildren().get(KeyLength.class);
109+
assertThat(keyLengthNode).isNotNull();
110+
assertThat(keyLengthNode.getChildren()).isEmpty();
111+
assertThat(keyLengthNode.asString()).isEqualTo("2048");
112+
113+
// Oid under PublicKeyEncryption
114+
INode oidNode = publicKeyEncryptionNode.getChildren().get(Oid.class);
115+
assertThat(oidNode).isNotNull();
116+
assertThat(oidNode.getChildren()).isEmpty();
117+
assertThat(oidNode.asString()).isEqualTo("1.2.840.113549.1.1.7");
118+
119+
// Padding under PublicKeyEncryption
120+
INode paddingNode = publicKeyEncryptionNode.getChildren().get(Padding.class);
121+
assertThat(paddingNode).isNotNull();
122+
assertThat(paddingNode.getChildren()).isEmpty();
123+
assertThat(paddingNode.asString()).isEqualTo("OAEP");
124+
125+
// Encapsulate under PublicKeyEncryption
126+
INode encapsulateNode = publicKeyEncryptionNode.getChildren().get(Encapsulate.class);
127+
assertThat(encapsulateNode).isNotNull();
128+
assertThat(encapsulateNode.getChildren()).isEmpty();
129+
assertThat(encapsulateNode.asString()).isEqualTo("ENCAPSULATE");
130+
131+
// Mode under PublicKeyEncryption
132+
INode modeNode = publicKeyEncryptionNode.getChildren().get(Mode.class);
133+
assertThat(modeNode).isNotNull();
134+
assertThat(modeNode.getChildren()).isEmpty();
135+
assertThat(modeNode.asString()).isEqualTo("ECB");
136+
137+
} else {
138+
/*
139+
* Detection Store
140+
*/
141+
142+
assertThat(detectionStore.getDetectionValues()).hasSize(1);
143+
assertThat(detectionStore.getDetectionValueContext()).isInstanceOf(CipherContext.class);
144+
IValue<Tree> value0 = detectionStore.getDetectionValues().get(0);
145+
assertThat(value0).isInstanceOf(Algorithm.class);
146+
assertThat(value0.asString()).isEqualTo("RSA/ECB/OAEPWith{ALG}AndMGF1Padding");
147+
148+
DetectionStore<JavaCheck, Tree, Symbol, JavaFileScannerContext> store_1 =
149+
getStoreOfValueType(OperationMode.class, detectionStore.getChildren());
150+
assertThat(store_1.getDetectionValues()).hasSize(1);
151+
assertThat(store_1.getDetectionValueContext()).isInstanceOf(CipherContext.class);
152+
IValue<Tree> value0_1 = store_1.getDetectionValues().get(0);
153+
assertThat(value0_1).isInstanceOf(OperationMode.class);
154+
assertThat(value0_1.asString()).isEqualTo("4");
155+
156+
/*
157+
* Translation
158+
*/
159+
assertThat(nodes).hasSize(1);
160+
161+
// PublicKeyEncryption
162+
INode publicKeyEncryptionNode1 = nodes.get(0);
163+
assertThat(publicKeyEncryptionNode1.getKind()).isEqualTo(PublicKeyEncryption.class);
164+
assertThat(publicKeyEncryptionNode1.getChildren()).hasSize(5);
165+
assertThat(publicKeyEncryptionNode1.asString()).isEqualTo("RSA-OAEP");
166+
167+
// Mode under PublicKeyEncryption
168+
INode modeNode1 = publicKeyEncryptionNode1.getChildren().get(Mode.class);
169+
assertThat(modeNode1).isNotNull();
170+
assertThat(modeNode1.getChildren()).isEmpty();
171+
assertThat(modeNode1.asString()).isEqualTo("ECB");
172+
173+
// Decapsulate under PublicKeyEncryption
174+
INode decapsulateNode = publicKeyEncryptionNode1.getChildren().get(Decapsulate.class);
175+
assertThat(decapsulateNode).isNotNull();
176+
assertThat(decapsulateNode.getChildren()).isEmpty();
177+
assertThat(decapsulateNode.asString()).isEqualTo("DECAPSULATE");
178+
179+
// Oid under PublicKeyEncryption
180+
INode oidNode1 = publicKeyEncryptionNode1.getChildren().get(Oid.class);
181+
assertThat(oidNode1).isNotNull();
182+
assertThat(oidNode1.getChildren()).isEmpty();
183+
assertThat(oidNode1.asString()).isEqualTo("1.2.840.113549.1.1.7");
184+
185+
// Padding under PublicKeyEncryption
186+
INode paddingNode1 = publicKeyEncryptionNode1.getChildren().get(Padding.class);
187+
assertThat(paddingNode1).isNotNull();
188+
assertThat(paddingNode1.getChildren()).isEmpty();
189+
assertThat(paddingNode1.asString()).isEqualTo("OAEP");
190+
191+
// KeyLength under PublicKeyEncryption
192+
INode keyLengthNode1 = publicKeyEncryptionNode1.getChildren().get(KeyLength.class);
193+
assertThat(keyLengthNode1).isNotNull();
194+
assertThat(keyLengthNode1.getChildren()).isEmpty();
195+
assertThat(keyLengthNode1.asString()).isEqualTo("2048");
196+
}
197+
}
198+
}

‎mapper/src/main/java/com/ibm/mapper/ITranslator.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ private Map<Integer, List<INode>> translateStore(@Nonnull DetectionStore<R, T, S
7575
.ifPresent(
7676
actionValue -> {
7777
final Optional<INode> translatedNode =
78-
translate(bundle, actionValue, context, filePath);
78+
this.translate(bundle, actionValue, context, filePath);
7979
translatedNode.ifPresent(
8080
node -> {
8181
final List<INode> newNodes = new ArrayList<>();
@@ -88,7 +88,7 @@ private Map<Integer, List<INode>> translateStore(@Nonnull DetectionStore<R, T, S
8888
final List<INode> translatedNodesForId = new ArrayList<>();
8989
for (IValue<T> value : values) {
9090
final Optional<INode> translatedNode =
91-
translate(bundle, value, context, filePath);
91+
this.translate(bundle, value, context, filePath);
9292
translatedNode.ifPresent(translatedNodesForId::add);
9393
}
9494
// to get the list for the key, or create a new one if it doesn't exist and add
@@ -224,8 +224,9 @@ private void append(
224224
.forEach(mergedCollectionNode::put);
225225

226226
parentNode.put(mergedCollectionNode);
227-
228-
} else {
227+
} else if (existingNode.is(childNode.getKind())
228+
&& !existingNode.asString().equals(childNode.asString())) {
229+
// add node to new roots
229230
final INode newParent = parentNode.deepCopy();
230231
newParent.put(childNode);
231232
newRoots.add(newParent);

‎mapper/src/main/java/com/ibm/mapper/model/algorithms/RSA.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,18 @@ public RSA(
9494
public RSA(@Nonnull final Class<? extends IPrimitive> asKind, @Nonnull RSA rsa) {
9595
super(rsa, asKind);
9696
}
97+
98+
private RSA(@Nonnull final RSA rsa) {
99+
super(rsa.name, rsa.kind, rsa.detectionLocation);
100+
}
101+
102+
@Nonnull
103+
@Override
104+
public INode deepCopy() {
105+
RSA copy = new RSA(this);
106+
for (INode child : this.children.values()) {
107+
copy.children.put(child.getKind(), child.deepCopy());
108+
}
109+
return copy;
110+
}
97111
}

‎mapper/src/main/java/com/ibm/mapper/model/padding/OAEP.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,18 @@ public Optional<MaskGenerationFunction> getMGF() {
6666
}
6767
return Optional.of((MaskGenerationFunction) node);
6868
}
69+
70+
private OAEP(@Nonnull OAEP oaep) {
71+
super(oaep.getName(), oaep.getDetectionContext(), Padding.class);
72+
}
73+
74+
@Nonnull
75+
@Override
76+
public INode deepCopy() {
77+
OAEP copy = new OAEP(this);
78+
for (INode child : this.children.values()) {
79+
copy.children.put(child.getKind(), child.deepCopy());
80+
}
81+
return copy;
82+
}
6983
}

0 commit comments

Comments
 (0)