WIP Adding necessary crysl rules to support ECIES

[svenfeld]

This pull request is not getting merged at the moment due to rules causing false positives. The false positives are caused as third party API data flows are not getting analysed. See the last comment for more information.

This pull request adds crysl rules for classes that are necessary for the ECIES encryption scheme #83.

BSI states in section 3.4 that there are 3 main components to ECIES:

Symmetric encryption: already supported by the current bouncycastle ruleset.

MAC: HMAC, GMAC, CMAC are recommended. HMAC is supported by the current bouncycastle ruleset.

Key derivation functions: Not supported by the current bouncycastle ruleset. BSI recommends key derivation through Extraction-then-Expansion. The bouncycastle class KDF2BytesGenerator implements that scheme. MGF1BytesGenerator would also be a candidate but the current IESCipher class doesn't support that class. KDF2BytesGenerator.crysl is added with the corresponding KDFParameters.crysl

Besides symmetric encryption, MAC, and key derivation. A key agreement is part of ECIES. Crysl rule for ECDHBasicAgreement.crysl is therefore added.

The class that runs the ECIES encryption scheme in the bouncycastle is the IESCipher and the underlying IESEngine.

Rules for both classes are added. The constraints for both classes are mostly realized through the REQUIRES section. Concerning is https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-1000345

Notable changes to AsymmetricCipherKeyPair. Added missing method calls getPrivate and getPublic. As AsymmetricCipherKeyPair can be used for RSAKeys or ECKeys the crysl rule now ENSURES a lot of predicates. Furthermore to allow the pair to only be of EC or RSA the CONSTRAINTS section is added.

[svenfeld]

The IESEngine rule is causing false positives errors. Precisely ORDER errors are getting generated for all objects used in the constructor of the IESEngine. Due to this, the pull request is not getting merged at the moment.

[schlichtig]

@svenfeld can you check, whether recent changes solved the issue with false positives?

[svenfeld]

@smeyer198 wrote this regarding third party libraries. Therefore, I think this does not work except if you could expand the analysis. Do you have any insights whether that would be possible/feasible @smeyer198 ?

[smeyer198]

@svenfeld What exactly do the changes do? Do they just add additional rules for the BouncyCastle API? Then, there should be no problem. Note that the DataFlowScope just determines which methods are analyzed. For example, if we have a method

public void example() {
     BCObject obj = new BCObject();     // defined in BouncyCastle
     obj.callToSomeMethod();         // defined in BouncyCastle
     furtherMethod();                // defined in the application
}

that is part of the application. Then, CryptoAnalysis (or more precisely SPDS) currently skips the internal logic of the constructor BCObject and the method callToSomeMethod because they are defined in BouncyCastle, i.e. in a third party library. This way, we avoid analyzing third party libraries because SPDS does not construct an IDE graph for those methods and searches for seeds within them. However, it still sees those methods, i.e. it generates a seed for obj, collects the calls and compares the calls to the rules if there are any. In contrast, if we consider furtherMethod that is defined in the application, it creates a corresponding IDE graph and includes corresponding dataflows in the analysis.

TL;DR The DataFlowScope should not impact new rules. Only dataflows inside third party libraries are ignored