Sitelet https://github.com/dnsjava/dnsjava/commit/bb5992421f2aed35396ffb7a5b8a262bb1a5628f
Skip to content

Commit bb59924

Browse files
Mahmoodifaribauersachs
Mahmoodifar
and
ibauersachs
authored
Validate SSHFP fingerprint lengths (#415)
* fix: validate SSHFP fingerprint lengths Fixes #414 * Validate length in rrFromWire too * Use shared consts for digest lengths --------- Co-authored-by: Ingo Bauersachs <ingo@jitsi.org>
1 parent be8d447 commit bb59924

7 files changed

Lines changed: 176 additions & 25 deletions

File tree

‎src/main/java/org/xbill/DNS/DNSSEC.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -237,17 +237,17 @@ private Digest() {}
237237
algs.setNumericAllowed(true);
238238

239239
algs.add(SHA1, "SHA-1");
240-
algLengths.put(SHA1, 20);
240+
algLengths.put(SHA1, DigestLengths.SHA1);
241241
algs.add(SHA256, "SHA-256");
242-
algLengths.put(SHA256, 32);
242+
algLengths.put(SHA256, DigestLengths.SHA256);
243243
algs.add(GOST3411, "GOST R 34.11-94");
244-
algLengths.put(GOST3411, 32);
244+
algLengths.put(GOST3411, DigestLengths.GOST3411);
245245
algs.add(SHA384, "SHA-384");
246-
algLengths.put(SHA384, 48);
246+
algLengths.put(SHA384, DigestLengths.SHA384);
247247
algs.add(GOST3411_12, "GOST12");
248-
algLengths.put(GOST3411_12, 64);
248+
algLengths.put(GOST3411_12, DigestLengths.GOST3411_12);
249249
algs.add(SM3, "SM3");
250-
algLengths.put(SM3, 32);
250+
algLengths.put(SM3, DigestLengths.SM3);
251251
}
252252

253253
/** Converts an algorithm into its textual representation */

‎src/main/java/org/xbill/DNS/DigestLengths.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
package org.xbill.DNS;
3+
4+
import lombok.experimental.UtilityClass;
5+
6+
/** Constants for common Hash/Digest lengths. */
7+
@UtilityClass
8+
class DigestLengths {
9+
static final int MD5 = 16;
10+
static final int SHA1 = 20;
11+
static final int SHA224 = 28;
12+
static final int SHA256 = 32;
13+
static final int SHA384 = 48;
14+
static final int SHA512 = 64;
15+
static final int GOST3411 = 32;
16+
static final int GOST3411_12 = 64;
17+
static final int SM3 = 32;
18+
}

‎src/main/java/org/xbill/DNS/SSHFPRecord.java‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package org.xbill.DNS;
55

66
import java.io.IOException;
7+
import java.util.function.Function;
78
import org.xbill.DNS.utils.base16;
89

910
/**
@@ -18,13 +19,25 @@ public static class Algorithm {
1819
private Algorithm() {}
1920

2021
public static final int RSA = 1;
21-
public static final int DSS = 2;
22+
23+
/**
24+
* Use {@link #DSA}; DSS was a typo in RFC 4255.
25+
*
26+
* @see <a href="https://errata.rfc-editor.org/eid6266/">Errata-ID: 6266</a>
27+
*/
28+
@Deprecated public static final int DSS = 2;
29+
30+
public static final int DSA = 2;
31+
public static final int ECDSA = 3;
32+
public static final int ED25519 = 4;
33+
public static final int ED448 = 6;
2234
}
2335

2436
public static class Digest {
2537
private Digest() {}
2638

2739
public static final int SHA1 = 1;
40+
public static final int SHA256 = 2;
2841
}
2942

3043
private int alg;
@@ -45,20 +58,43 @@ public SSHFPRecord(Name name, int dclass, long ttl, int alg, int digestType, byt
4558
this.alg = checkU8("alg", alg);
4659
this.digestType = checkU8("digestType", digestType);
4760
this.fingerprint = fingerprint;
61+
validateFingerprintLength(IllegalArgumentException::new);
4862
}
4963

