Skip to content

Commit b0c3ce9

Browse files
committed
added length check for sequence in DSA signatures
1 parent 0049491 commit b0c3ce9

File tree

2 files changed

+110
-3
lines changed
  • prov/src

2 files changed

+110
-3
lines changed

Diff for: prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dsa/DSASigner.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
import java.security.SecureRandom;
99
import java.security.SignatureException;
1010
import java.security.SignatureSpi;
11-
import java.security.interfaces.DSAKey;
12-
import java.security.interfaces.DSAPublicKey;
1311
import java.security.spec.AlgorithmParameterSpec;
1412

1513
import org.bouncycastle.asn1.ASN1Encoding;
@@ -18,7 +16,6 @@
1816
import org.bouncycastle.asn1.ASN1Sequence;
1917
import org.bouncycastle.asn1.DERSequence;
2018
import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers;
21-
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
2219
import org.bouncycastle.asn1.x509.X509ObjectIdentifiers;
2320
import org.bouncycastle.crypto.CipherParameters;
2421
import org.bouncycastle.crypto.DSA;
@@ -179,6 +176,11 @@ private BigInteger[] derDecode(
179176
throws IOException
180177
{
181178
ASN1Sequence s = (ASN1Sequence)ASN1Primitive.fromByteArray(encoding);
179+
if (s.size() != 2)
180+
{
181+
throw new IOException("malformed signature");
182+
}
183+
182184
return new BigInteger[]{
183185
((ASN1Integer)s.getObjectAt(0)).getValue(),
184186
((ASN1Integer)s.getObjectAt(1)).getValue()

Diff for: prov/src/test/java/org/bouncycastle/jce/provider/test/DSATest.java

+105
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,110 @@ public class DSATest
7070

7171
SecureRandom random = new FixedSecureRandom(new byte[][] { k1, k2 });
7272

73+
// DSA modified signatures, courtesy of the Google security team
74+
static final DSAPrivateKeySpec PRIVATE_KEY = new DSAPrivateKeySpec(
75+
// x
76+
new BigInteger(
77+
"15382583218386677486843706921635237927801862255437148328980464126979"),
78+
// p
79+
new BigInteger(
80+
"181118486631420055711787706248812146965913392568235070235446058914"
81+
+ "1170708161715231951918020125044061516370042605439640379530343556"
82+
+ "4101919053459832890139496933938670005799610981765220283775567361"
83+
+ "4836626483403394052203488713085936276470766894079318754834062443"
84+
+ "1033792580942743268186462355159813630244169054658542719322425431"
85+
+ "4088256212718983105131138772434658820375111735710449331518776858"
86+
+ "7867938758654181244292694091187568128410190746310049564097068770"
87+
+ "8161261634790060655580211122402292101772553741704724263582994973"
88+
+ "9109274666495826205002104010355456981211025738812433088757102520"
89+
+ "562459649777989718122219159982614304359"),
90+
// q
91+
new BigInteger(
92+
"19689526866605154788513693571065914024068069442724893395618704484701"),
93+
// g
94+
new BigInteger(
95+
"2859278237642201956931085611015389087970918161297522023542900348"
96+
+ "0877180630984239764282523693409675060100542360520959501692726128"
97+
+ "3149190229583566074777557293475747419473934711587072321756053067"
98+
+ "2532404847508798651915566434553729839971841903983916294692452760"
99+
+ "2490198571084091890169933809199002313226100830607842692992570749"
100+
+ "0504363602970812128803790973955960534785317485341020833424202774"
101+
+ "0275688698461842637641566056165699733710043802697192696426360843"
102+
+ "1736206792141319514001488556117408586108219135730880594044593648"
103+
+ "9237302749293603778933701187571075920849848690861126195402696457"
104+
+ "4111219599568903257472567764789616958430"));
105+
106+
static final DSAPublicKeySpec PUBLIC_KEY = new DSAPublicKeySpec(
107+
new BigInteger(
108+
"3846308446317351758462473207111709291533523711306097971550086650"
109+
+ "2577333637930103311673872185522385807498738696446063139653693222"
110+
+ "3528823234976869516765207838304932337200968476150071617737755913"
111+
+ "3181601169463467065599372409821150709457431511200322947508290005"
112+
+ "1780020974429072640276810306302799924668893998032630777409440831"
113+
+ "4314588994475223696460940116068336991199969153649625334724122468"
114+
+ "7497038281983541563359385775312520539189474547346202842754393945"
115+
+ "8755803223951078082197762886933401284142487322057236814878262166"
116+
+ "5072306622943221607031324846468109901964841479558565694763440972"
117+
+ "5447389416166053148132419345627682740529"),
118+
PRIVATE_KEY.getP(),
119+
PRIVATE_KEY.getQ(),
120+
PRIVATE_KEY.getG());
121+
122+
// The following test vectors check for signature malleability and bugs. That means the test
123+
// vectors are derived from a valid signature by modifying the ASN encoding. A correct
124+
// implementation of DSA should only accept correct DER encoding and properly handle the others.
125+
// Allowing alternative BER encodings is in many cases benign. An example where this kind of
126+
// signature malleability was a problem: https://en.bitcoin.it/wiki/Transaction_Malleability
127+
static final String[] MODIFIED_SIGNATURES = {
128+
"303e02811c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f9e"
129+
+ "f41dd424a4e1c8f16967cf3365813fe8786236",
130+
"303f0282001c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f"
131+
+ "9ef41dd424a4e1c8f16967cf3365813fe8786236",
132+
"303e021d001e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f9e"
133+
+ "f41dd424a4e1c8f16967cf3365813fe8786236",
134+
"303e021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd02811d00ade65988d237d30f9e"
135+
+ "f41dd424a4e1c8f16967cf3365813fe8786236",
136+
"303f021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd0282001d00ade65988d237d30f"
137+
+ "9ef41dd424a4e1c8f16967cf3365813fe8786236",
138+
"303e021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021e0000ade65988d237d30f9e"
139+
+ "f41dd424a4e1c8f16967cf3365813fe8786236",
140+
"30813d021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f9e"
141+
+ "f41dd424a4e1c8f16967cf3365813fe8786236",
142+
"3082003d021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f"
143+
+ "9ef41dd424a4e1c8f16967cf3365813fe8786236",
144+
"303d021c1e41b479ad576905b960fe14eadb91b0ccf34843dab916173bb8c9cd021d00ade65988d237d30f9ef4"
145+
+ "1dd424a4e1c8f16967cf3365813fe87862360000",
146+
"3040021c57b10411b54ab248af03d8f2456676ebc6d3db5f1081492ac87e9ca8021d00942b117051d7d9d107fc42cac9c5a36a1fd7f0f8916ccca86cec4ed3040100"
147+
};
148+
149+
private void testModified()
150+
throws Exception
151+
{
152+
KeyFactory kFact = KeyFactory.getInstance("DSA", "BC");
153+
PublicKey pubKey = kFact.generatePublic(PUBLIC_KEY);
154+
Signature sig = Signature.getInstance("DSA", "BC");
155+
156+
for (int i = 0; i != MODIFIED_SIGNATURES.length; i++)
157+
{
158+
sig.initVerify(pubKey);
159+
160+
sig.update(Strings.toByteArray("Hello"));
161+
162+
boolean failed;
163+
164+
try
165+
{
166+
failed = !sig.verify(Hex.decode(MODIFIED_SIGNATURES[i]));
167+
}
168+
catch (SignatureException e)
169+
{
170+
failed = true;
171+
}
172+
173+
isTrue("sig verified when shouldn't", failed);
174+
}
175+
}
176+
73177
private void testCompat()
74178
throws Exception
75179
{
@@ -1223,6 +1327,7 @@ public void performTest()
12231327
testDSA2Parameters();
12241328
testNullParameters();
12251329
testValidate();
1330+
testModified();
12261331
}
12271332

12281333
protected BigInteger[] derDecode(

0 commit comments

Comments
 (0)