Add optional tsig usage for new dns backends (#1032)

It is not always desirable to use a TSIG key for interacting with DNS backends.  This PR makes use of a TSIG key optional.  A new `tsig-usage` configuration parameter is added for DNS backends to allow one to determine how/if TSIG keys are applied.  Note: due to the nature of the configuration, the user must still specify SOME value for the key information; however, the new `tsig-usage` config parameter can choose when or if to apply it.  The values are:

- `always` - always use the 
- `never` - never use the tsig key for either update OR transfers
- `transfer` - use the tsig key for TRANSFER only, updates will not use any keys
- `update` - use the tsig key for UPDATES only, transfers will not use any keys

**Note: this does not yet apply to the UI or the API, changes there will be a future PR**

.github/workflows/ci.yml

@@ -27,7 +27,9 @@ jobs:
           fetch-depth: 0
 
       - name: Setup Java and Scala
-        uses: olafurpg/setup-scala@v5
+        uses: olafurpg/setup-scala@v10
+        env:
+          ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'
         with:
           java-version: ${{ matrix.java }}
 
@@ -93,7 +95,9 @@ jobs:
           fetch-depth: 0
 
       - name: Setup Java and Scala
-        uses: olafurpg/setup-scala@v5
+        uses: olafurpg/setup-scala@v10
+        env:
+          ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'
         with:
           java-version: ${{ matrix.java }}
 

.github/workflows/publish-site.yml

@@ -24,7 +24,9 @@ jobs:
           fetch-depth: 0
 
       - name: Setup Java and Scala
-        uses: olafurpg/setup-scala@v5
+        uses: olafurpg/setup-scala@v10
+        env:
+          ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'
         with:
           java-version: ${{ matrix.java }}
 
@@ -80,3 +82,4 @@ jobs:
       - run: sbt ++${{ matrix.scala }} ";project docs; publishMicrosite";
         env:
           SBT_MICROSITES_PUBLISH_TOKEN: ${{ secrets.VINYLDNS_MICROSITE }}
+          ACTIONS_ALLOW_UNSECURE_COMMANDS: 'true'

docker/api/docker.conf

@@ -67,7 +67,8 @@ vinyldns {
                 key = ${?DEFAULT_DNS_KEY_SECRET}
                 primary-server = "vinyldns-bind9"
                 primary-server = ${?DEFAULT_DNS_ADDRESS}
-              }
+              },
+              tsig-usage = "always"
             },
             {
               id = "func-test-backend"
@@ -88,7 +89,8 @@ vinyldns {
                 key = ${?DEFAULT_DNS_KEY_SECRET}
                 primary-server = "vinyldns-bind9"
                 primary-server = ${?DEFAULT_DNS_ADDRESS}
-              }
+              },
+              tsig-usage = "always"
             }
           ]
         }

docker/bind9/etc/named.conf.local

@@ -218,3 +218,10 @@ zone "list-records" {
 	file "/var/bind/list-records.hosts";
 	allow-update { key "vinyldns."; };
 	};
+
+zone "open" {
+	type master;
+	file "/var/bind/open.hosts";
+	allow-update { any; };
+	allow-transfer { any; };
+	};

docker/bind9/zones/open.hosts

@@ -0,0 +1,8 @@
+$ttl 38400
+open.	IN	SOA	172.17.42.1. admin.test.com. (
+			1439234395
+			10800
+			3600
+			604800
+			38400 )
+open.      IN  NS  172.17.42.1.

modules/api/src/it/scala/vinyldns/api/backend/dns/DnsBackendIntegrationSpec.scala

