Incorrect algorithm for AlgorithmParameters

One of the algorithms defined in AlgorithmParameters is OEAP, while the correct algorithm name should be OAEP (Optimal Asymmetric Encryption Padding).

Missing SHA3 algorithms BC-JCA

SHA3 algorithms are supported by BouncyCastle. Therefore, they should be added to the appropriate crysl rules. These commits help as a starting point.

Missing key constraints KeyAgreement

The current KeyAgreement crysl rule does not require the key used for the method calls init(...) and doPhase(...) to be generated. However, init should be called with a private key and doPhase with a public key of an other participant of the key agreement (like seen here).

Wrong aggregate order MAC

The current order section of the MAC crysl rule is incorrect. At the moment f1 and f3 can be called without an Updates call. And f2 has to be called after an Updates call. However, f2 = doFinal(byte[]) calls an update method (e.g. seen here) while f1,f3 do not (e.g. seen here).

KeyStore should support android

In the JCA and BouncyCalste-JCA KeyStore.crysl we currently only permit the values "jceks", "jks", "dks", "pkcs11", "pkcs12".

However as
androidx.security.crypto.MasterKeys.java suggests in Android Apps it's totally OK, if even not preferred to use the value AndroidKeyStore.

Some insecure configurations listed in rule

According to the crypto team of Oracle, we are missing some specifications/specified them insecurely:

Key size rules include insecure sizes (eg KeyPairGenerator - 1024 bit keys, DESede 112)
There are a number of secure algorithms for which there are no rules. Eg. ECC algorithms, CCM Cipher mode, AES KW (key wrap)
For PBEKeySpec, the iteration count rule for PBE is too small (>= 1000)

API usage rule for String

I don't understand this rule:
https://github.com/CROSSINGTUD/Crypto-API-Rules/blob/master/src/de/darmstadt/tu/crossing/String.cryptsl

What's is purpose? Surely we don't want to forbid all the FORBIDDEN_EVENTS for every string!?

Finite State Machine for KeyStore is wrong

Left: -1 ====[getInstance( alg);, getInstance( alg _);]====> Right:0
Left: 0 ====[load( fileinput passwordIn);, load( paramLoad);]====> Right:1
Left: 1 ====[getEntry( aliasGet protParamGet);, setEntry( aliasSet entry protParamSet);]====> Right:2
Left: 2 ====[store( paramStore);, store( fileoutput passwordOut);]====> Right:3

The regular expression is
Gets, Loads, (Entries, Stores)?

The FSM should have the form
-1 -> 0 [Gets]
0 ->1 [Loads]
1 -> 1 [Entries, Stores]

Specifications for Bouncy Castle CCM and CTR modes are missing

Section 2.1.1 of BSI documentation states CCM and CTR as recommended modes of operation for block ciphers. The following classes of Bouncy Castle library implement them.

All Predicates in REQUIRES block needs to be generated

According to the CrySL spec for BufferedBlockCipher, the EVENT block has two constructors

c1 : BufferedBlockCipher(mode);     // According to AESTest
c2 : BufferedBlockCipher(engine);   // BlockCipherVectorTest or BlockCipherMonteCarloTest has such usages
Cons := c1 | c2;

Hence the REQUIRES block has two predicates namely

generatedMode[mode];   // corresponding to BufferedBlockCipher(mode)
generatedEngine[engine];   // corresponding to BufferedBlockCipher(engine)

CryptoAnalysis throws RequiredPredicateError error markers for every predicate which is not generated. So even though I use only BufferedBlockCipher(mode) in my code and generate the mode properly, I get an error marker for the other constructor

Below is the code snippet of my test case

...
BlockCipher engine = new AESEngine();
BlockCipher mode = new CBCBlockCipher(engine);
BufferedBlockCipher cipher = new BufferedBlockCipher(mode);
...

What Cognicrypt outputs

RequiredPredicateError violating CrySL rule for BufferedBlockCipher
			First parameter was not properly generated as generated Engine
			at statement: specialinvoke $r10.<org.bouncycastle.crypto.BufferedBlockCipher: void <init>(org.bouncycastle.crypto.BlockCipher)>(r2)

Model Tink

https://github.com/google/tink/blob/master/docs/JAVA-HOWTO.md

Model AlgorithmParameterSpec classes

Model BouncyCastle (as JCA Provider)