5064
@Override
5165
protected void rrFromWire(DNSInput in) throws IOException {
5266
alg = in.readU8();
5367
digestType = in.readU8();
5468
fingerprint = in.readByteArray();
69+
validateFingerprintLength(WireParseException::new);
5570
}
5671

5772
@Override
5873
protected void rdataFromString(Tokenizer st, Name origin) throws IOException {
5974
alg = st.getUInt8();
6075
digestType = st.getUInt8();
6176
fingerprint = st.getHex(true);
77+
validateFingerprintLength(st::exception);
78+
}
79+
80+
private <T extends Throwable> void validateFingerprintLength(Function<String, T> exceptionCreator)
81+
throws T {
82+
int expectedLength = fingerprintLength(digestType);
83+
if (expectedLength >= 0 && fingerprint.length != expectedLength) {
84+
throw exceptionCreator.apply(
85+
"Expected " + expectedLength + " fingerprint bytes, got " + fingerprint.length);
86+
}
87+
}
88+
89+
private static int fingerprintLength(int digestType) {
90+
switch (digestType) {
91+
case Digest.SHA1:
92+
return DigestLengths.SHA1;
93+
case Digest.SHA256:
94+
return DigestLengths.SHA256;
95+
default:
96+
return -1;
97+
}
6298
}
6399

64100
@Override

