Fix odd-length key tag calculation

computeKeytag() previously failed if data.length was odd.
ensdomains · Jul 18, 2019 · 44333d2 · 44333d2
1 parent a85463e
commit 44333d2
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 17 deletions.
diff --git a/contracts/DNSSECImpl.sol b/contracts/DNSSECImpl.sol
@@ -565,11 +565,10 @@ contract DNSSECImpl is DNSSEC, Owned {
      * @return The computed key tag.
      */
     function computeKeytag(bytes memory data) internal pure returns (uint16) {
-        uint ac;
-        for (uint i = 0; i < data.length; i += 2) {
-            ac += data.readUint16(i);
+        uint32 ac;
+        for (uint i = 0; i < data.length; i++) {
+            ac += i & 1 == 0 ? uint16(data.readUint8(i)) << 8 : data.readUint8(i);
         }
-        ac += (ac >> 16) & 0xFFFF;
-        return uint16(ac & 0xFFFF);
+        return uint16(ac + (ac >> 16));
     }
 }
diff --git a/lib/anchors.js b/lib/anchors.js
@@ -39,7 +39,7 @@ exports.dummyEntry = {
   class: 'IN',
   ttl: 3600,
   data: {
-    keyTag: 5647, // Empty body, flags == 0x0101, algorithm = 253, body = 0x1111
+    keyTag: 1278, // Empty body, flags == 0x0101, algorithm = 253, body = 0x0000
     algorithm: 253,
     digestType: 253,
     digest: new Buffer('', 'hex')

diff --git a/test/TestDNSSEC.js b/test/TestDNSSEC.js
@@ -145,7 +145,7 @@ contract('DNSSEC', function(accounts) {
         originalTTL: 3600,
         expiration: Date.now() / 1000 + 2419200,
         inception: Date.now() / 1000,
-        keyTag: 5647,
+        keyTag: 1278,
         signersName: '.',
         signature: new Buffer([])
       }
@@ -157,14 +157,14 @@ contract('DNSSEC', function(accounts) {
         type: 'DNSKEY',
         class: 'IN',
         ttl: 3600,
-        data: { flags: 0x0101, algorithm: 253, key: Buffer.from('1111', 'HEX') }
+        data: { flags: 0x0101, algorithm: 253, key: Buffer.from('0000', 'HEX') }
       },
       {
         name: '.',
         type: 'DNSKEY',
         class: 'IN',
         ttl: 3600,
-        data: { flags: 0, algorithm: 253, key: Buffer.from('1111', 'HEX') }
+        data: { flags: 0, algorithm: 253, key: Buffer.from('0000', 'HEX') }
       },
       {
         name: '.',
@@ -208,6 +208,24 @@ contract('DNSSEC', function(accounts) {
     await verifyFailedSubmission(instance, ...hexEncodeSignedSet(keys));
   });
 
+  it('should accept odd-length public keys', async () => {
+    const instance = await dnssec.deployed();
+    const keys = rootKeys();
+    keys.rrs = [
+      {
+        name: '.',
+        type: 'DNSKEY',
+        data: {
+          flags: 257,
+          algorithm: 253,
+          key: Buffer.from('00', 'hex')
+        }
+      }
+    ];
+    const [signedData] = hexEncodeSignedSet(keys);
+    await verifySubmission(instance, signedData, Buffer.alloc(0));
+  });
+
   it('should reject signatures by keys without the ZK bit set', async function() {
     var instance = await dnssec.deployed();
     var keys = rootKeys();
@@ -272,7 +290,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -311,7 +329,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -348,7 +366,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -385,7 +403,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -422,7 +440,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -459,7 +477,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: '.',
             signature: new Buffer([])
           }
@@ -494,7 +512,7 @@ contract('DNSSEC', function(accounts) {
             originalTTL: 3600,
             expiration: Date.now() / 1000 + 2419200,
             inception: Date.now() / 1000,
-            keyTag: 5647,
+            keyTag: 1278,
             signersName: 'com',
             signature: new Buffer([])
           }
@@ -578,7 +596,7 @@ contract('DNSSEC', function(accounts) {
         originalTTL: 3600,
         expiration: Date.now() / 1000 + 2419200,
         inception: Date.now() / 1000,
-        keyTag: 5647,
+        keyTag: 1278,
         signersName: '.',
         signature: new Buffer([])
       }