Create rule to detect hard coded password for PasswordAuthentication

The class java.net.PasswordAuthentication takes as second parameter a password as input. It shoud never be hard-coded within the source code.

See
PasswordAuthentication

Rule for String is unclear

What exactly do we use the rule for String for? Is it necessary?

Deprecated support for TLSv1.1 protocol

As of now, we are still using the TLS version 1.1 protocol in our SSLContect rule here.

According to many articles from Bitbucket, Atlassian and many more, usage of this protocol should be deprecated. The only protocol that should be used is TLSv1.2 and its upcoming successor v1.3.

Possible typo in SecretKeyFactory default rule

The SecretKeyFactory rule has an instance keylength of type INTEGER in the OBJECTS section as shown in this link.

In the official Oracle documentation of this rule here, the keylength variable is inexistent. Possibly this is a typo and the keylength should be removed from the rule.

Wrong argument to doFinal call

In this line it should be ciphertext_off instead of plain_off. (JCA Doc)

Write CrySL rules for DigestInputStream, DigestOutputStream, CipherInputStream, CipherOutputStream

Specification of SecretKeyFactory uses wrong type

The argument to generateSecret(KeySpec keySpec) has type java.security.spec.KeySpec. In the rule it is specified to be of type javax.crypto.spec.SecretKeySpec.

NoPadding must be used for block modes GCM, CTR, CFB, OFB

These block modes are used in stream ciphers, and only NoPadding makes sense.

Github Service implementing a regulator pattern to update rules

The authors of this paper argue for the use of a Regulator pattern for cryptographic libraries. With a regular pattern, such a library could automatically update the algorithms it uses to avoid using deprecated ones.
Now, CrySL is not a library, but as algorithms become deprecated, CrySL rules become outdated. A web or github service regularly checking for the latest recommendations by authorities like NIST or the BSI might make maintaining rules easier.
Implement such a service that collects recommendations from multiple sources. Equipped with those recommendations, the service should then compare which algorithms CrySL rules currently allow and open a PR on this repository with appropriate modifications.

Bouncy Castle AES specifications are incomplete

Currently there is only CrySL specification for AESEngine, which supports block size of 128 bits. But the BSI documentation also recommends AES-192, AES-256 as secure block ciphers in Section 2.1. After some research I found that the Rijndael is the former name of AES and Bouncy Castle has an associated class with that name. One of the constructors in RijndaelEngine class takes block sizes of 128, 192 & 256 as its argument. Furthermore, there are two optimized variants of AES provided by Bouncy Castle, namely AESFastEngine and AESLightEngine. These are aren't specified yet.

Check BigInteger arguments to constructor of DSAParameterSpec, DHParameterSpec

DHParameterSpec and DSAParameterSpec takes BigInteger as arguments. The rules can additionally check on the size of these integers.

Possible CrySL rules to implement from CryptoGuard Crypto-API Benchmark

CryptoGuard Crypto-API Benchmark reports misuses that are not found from static analysis tools such as CogniCrypt. In their list of tests, the following CrySL rules are not implemented and therefore headless tests could not be done.

Reference of the issue is in this link

Java classes that are used in the CryptoGuard tests and are not yet implemented as CrySL rules can be found below.

Checklist:

Model correct PBEKeySpec usage

final byte[] salt = new byte[32];
SecureRandom.getInstanceStrong().nextBytes(salt);
final PBEKeySpec pbekeyspec = new PBEKeySpec(new char[] {'p','a','s','s','w','o','r','d'}, salt, 65000, 128);
final SecretKeyFactory secFac = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
final Cipher c = Cipher.getInstance("AES/GCM/PKCS5Padding");
SecretKey tmpKey = secFac.generateSecret(pbekeyspec);
byte[] keyMaterial = tmpKey.getEncoded();
final SecretKeySpec actKey = new SecretKeySpec(keyMaterial, "AES");
c.init(Cipher.ENCRYPT_MODE, actKey);
c.doFinal("TESTPLAIN".getBytes("UTF-8"));
c.getIV();

Bouncy Castle EC specifications are incomplete

The Section 3.4 of the BSI document states that ECIES (Elliptic Curve Integrated Encryption Scheme) is supported. There is also an associated class for it in the bouncy castle documentation IESEngine. One can find examples of its usage in the following test cases here

Model JCA providers individually