‎src/main/java/org/xbill/DNS/TSIG.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,12 @@ public class TSIG {
145145
algMap = Collections.unmodifiableMap(names);
146146

147147
Map<Name, Integer> lengths = new HashMap<>();
148-
lengths.put(HMAC_MD5, 16);
149-
lengths.put(HMAC_SHA1, 20);
150-
lengths.put(HMAC_SHA224, 28);
151-
lengths.put(HMAC_SHA256, 32);
152-
lengths.put(HMAC_SHA384, 48);
153-
lengths.put(HMAC_SHA512, 64);
148+
lengths.put(HMAC_MD5, DigestLengths.MD5);
149+
lengths.put(HMAC_SHA1, DigestLengths.SHA1);
150+
lengths.put(HMAC_SHA224, DigestLengths.SHA224);
151+
lengths.put(HMAC_SHA256, DigestLengths.SHA256);
152+
lengths.put(HMAC_SHA384, DigestLengths.SHA384);
153+
lengths.put(HMAC_SHA512, DigestLengths.SHA512);
154154
lengths.put(HMAC_SHA256_128, 16);
155155
lengths.put(HMAC_SHA384_192, 24);
156156
lengths.put(HMAC_SHA512_256, 32);

‎src/main/java/org/xbill/DNS/ZoneMDRecord.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ public static class Hash {
8383
schemes.setNumericAllowed(true);
8484
schemes.add(RESERVED, "RESERVED");
8585
schemes.add(SHA384, "SHA384");
86-
hashLengths.put(SHA384, 48);
86+
hashLengths.put(SHA384, DigestLengths.SHA384);
8787
schemes.add(SHA512, "SHA512");
88-
hashLengths.put(SHA512, 64);
88+
hashLengths.put(SHA512, DigestLengths.SHA512);
8989
}
9090

9191
/** Converts an algorithm into its textual representation */

‎src/test/java/org/xbill/DNS/SSHFPRecordTest.java‎

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,116 @@
11
// SPDX-License-Identifier: BSD-3-Clause
22
package org.xbill.DNS;
33

4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
45
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
56
import static org.junit.jupiter.api.Assertions.assertEquals;
67

78
import java.io.IOException;
89
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.params.ParameterizedTest;
11+
import org.junit.jupiter.params.provider.CsvSource;
912
import org.xbill.DNS.utils.base16;
1013

1114
class SSHFPRecordTest {
12-
1315
@Test
1416
void rdataFromString() throws IOException {
15-
Tokenizer t = new Tokenizer("2 1 CAFEBABE");
17+
String fingerprint = "CAFEBABECAFEBABECAFEBABECAFEBABECAFEBABE";
18+
Tokenizer t = new Tokenizer("2 1 " + fingerprint);
1619
SSHFPRecord sshfpRecord = new SSHFPRecord();
1720
sshfpRecord.rdataFromString(t, null);
18-
assertEquals(SSHFPRecord.Algorithm.DSS, sshfpRecord.getAlgorithm());
21+
assertEquals(SSHFPRecord.Algorithm.DSA, sshfpRecord.getAlgorithm());
1922
assertEquals(SSHFPRecord.Digest.SHA1, sshfpRecord.getDigestType());
20-
assertArrayEquals(base16.fromString("CAFEBABE"), sshfpRecord.getFingerPrint());
23+
assertArrayEquals(base16.fromString(fingerprint), sshfpRecord.getFingerPrint());
24+
}
25+
26+
@ParameterizedTest
27+
@CsvSource({
28+
SSHFPRecord.Digest.SHA1 + "," + DigestLengths.SHA1,
29+
SSHFPRecord.Digest.SHA256 + "," + DigestLengths.SHA256,
30+
"3,4",
31+
"4,4",
32+
"255,4"
33+
})
34+
void validFingerprintLengths(int digestType, int length) throws IOException {
35+
byte[] fingerprint = new byte[length];
36+
SSHFPRecord sshfpRecord =
37+
(SSHFPRecord)
38+
Record.fromString(
39+
Name.root,
40+
Type.SSHFP,
41+
DClass.IN,
42+
3600,
43+
"1 " + digestType + " " + base16.toString(fingerprint),
44+
Name.root);
45+
assertEquals(digestType, sshfpRecord.getDigestType());
46+
assertArrayEquals(fingerprint, sshfpRecord.getFingerPrint());
47+
48+
SSHFPRecord constructed =
49+
new SSHFPRecord(Name.root, DClass.IN, 3600, 1, digestType, fingerprint);
50+
assertArrayEquals(fingerprint, constructed.getFingerPrint());
51+
}
52+
53+
@ParameterizedTest
54+
@CsvSource({
55+
SSHFPRecord.Digest.SHA1 + ",4",
56+
SSHFPRecord.Digest.SHA1 + ",19",
57+
SSHFPRecord.Digest.SHA1 + ",21",
58+
SSHFPRecord.Digest.SHA1 + ",32",
59+
SSHFPRecord.Digest.SHA256 + ",4",
60+
SSHFPRecord.Digest.SHA256 + ",20",
61+
SSHFPRecord.Digest.SHA256 + ",31",
62+
SSHFPRecord.Digest.SHA256 + ",33"
63+
})
64+
void rdataFromStringInvalidFingerprintLengths(int digestType, int length) {
65+
String rdata = "1 " + digestType + " " + base16.toString(new byte[length]);
66+
assertThatThrownBy(
67+
() -> Record.fromString(Name.root, Type.SSHFP, DClass.IN, 3600, rdata, Name.root))
68+
.isInstanceOf(TextParseException.class)
69+
.hasMessageMatching(".+Expected.+fingerprint bytes, got.+");
70+
}
71+
72+
@ParameterizedTest
73+
@CsvSource({
74+
SSHFPRecord.Digest.SHA1 + ",4",
75+
SSHFPRecord.Digest.SHA1 + ",19",
76+
SSHFPRecord.Digest.SHA1 + ",21",
77+
SSHFPRecord.Digest.SHA1 + ",32",
78+
SSHFPRecord.Digest.SHA256 + ",4",
79+
SSHFPRecord.Digest.SHA256 + ",20",
80+
SSHFPRecord.Digest.SHA256 + ",31",
81+
SSHFPRecord.Digest.SHA256 + ",33"
82+
})
83+
void rrFromWireInvalidFingerprintLengths(int digestType, int length) {
84+
DNSOutput rr = new DNSOutput();
85+
rr.writeByteArray(
86+
Record.newRecord(Name.root, Type.SSHFP, DClass.IN, 3600).toWire(Section.ANSWER));
87+
int lengthPos = rr.current() - 2;
88+
rr.writeU8(1);
89+
rr.writeU8(digestType);
90+
rr.writeByteArray(new byte[length]);
91+
rr.writeU16At(2 + length, lengthPos);
92+
assertThatThrownBy(() -> Record.fromWire(rr.toByteArray(), Section.ANSWER))
93+
.isInstanceOf(WireParseException.class)
94+
.hasMessageMatching("Expected.+fingerprint bytes, got.+");
95+
}
96+
97+
@ParameterizedTest
98+
@CsvSource({
99+
SSHFPRecord.Digest.SHA1 + ",0",
100+
SSHFPRecord.Digest.SHA1 + ",4",
101+
SSHFPRecord.Digest.SHA1 + ",19",
102+
SSHFPRecord.Digest.SHA1 + ",21",
103+
SSHFPRecord.Digest.SHA1 + ",32",
104+
SSHFPRecord.Digest.SHA256 + ",0",
105+
SSHFPRecord.Digest.SHA256 + ",4",
106+
SSHFPRecord.Digest.SHA256 + ",20",
107+
SSHFPRecord.Digest.SHA256 + ",31",
108+
SSHFPRecord.Digest.SHA256 + ",33"
109+
})
110+
void constructorInvalidFingerprintLengths(int digestType, int length) {
111+
assertThatThrownBy(
112+
() -> new SSHFPRecord(Name.root, DClass.IN, 3600, 1, digestType, new byte[length]))
113+
.isInstanceOf(IllegalArgumentException.class)
114+
.hasMessageMatching("Expected.+fingerprint bytes, got.+");
21115
}
22116
}

‎src/test/java/org/xbill/DNS/ZoneMDRecordTest.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@
1818
public class ZoneMDRecordTest {
1919
@ParameterizedTest
2020
@CsvSource({
21-
"1,48", "2,64",
21+
"1," + DigestLengths.SHA384,
22+
"2," + DigestLengths.SHA512,
2223
})
2324
void testKnownHashLengths(int alg, int len) {
2425
assertEquals(len, Hash.hashLength(alg));
@@ -34,7 +35,10 @@ void testKnownHashNames(int alg, String suffix) {
3435

3536
@ParameterizedTest
3637
@CsvSource({
37-
"0,0,12", "1,0,12", "0,1,48", "0,2,64",
38+
"0,0,12",
39+
"1,0,12",
40+
"0,1," + DigestLengths.SHA384,
41+
"0,2," + DigestLengths.SHA512,
3842
})
3943
void testConstructorSuccess(int scheme, int hash, int digestSize) {
4044
ZoneMDRecord md =
@@ -102,11 +106,10 @@ void testFromWireSuccess(long serial, int scheme, int hash, String digest) {
102106
"2147483648,0,257,FEBE3D4CFEBEFEBE3D4CFEBE",
103107
})
104108
void testFromWireFails(long serial, int scheme, int hash, String digest) {
109+
byte[] base16digest = base16.fromString(digest);
105110
assertThrows(
106111
IllegalArgumentException.class,
107-
() ->
108-
new ZoneMDRecord(
109-
Name.root, DClass.IN, 3600, serial, scheme, hash, base16.fromString(digest)));
112+
() -> new ZoneMDRecord(Name.root, DClass.IN, 3600, serial, scheme, hash, base16digest));
110113
}
111114

112115
@ParameterizedTest
@@ -145,7 +148,7 @@ void testToAndFromWire(long serial, int scheme, int hash, String digestHex) thro
145148
"00_003F_0001_00000E10_0047_80000000_00_02_FEBE3D4CE2EC2FFA4BA99D46CD69D6D29711E55217057BEEFEBE3D4CFEBE3D4CFEBE3D4CE2EC2FFA4BA99D46CD69D6D29711E55217057BEEFEBE3D4CFEBE3D4C5D",
146149
})
147150
void testFromWireFails(String hex) {
148-
byte[] data = base16.fromString(hex.replaceAll("_", ""));
151+
byte[] data = base16.fromString(hex.replace("_", ""));
149152
assertThrows(WireParseException.class, () -> Record.fromWire(data, Section.ANSWER));
150153
}
151154

0 commit comments

Comments
 (0)