diff --git a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll index c7cb3a10331..b10bc83c64a 100644 --- a/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll +++ b/java/ql/lib/semmle/code/java/security/InsufficientKeySizeQuery.qll @@ -1,4 +1,5 @@ import semmle.code.java.security.Encryption +import semmle.code.java.dataflow.TaintTracking2 import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.DataFlow @@ -11,7 +12,7 @@ import semmle.code.java.dataflow.DataFlow /** * Asymmetric (RSA, DSA, DH) key length data flow tracking configuration. */ -class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration { +class AsymmetricKeyTrackingConfiguration extends TaintTracking2::Configuration { AsymmetricKeyTrackingConfiguration() { this = "AsymmetricKeyTrackingConfiguration" } override predicate isSource(DataFlow::Node source) { @@ -27,15 +28,25 @@ class AsymmetricKeyTrackingConfiguration extends DataFlow::Configuration { override predicate isSink(DataFlow::Node sink) { exists(MethodAccess ma, VarAccess va | ma.getMethod() instanceof KeyPairGeneratorInitMethod and - va.getVariable() - .getAnAssignedValue() - .(JavaSecurityKeyPairGenerator) - .getAlgoSpec() - .(StringLiteral) - .getValue() - .toUpperCase() - .matches(["RSA", "DSA", "DH"]) and - ma.getQualifier() = va and + ma.getFile().getBaseName().matches("SignatureTest.java") and + // va.getVariable() + // .getAnAssignedValue() + // .(JavaSecurityKeyPairGenerator) + // .getAlgoSpec() + // .(StringLiteral) + // .getValue() + // .toUpperCase() + // .matches(["RSA", "DSA", "DH"]) and + // ma.getQualifier() = va and + exists( + JavaSecurityKeyPairGenerator jpg, KeyPairGeneratorInitConfiguration kpgConfig, + DataFlow::PathNode source, DataFlow::PathNode dest + | + jpg.getAlgoSpec().(StringLiteral).getValue().toUpperCase().matches(["RSA", "DSA", "DH"]) and + source.getNode().asExpr() = jpg and + dest.getNode().asExpr() = ma.getQualifier() and + kpgConfig.hasFlowPath(source, dest) + ) and sink.asExpr() = ma.getArgument(0) ) } @@ -102,12 +113,11 @@ class SymmetricKeyTrackingConfiguration extends DataFlow::Configuration { } // ! below doesn't work for some reason... -predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) { - exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) - or - exists(SymmetricKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) -} - +// predicate hasInsufficientKeySize2(DataFlow::PathNode source, DataFlow::PathNode sink) { +// exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) +// or +// exists(SymmetricKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) +// } // ******** Need the below for the above ******** // ! move to Encryption.qll? /** The Java class `java.security.spec.ECGenParameterSpec`. */ diff --git a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql index c6dce740588..9b52707c594 100644 --- a/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql +++ b/java/ql/src/Security/CWE/CWE-326/InsufficientKeySize.ql @@ -26,9 +26,17 @@ import DataFlow::PathGraph // cfg2.hasFlowPath(source, sink) // select sink.getNode(), source, sink, "The $@ of an asymmetric key should be at least 2048 bits.", // sink.getNode(), "size" -from DataFlow::PathNode source, DataFlow::PathNode sink -where - exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) or - exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) or - exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink)) -select sink.getNode(), source, sink, "This $@ is too small.", sink.getNode(), "key size" +// * Use Below +// from DataFlow::PathNode source, DataFlow::PathNode sink +// where exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlowPath(source, sink)) //or +// //exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) //or +// //exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink)) +// select sink.getNode(), source, sink, "This $@ is too small, and flows to $@.", source.getNode(), +// "key size", sink.getNode(), "here" +// * Use Above +from DataFlow::Node source, DataFlow::Node sink +where exists(AsymmetricKeyTrackingConfiguration config1 | config1.hasFlow(source, sink)) //or +//exists(AsymmetricECCKeyTrackingConfiguration config2 | config2.hasFlowPath(source, sink)) //or +//exists(SymmetricKeyTrackingConfiguration config3 | config3.hasFlowPath(source, sink)) +select sink, source, sink, "This $@ is too small, and flows to $@.", source, "key size", sink, + "here" diff --git a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java index b383892361b..75ba5e36e76 100644 --- a/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java +++ b/java/ql/test/query-tests/security/CWE-326/InsufficientKeySizeTest.java @@ -1,7 +1,11 @@ +import javax.crypto.KeyGenerator; import java.security.KeyPairGenerator; + import java.security.spec.ECGenParameterSpec; import java.security.spec.RSAKeyGenParameterSpec; -import javax.crypto.KeyGenerator; +import java.security.spec.DSAGenParameterSpec; +import javax.crypto.spec.DHGenParameterSpec; + public class InsufficientKeySizeTest { public void keySizeTesting() throws java.security.NoSuchAlgorithmException, java.security.InvalidAlgorithmParameterException { @@ -49,6 +53,16 @@ public class InsufficientKeySizeTest { // GOOD: Key size is no less than 2048 KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DSA"); keyPairGen4.initialize(2048); // Safe + + // test with spec? + // // BAD: Key size is less than 2048 + // KeyPairGenerator keyPairGen5 = KeyPairGenerator.getInstance("DSA"); + // DSAGenParameterSpec dsaSpec = new DSAGenParameterSpec(1024, null); + // keyPairGen5.initialize(dsaSpec); // $ hasInsufficientKeySize + + // // BAD: Key size is less than 2048 + // KeyPairGenerator keyPairGen6 = KeyPairGenerator.getInstance("DSA"); + // keyPairGen6.initialize(new DSAGenParameterSpec(1024, null)); // $ hasInsufficientKeySize } // DH (Asymmetric) @@ -60,6 +74,16 @@ public class InsufficientKeySizeTest { // GOOD: Key size is no less than 2048 KeyPairGenerator keyPairGen17 = KeyPairGenerator.getInstance("DH"); keyPairGen17.initialize(2048); // Safe + + // test with spec? + // // BAD: Key size is less than 2048 + // KeyPairGenerator keyPairGen3 = KeyPairGenerator.getInstance("DH"); + // DHGenParameterSpec dhSpec = new DHGenParameterSpec(1024, null); + // keyPairGen3.initialize(dhSpec); // $ hasInsufficientKeySize + + // // BAD: Key size is less than 2048 + // KeyPairGenerator keyPairGen4 = KeyPairGenerator.getInstance("DH"); + // keyPairGen4.initialize(new DHGenParameterSpec(1024, null)); // $ hasInsufficientKeySize } // EC (Asymmetric) diff --git a/java/ql/test/query-tests/security/CWE-326/SignatureTest.java b/java/ql/test/query-tests/security/CWE-326/SignatureTest.java new file mode 100644 index 00000000000..016a62a41f8 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-326/SignatureTest.java @@ -0,0 +1,196 @@ +//package org.bouncycastle.jce.provider.test; + +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.SecureRandom; +import java.security.Security; +import java.security.Signature; + +// import org.bouncycastle.asn1.cryptopro.CryptoProObjectIdentifiers; +// import org.bouncycastle.jce.provider.BouncyCastleProvider; +// import org.bouncycastle.jce.spec.ECNamedCurveGenParameterSpec; +// import org.bouncycastle.jce.spec.GOST3410ParameterSpec; +// import org.bouncycastle.util.encoders.Hex; +// import org.bouncycastle.util.test.SimpleTest; + +public class SignatureTest + //extends SimpleTest +{ + // private static final byte[] DATA = Hex.decode("00000000deadbeefbeefdeadffffffff00000000"); + + private void checkSig(KeyPair kp, String name) + throws Exception + { + // Signature sig = Signature.getInstance(name, "BC"); + + // sig.initSign(kp.getPrivate()); + // sig.update(DATA); + + // byte[] signature1 = sig.sign(); + + // sig.update(DATA); + + // byte[] signature2 = sig.sign(); + + // sig.initVerify(kp.getPublic()); + + // sig.update(DATA); + // if (!sig.verify(signature1)) + // { + // fail("did not verify: " + name); + // } + + // // After verify, should be reusable as if we are after initVerify + // sig.update(DATA); + // if (!sig.verify(signature1)) + // { + // fail("second verify failed: " + name); + // } + + // sig.update(DATA); + // if (!sig.verify(signature2)) + // { + // fail("second verify failed (2): " + name); + // } + } + + public void performTest() + throws Exception + { + KeyPairGenerator kpGen = KeyPairGenerator.getInstance("RSA", "BC"); + + kpGen.initialize(2048); // Safe + + KeyPair kp = kpGen.generateKeyPair(); + + checkSig(kp, "SHA1withRSA"); + checkSig(kp, "SHA224withRSA"); + checkSig(kp, "SHA256withRSA"); + checkSig(kp, "SHA384withRSA"); + checkSig(kp, "SHA512withRSA"); + + checkSig(kp, "SHA3-224withRSA"); + checkSig(kp, "SHA3-256withRSA"); + checkSig(kp, "SHA3-384withRSA"); + checkSig(kp, "SHA3-512withRSA"); + + checkSig(kp, "MD2withRSA"); + checkSig(kp, "MD4withRSA"); + checkSig(kp, "MD5withRSA"); + checkSig(kp, "RIPEMD160withRSA"); + checkSig(kp, "RIPEMD128withRSA"); + checkSig(kp, "RIPEMD256withRSA"); + + checkSig(kp, "SHA1withRSAandMGF1"); + checkSig(kp, "SHA1withRSAandMGF1"); + checkSig(kp, "SHA224withRSAandMGF1"); + checkSig(kp, "SHA256withRSAandMGF1"); + checkSig(kp, "SHA384withRSAandMGF1"); + checkSig(kp, "SHA512withRSAandMGF1"); + + checkSig(kp, "SHA1withRSAandSHAKE128"); + checkSig(kp, "SHA1withRSAandSHAKE128"); + checkSig(kp, "SHA224withRSAandSHAKE128"); + checkSig(kp, "SHA256withRSAandSHAKE128"); + checkSig(kp, "SHA384withRSAandSHAKE128"); + checkSig(kp, "SHA512withRSAandSHAKE128"); + + checkSig(kp, "SHA1withRSAandSHAKE256"); + checkSig(kp, "SHA1withRSAandSHAKE256"); + checkSig(kp, "SHA224withRSAandSHAKE256"); + checkSig(kp, "SHA256withRSAandSHAKE256"); + checkSig(kp, "SHA384withRSAandSHAKE256"); + checkSig(kp, "SHA512withRSAandSHAKE256"); + + checkSig(kp, "SHAKE128withRSAPSS"); + checkSig(kp, "SHAKE256withRSAPSS"); + + checkSig(kp, "SHA1withRSA/ISO9796-2"); + checkSig(kp, "MD5withRSA/ISO9796-2"); + checkSig(kp, "RIPEMD160withRSA/ISO9796-2"); + +// checkSig(kp, "SHA1withRSA/ISO9796-2PSS"); +// checkSig(kp, "MD5withRSA/ISO9796-2PSS"); +// checkSig(kp, "RIPEMD160withRSA/ISO9796-2PSS"); + + checkSig(kp, "RIPEMD128withRSA/X9.31"); + checkSig(kp, "RIPEMD160withRSA/X9.31"); + checkSig(kp, "SHA1withRSA/X9.31"); + checkSig(kp, "SHA224withRSA/X9.31"); + checkSig(kp, "SHA256withRSA/X9.31"); + checkSig(kp, "SHA384withRSA/X9.31"); + checkSig(kp, "SHA512withRSA/X9.31"); + checkSig(kp, "WhirlpoolwithRSA/X9.31"); + + kpGen = KeyPairGenerator.getInstance("DSA", "BC"); + + kpGen.initialize(2048); // Safe + + kp = kpGen.generateKeyPair(); + + checkSig(kp, "SHA1withDSA"); + checkSig(kp, "SHA224withDSA"); + checkSig(kp, "SHA256withDSA"); + checkSig(kp, "SHA384withDSA"); + checkSig(kp, "SHA512withDSA"); + checkSig(kp, "NONEwithDSA"); + + kpGen = KeyPairGenerator.getInstance("EC", "BC"); + + kpGen.initialize(256); // Safe + + kp = kpGen.generateKeyPair(); + + checkSig(kp, "SHA1withECDSA"); + checkSig(kp, "SHA224withECDSA"); + checkSig(kp, "SHA256withECDSA"); + checkSig(kp, "SHA384withECDSA"); + checkSig(kp, "SHA512withECDSA"); + checkSig(kp, "RIPEMD160withECDSA"); + checkSig(kp, "SHAKE128withECDSA"); + checkSig(kp, "SHAKE256withECDSA"); + + kpGen = KeyPairGenerator.getInstance("EC", "BC"); + + kpGen.initialize(521); // Safe + + kp = kpGen.generateKeyPair(); + + checkSig(kp, "SHA1withECNR"); + checkSig(kp, "SHA224withECNR"); + checkSig(kp, "SHA256withECNR"); + checkSig(kp, "SHA384withECNR"); + checkSig(kp, "SHA512withECNR"); + + // kpGen = KeyPairGenerator.getInstance("ECGOST3410", "BC"); + + // kpGen.initialize(new ECNamedCurveGenParameterSpec("GostR3410-2001-CryptoPro-A"), new SecureRandom()); + + // kp = kpGen.generateKeyPair(); + + // checkSig(kp, "GOST3411withECGOST3410"); + + // kpGen = KeyPairGenerator.getInstance("GOST3410", "BC"); + + // GOST3410ParameterSpec gost3410P = new GOST3410ParameterSpec(CryptoProObjectIdentifiers.gostR3410_94_CryptoPro_A.getId()); + + // kpGen.initialize(gost3410P); + + // kp = kpGen.generateKeyPair(); + + // checkSig(kp, "GOST3411withGOST3410"); + } + + public String getName() + { + return "SigNameTest"; + } + + // public static void main( + // String[] args) + // { + // //Security.addProvider(new BouncyCastleProvider()); + + // //runTest(new SignatureTest()); + // } +}