@@ -0,0 +1,94 @@
+/*
+ * Copyright 2018 Comcast Cable Communications Management, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package vinyldns.api.backend.dns
+
+import org.joda.time.DateTime
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+import vinyldns.api.backend.dns.DnsProtocol.NoError
+import vinyldns.core.crypto.NoOpCrypto
+import vinyldns.core.domain.record.{
+  AData,
+  RecordSet,
+  RecordSetChange,
+  RecordSetChangeType,
+  RecordSetStatus,
+  RecordType
+}
+import vinyldns.core.domain.zone.{Algorithm, Zone, ZoneConnection}
+
+class DnsBackendIntegrationSpec extends AnyWordSpec with Matchers {
+
+  private val testConnection = ZoneConnection(
+    "test",
+    "test",
+    "nzisn+4G2ldMn0q1CV3vsg==",
+    "127.0.0.1:19001",
+    Algorithm.HMAC_MD5
+  )
+
+  "DNSBackend" should {
+    "connect to a zone without a tsig key for transfer or update" in {
+      val config =
+        DnsBackendConfig(
+          "test",
+          testConnection,
+          Some(testConnection),
+          DnsTsigUsage.Never
+        )
+
+      val backend = DnsBackend(
+        "test",
+        config.zoneConnection,
+        config.transferConnection,
+        NoOpCrypto.instance,
+        config.tsigUsage
+      )
+      val testZone = Zone(
+        "open.",
+        "test@test.com",
+        connection = Some(testConnection)
+      )
+
+      val records = backend.loadZone(testZone, 100).unsafeRunSync()
+      records should not be empty
+
+      val testRs = RecordSet(
+        testZone.id,
+        "ok",
+        RecordType.A,
+        200,
+        RecordSetStatus.Active,
+        DateTime.now,
+        None,
+        List(AData("10.1.1.1"))
+      )
+      val update = backend
+        .addRecord(
+          RecordSetChange.apply(
+            testZone,
+            testRs,
+            "user",
+            RecordSetChangeType.Create
+          )
+        )
+        .unsafeRunSync()
+
+      update shouldBe a[NoError]
+    }
+  }
+}

modules/api/src/main/scala/vinyldns/api/backend/dns/DnsBackend.scala

@@ -88,7 +88,7 @@ class DnsQuery(val lookup: DNS.Lookup, val zoneName: DNS.Name) {
   def error: String = lookup.getErrorString
 }
 
-final case class TransferInfo(address: SocketAddress, tsig: DNS.TSIG)
+final case class TransferInfo(address: SocketAddress, tsig: Option[DNS.TSIG])
 
 class DnsBackend(val id: String, val resolver: DNS.SimpleResolver, val xfrInfo: TransferInfo)
     extends Backend
@@ -120,7 +120,11 @@ class DnsBackend(val id: String, val resolver: DNS.SimpleResolver, val xfrInfo:
 
   def loadZone(zone: Zone, maxZoneSize: Int): IO[List[RecordSet]] = {
     val dnsZoneName = zoneDnsName(zone.name)
-    val zti = DNS.ZoneTransferIn.newAXFR(dnsZoneName, xfrInfo.address, xfrInfo.tsig)
+
+    // Use null for tsig key if none
+    val zti = xfrInfo.tsig
+      .map(key => DNS.ZoneTransferIn.newAXFR(dnsZoneName, xfrInfo.address, key))
+      .getOrElse(DNS.ZoneTransferIn.newAXFR(dnsZoneName, xfrInfo.address, null))
 
     for {
       zoneXfr <- IO {
@@ -253,27 +257,33 @@ object DnsBackend {
       id: String,
       conn: ZoneConnection,
       xfrConn: Option[ZoneConnection],
-      crypto: CryptoAlgebra
+      crypto: CryptoAlgebra,
+      tsigUsage: DnsTsigUsage = DnsTsigUsage.UpdateAndTransfer
   ): DnsBackend = {
-    val tsig = createTsig(conn, crypto)
+    val updateTsig = if (tsigUsage.forUpdates) Some(createTsig(conn, crypto)) else None
-    val resolver = createResolver(conn, tsig)
+    val xfrTsig =
+      if (tsigUsage.forTransfers)
+        xfrConn.map(createTsig(_, crypto)).orElse(Some(createTsig(conn, crypto)))
+      else None
 
+    // if we do not use key for updates, do not create the resolver with it
+    val updateResolver = createResolver(conn, updateTsig)
+
+    // fallback to the update connection if we have no transfer connection
     val xfrInfo = xfrConn
       .map { xc =>
-        val xt = createTsig(xc, crypto)
+        val xr = createResolver(xc, xfrTsig)
-        val xr = createResolver(xc, xt)
+        TransferInfo(xr.getAddress, xfrTsig)
-        TransferInfo(xr.getAddress, xt)
       }
-      .getOrElse(TransferInfo(resolver.getAddress, tsig))
+      .getOrElse(TransferInfo(updateResolver.getAddress, xfrTsig))
-    new DnsBackend(id, resolver, xfrInfo)
+    new DnsBackend(id, updateResolver, xfrInfo)
   }
 
-  def createResolver(conn: ZoneConnection, tsig: DNS.TSIG): DNS.SimpleResolver = {
+  def createResolver(conn: ZoneConnection, tsig: Option[DNS.TSIG]): DNS.SimpleResolver = {
     val (host, port) = parseHostAndPort(conn.primaryServer)
     val resolver = new DNS.SimpleResolver(host)
     resolver.setPort(port)
-    resolver.setTSIGKey(tsig)
+    tsig.foreach(resolver.setTSIGKey)
-
     resolver
   }
 

modules/api/src/main/scala/vinyldns/api/backend/dns/DnsBackendConfig.scala

@@ -22,8 +22,9 @@ import vinyldns.core.domain.zone.ZoneConnection
 final case class DnsBackendConfig(
     id: String,
     zoneConnection: ZoneConnection,
-    transferConnection: Option[ZoneConnection]
+    transferConnection: Option[ZoneConnection],
+    tsigUsage: DnsTsigUsage
 ) {
   def toDnsConnection(crypto: CryptoAlgebra): DnsBackend =
-    DnsBackend.apply(id, zoneConnection, transferConnection, crypto)
+    DnsBackend.apply(id, zoneConnection, transferConnection, crypto, tsigUsage)
 }

modules/api/src/main/scala/vinyldns/api/backend/dns/DnsTsigUsage.scala

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2018 Comcast Cable Communications Management, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package vinyldns.api.backend.dns
+
+import cats.implicits._
+import pureconfig.ConfigReader
+import pureconfig.error.FailureReason
+import vinyldns.api.backend.dns.DnsTsigUsage.{TransferOnly, UpdateAndTransfer, UpdateOnly}
+
+sealed abstract class DnsTsigUsage(val humanized: String) {
+  def forUpdates: Boolean = this match {
+    case UpdateOnly | UpdateAndTransfer => true
+    case _ => false
+  }
+  def forTransfers: Boolean = this match {
+    case TransferOnly | UpdateAndTransfer => true
+    case _ => false
+  }
+}
+object DnsTsigUsage {
+  final case object UpdateOnly extends DnsTsigUsage("update")
+  final case object TransferOnly extends DnsTsigUsage("transfer")
+  final case object UpdateAndTransfer extends DnsTsigUsage("always")
+  final case object Never extends DnsTsigUsage("never")
+
+  val Values: List[DnsTsigUsage] = List(UpdateOnly, TransferOnly, UpdateAndTransfer, Never)
+
+  def fromString(value: String): Option[DnsTsigUsage] = Values.find(_.humanized == value)
+
+  implicit val configReader: ConfigReader[DnsTsigUsage] = ConfigReader.fromString { value =>
+    fromString(value).getOrElse(UpdateAndTransfer).asRight[FailureReason]
+  }
+}

modules/api/src/test/scala/vinyldns/api/backend/dns/DnsBackendSpec.scala

@@ -84,7 +84,7 @@ class DnsBackendSpec
   private val mockDnsQuery = mock[DnsQuery]
   private val mockSocketAddress = mock[SocketAddress]
   private val mockTsig = mock[TSIG]
-  private val transferInfo = TransferInfo(mockSocketAddress, mockTsig)
+  private val transferInfo = TransferInfo(mockSocketAddress, Some(mockTsig))
   private val underTest = new DnsBackend("test", mockResolver, transferInfo) {
     override def toQuery(
         name: String,
@@ -126,11 +126,34 @@ class DnsBackendSpec
   }
 
   "Creating a Dns Connection" should {
+    "omit the tsig key for transfers when update only" in {
+      val backend =
+        DnsBackend("test", zoneConnection, None, new NoOpCrypto(), DnsTsigUsage.UpdateOnly)
+      backend.xfrInfo.tsig shouldBe empty
+    }
+
+    "omit the tsig key for transfers when never" in {
+      val backend = DnsBackend("test", zoneConnection, None, new NoOpCrypto(), DnsTsigUsage.Never)
+      backend.xfrInfo.tsig shouldBe empty
+    }
+
+    "include the tsig key for transfers when transfer only" in {
+      val backend =
+        DnsBackend("test", zoneConnection, None, new NoOpCrypto(), DnsTsigUsage.TransferOnly)
+      backend.xfrInfo.tsig shouldBe defined
+    }
+
+    "include the tsig key for transfers when transfer and update" in {
+      val backend =
+        DnsBackend("test", zoneConnection, None, new NoOpCrypto(), DnsTsigUsage.UpdateAndTransfer)
+      backend.xfrInfo.tsig shouldBe defined
+    }
+
     "decrypt the zone connection" in {
       val conn = spy(zoneConnection)
       DnsBackend("test", conn, None, new NoOpCrypto())
 
-      verify(conn).decrypted(any[CryptoAlgebra])
+      verify(conn, times(2)).decrypted(any[CryptoAlgebra])
     }
 
     "parse the port when specified on the primary server" in {

modules/api/src/test/scala/vinyldns/api/backend/dns/DnsTsigUsageSpec.scala

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2018 Comcast Cable Communications Management, LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package vinyldns.api.backend.dns
+
+import com.typesafe.config.ConfigFactory
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class DnsTsigUsageSpec extends AnyWordSpec with Matchers {
+
+  "DnsTsigUsage" should {
+    "convert from string" in {
+      DnsTsigUsage.Values.foreach(v => DnsTsigUsage.fromString(v.humanized) shouldBe defined)
+    }
+    "return none from string when no match" in {
+      DnsTsigUsage.fromString("foo") shouldBe empty
+    }
+    "parse from config" in {
+      DnsTsigUsage.Values.foreach { u =>
+        val config = ConfigFactory.parseString(s"""{ tsig-usage = "${u.humanized}" } """)
+        DnsTsigUsage.configReader.from(config.getValue("tsig-usage")).toOption shouldBe defined
+      }
+    }
+    "default to use all when not found" in {
+      val config = ConfigFactory.parseString(s"""{ tsig-usage = "foo" } """)
+      DnsTsigUsage.configReader.from(config.getValue("tsig-usage")).toOption shouldBe Some(
+        DnsTsigUsage.UpdateAndTransfer
+      )
+    }
+  }
+}