From 44333d22dd3d3cb87a2e42713e36e9db3d5c88ae Mon Sep 17 00:00:00 2001
From: Jack Bates <jack@nottheoilrig.com>
Date: Tue, 16 Jul 2019 10:13:14 -0700
Subject: [PATCH] Fix odd-length key tag calculation

computeKeytag() previously failed if data.length was odd.
---
 contracts/DNSSECImpl.sol |  9 ++++-----
 lib/anchors.js           |  2 +-
 test/TestDNSSEC.js       | 40 +++++++++++++++++++++++++++++-----------
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/contracts/DNSSECImpl.sol b/contracts/DNSSECImpl.sol
index 6ec4d8c..5f1d72c 100644
--- 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
index 819fd54..3542f2e 100644
--- 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
index b5aef13..4ce1f5e 100644
--- 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([])
       }