Skip to content

Commit

Permalink
Fix indeterminism and issues introduced by #250 (#295)
Browse files Browse the repository at this point in the history
* chore: Clean up test output print and clean up the whitespace in some of the test files.

commit-id:1ca6851c

* feature: Introduce the LarkyDigitalSignatureAlgorithm (DSA) implementation.

commit-id:d5450258

* feature: Implement coerceToInt as per python's `__index__` and `__int__` (deprecated since 3.8) protocol

commit-id:9a0135bc

* feature: Respect `__getattr__` when looking up a field on `SimpleStruct` and implement the `__bool__()` protocol for truth.

This is two features in one.

1. Enhance field lookup to respect `__getattr__` if it exists (which only was respected if you used `getattr()`, not the `.`. When getting a field, check to see if the class implements `__getattr__` and if the field is not found, then invoke the function.

2. `truth()` should implement the same semantics as python's `__bool__()`. `__bool__()` is used to implement truth value testing and the built-in operation `bool()` and it should return `False` or `True`. If the class does not define `__bool__()`, then check to see if the class defines `__len__()` and it returns non-zero integer (or boolean). If a class defines neither `__len__()` nor `__bool__()`, all its instances are considered true.

An unrelated bug fix: the reflective property of `__eq__` is `__eq__`, not `__ne__` (this diff is included for ease of merging).

commit-id:f5080216

* chore: With the previous diff, we explicitly check for None-ness to respect the `__bool__()` implementation (for truthy-ness).

commit-id:6cdc9e01

* Following on the previous diffs, we use the newly introduced `coerceToInt` for the built-in function `int()` as well as introduce `AttributeError`.

This allows us to use `hasattr()` on a "class" that implements `__getattr__`, but throws an exception to signal that the attribute does not exist. We follow it up with an implementation of `hasattr()` that DOES respect `AttributeError`.

commit-id:1eb18d65

* chore: dutifully replicate expand_subject_public_key_info from `Crypto/PublicKey/__init__.py`

We've made some big strides in Larky and now we can use `operator` to mimic the upstream code

commit-id:96317127

* chore: Expose the `jacobi_symbol` as a "classmethod" on the `Integer` "class"

FWIW, this `Crypto.Numbers.Integer` class will need to be re-written at some point.

commit-id:2dc6794d

* chore: Fix the prime searching code but use the Java `miller_rabin_test` implementation *instead*.

We probably should also implement the `Lucas Lehmer` primality test at some point in Java, but that's not needed as of now.

commit-id:6197a548

* chore: Remove comments

commit-id:f01dee2e

* chore: Uncomment failing test

commit-id:b49393e7

* chore: Comment on a particular test that fails 2 times in 100 runs. This in-determinism is due to `Random.new().read...` and can be fixed by going through a loop

commit-id:ba145bb6

* fix: Fix `toStarlark()` to always return an *encoded* version instead of `toString()`

This was the reason that Josh needed to do all of these string hacks and repacking of OIDs, etc.

commit-id:1f830864

* chore: Revert the commits and changes that were introduced due to the ASN1 encoding issues solved in the previous commit.

commit-id:48ed7020

* chore: Add failing tests that were ported from pycryptodome regarding implementation of the DSA algorithm

commit-id:8fe69ba4

* fix: Make all the tests pass

The key issue was just sloppy porting to Larky. Comparisons can be done via the `operator` module.

In this commit, is a fix for a *HUGE* issue in `LarkyDigest` that was checking to see if `LarkyDigest` was of instance `Memoable` *INSTEAD* of the underlying `ExtendedDigest` from bouncycastle. This caused a failure where repeated invocations of `hexdigest()` or `digest()` would produce the wrong digest value. The first attempt to fix this was in 020fde2 for reported issue #179.

commit-id:8858cdb3

* fix: Do not copy cryptography but use a similar implementation of the ecdsa one instead.

commit-id:b89034bc
  • Loading branch information
mahmoudimus authored May 27, 2022
1 parent 672e305 commit ada686b
Show file tree
Hide file tree
Showing 33 changed files with 1,211 additions and 854 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import net.starlark.java.ext.ByteList;

import org.bouncycastle.crypto.ExtendedDigest;
import org.bouncycastle.crypto.digests.GeneralDigest;
import org.bouncycastle.util.Memoable;

public abstract class LarkyDigest implements StarlarkValue {
Expand Down Expand Up @@ -44,8 +43,8 @@ public StarlarkBytes digest(StarlarkThread thread) throws EvalException {
protected byte[] reuseDigestIfPossible() {
byte[] resBuf = new byte[this.getDigest().getDigestSize()];
final ExtendedDigest copy;
if(this instanceof Memoable) {
copy = (ExtendedDigest) ((GeneralDigest) this.getDigest()).copy();
if(this.getDigest() instanceof Memoable) {
copy = (ExtendedDigest) ((Memoable)this.getDigest()).copy();
} else {
copy = this.getDigest();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package com.verygood.security.larky.modules.crypto.PublicKey;

import java.security.SecureRandom;

import net.starlark.java.annot.Param;
import net.starlark.java.annot.ParamType;
import net.starlark.java.annot.StarlarkMethod;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.NoneType;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkInt;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkValue;
import net.starlark.java.eval.Tuple;

import org.bouncycastle.crypto.AsymmetricCipherKeyPair;
import org.bouncycastle.crypto.CryptoServicesRegistrar;
import org.bouncycastle.crypto.digests.SHA256Digest;
import org.bouncycastle.crypto.generators.DSAKeyPairGenerator;
import org.bouncycastle.crypto.generators.DSAParametersGenerator;
import org.bouncycastle.crypto.params.DSAKeyGenerationParameters;
import org.bouncycastle.crypto.params.DSAParameterGenerationParameters;
import org.bouncycastle.crypto.params.DSAParameters;
import org.bouncycastle.crypto.params.DSAPrivateKeyParameters;
import org.bouncycastle.crypto.params.DSAPublicKeyParameters;
import org.bouncycastle.jcajce.provider.asymmetric.util.PrimeCertaintyCalculator;

public class LarkyDigitalSignatureAlgorithm implements StarlarkValue {
public static LarkyDigitalSignatureAlgorithm INSTANCE = new LarkyDigitalSignatureAlgorithm();

@StarlarkMethod(
name = "generate",
parameters = {
@Param(name = "bits"),
@Param(name = "randfunc", defaultValue = "None"),
@Param(name = "domain", allowedTypes = {
@ParamType(type= Tuple.class),
@ParamType(type= NoneType.class)
}, defaultValue = "None"),
}, useStarlarkThread = true)
public Dict<String, StarlarkInt> DSA_generate(StarlarkInt bits_, Object randomFuncO, Object domainO, StarlarkThread thread) throws EvalException {
int bits = bits_.toIntUnchecked();
if (bits != 1024 && bits != 2048 && bits != 3072) {
throw Starlark.errorf("ValueError: Invalid modulus length (%d). Must be one of: 1024, 2048, or 3072.", bits);
}
SecureRandom secureRandom = CryptoServicesRegistrar.getSecureRandom();
DSAKeyPairGenerator dsaKeyPairGenerator = new DSAKeyPairGenerator();
DSAKeyGenerationParameters dsaKeyGenerationParameters;
if(!Starlark.isNullOrNone(domainO)) {
Tuple domain = (Tuple) domainO;
dsaKeyGenerationParameters = new DSAKeyGenerationParameters(
secureRandom,
new DSAParameters(
((StarlarkInt) domain.get(0)).toBigInteger(),
((StarlarkInt) domain.get(1)).toBigInteger(),
((StarlarkInt) domain.get(2)).toBigInteger()
)
);
} else {
final DSAParametersGenerator dsaParametersGenerator = new DSAParametersGenerator(new SHA256Digest());
final int N;
switch(bits) {
case 1024:
N = 160;
break;
case 2048:
N = 224;
break;
case 3072:
N = 256;
break;
default:
throw Starlark.errorf("Should absolutely never get here!");
}
DSAParameterGenerationParameters params = new DSAParameterGenerationParameters(bits, N, PrimeCertaintyCalculator.getDefaultCertainty(bits), secureRandom);
dsaParametersGenerator.init(params);
dsaKeyGenerationParameters = new DSAKeyGenerationParameters(
secureRandom,
dsaParametersGenerator.generateParameters()
);
}
dsaKeyPairGenerator.init(dsaKeyGenerationParameters);
AsymmetricCipherKeyPair asymKeyPair;
try {
asymKeyPair = dsaKeyPairGenerator.generateKeyPair();
}catch(IllegalArgumentException ex) {
throw Starlark.errorf("ValueError: Invalid DSA domain parameters (%s)", ex.getMessage());
}
/*
y : integer
Public key.
g : integer
Generator
p : integer
DSA modulus
q : integer
Order of the subgroup
x : integer
Private key.
*/
DSAPublicKeyParameters pubKey = ((DSAPublicKeyParameters) asymKeyPair.getPublic());
DSAPrivateKeyParameters privateKey = ((DSAPrivateKeyParameters) asymKeyPair.getPrivate());
return Dict.<String, StarlarkInt>builder()
.put("y", StarlarkInt.of(pubKey.getY()))
.put("g", StarlarkInt.of(pubKey.getParameters().getG()))
.put("p", StarlarkInt.of(pubKey.getParameters().getP()))
.put("q", StarlarkInt.of(pubKey.getParameters().getQ()))
.put("x", StarlarkInt.of(privateKey.getX()))
.build(thread.mutability());
}
}
Loading

0 comments on commit ada686b

Please sign in to comment.