As of now, there are rules for the JCA. However, "the JCA" in its default configuration actually comes as a set of individual providers.
Create one project for each of the providers and model their usages. Once that is done, remove the JavaCryptographicArchitecture project from the repo.

AlgorithmParameterSpec, KeySpec predicate refinement

Currently, CrySL provides different predicates for AlgorithmParamterSpec interface implementers. We should have the same for KeySpec implementers. In the following example, we get an InvalidKeySpecException, due to the wrong KeySpec object.

public static PrivateKey createPrivateKey(){
 KeyFactory keyFactory = KeyFactory.getInstance("DSA");
 PBEKeySpec spec = new PBEKeySpec(new char[16]);
 PrivateKey privateKey = keyFactory.generatePrivate(spec);
 return privateKey;
}

java.security.spec.AlgorithmParameterSpec

java.security.spec.KeySpec

Sources:
https://docs.oracle.com/javase/8/docs/api/java/security/spec/package-tree.html
https://docs.oracle.com/javase/8/docs/api/javax/crypto/spec/package-tree.html

Signature should have additional correct algortihms.

Our analysis reports 21 internal constraints violation for Signature.

The selected algorithm is not specified. I guess, these should be ok to rely on?

SHA256withRSA
SHA256withECDSA

No constraints on ECGenParameterSpec

There are no constraints for ECGenParameterSpec which curve to use.

BSI recommends in different documents ([1], [2], [3]) following curves: BrainpoolP224r (til 2022), BrainpoolP256r1, BrainpoolP384r1, BrainpoolP320r1, BrainpoolP512r1,NIST Curve P-224 (til 2022), NIST Curve P-256, NIST Curve P-384, NIST Curve P-521

Rule for cipher should allow multiple `init` calls

I saw an error that cipher expected an update/doFinal call where I was calling init (AGAIN!).

Is it not correct to reuse a created instance of a cipher object?

The JDK states

Note that when a Cipher object is initialized, it loses all previously-acquired state. In other words, initializing a Cipher is equivalent to creating a new instance of that Cipher and initializing it.

So this should allow me to call init again. If that is true, the rule should be updated.

CryptoGuard projects and their respective cases

Below are cases that were headless tested. They are grouped according to their project name in CryptoGuard and the JCA object involved.

predictablekeystorepassword (KeyStore)
The project below is PredictableKeyStorePasswordBBCase1

package example.predictablekeystorepassword;

import java.io.IOException;
import java.net.URL;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;

public class PredictableKeyStorePasswordBBCase1 {

    URL cacerts;
    public static void main(String args[]) throws KeyStoreException, IOException, CertificateException, NoSuchAlgorithmException {
        PredictableKeyStorePasswordBBCase1 pksp = new PredictableKeyStorePasswordBBCase1();
        pksp.go();
    }

    public void go() throws KeyStoreException, IOException, CertificateException, NoSuchAlgorithmException {
        String type = "JKS";
        KeyStore ks = KeyStore.getInstance(type);
        cacerts = new URL("https://www.google.com");
        String defaultKey = "changeit";
        ks.load(cacerts.openStream(), defaultKey.toCharArray());
    }
}

predictablepbepassword (PBEKeySpec)
The project below is PredictablePBEPasswordBBCase1

package example.predictablepbepassword;

import javax.crypto.spec.PBEKeySpec;
import javax.crypto.spec.PBEParameterSpec;
import java.security.SecureRandom;

public class PredictablePBEPasswordBBCase1 {
    private PBEKeySpec pbeKeySpec = null;
    private PBEParameterSpec pbeParamSpec = null;

    public static void main(String [] args){
        PredictablePBEPasswordBBCase1 ckp = new PredictablePBEPasswordBBCase1();
        ckp.key();
    }
    public void key() {
        String defaultKey = "saagar";
        byte [] salt = new byte[16];
        SecureRandom sr = new SecureRandom();
        sr.nextBytes(salt);
        int iterationCount = 11010;
        int keyLength = 16;
        pbeKeySpec = new PBEKeySpec(defaultKey.toCharArray(),salt,iterationCount,keyLength);
    }

}

crossingtud / crypto-api-rules Goto Github PK

crypto-api-rules's People

Contributors

Stargazers

Watchers

Forkers

crypto-api-rules's Issues

Checklist:

CryptoGuard projects and their respective cases

Recommend Projects

Recommend Topics

Recommend Org