Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added forward slash validation for forward and reverse zone #1134

Merged
merged 11 commits into from
Sep 13, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ import scala.util.matching.Regex
Object to house common domain validations
*/
object DomainValidations {
val validReverseZoneFQDNRegex: Regex =
"""^(?:([0-9a-zA-Z\-\/_]{1,63}|[0-9a-zA-Z\-\/_]{1}[0-9a-zA-Z\-\/_]{0,61}[0-9a-zA-Z\-\/_]{1}|[*.]{2}[0-9a-zA-Z\-\/_]{0,60}[0-9a-zA-Z\-\/_]{1})\.)*$""".r
val validForwardZoneFQDNRegex: Regex =
"""^(?:([0-9a-zA-Z_]{1,63}|[0-9a-zA-Z_]{1}[0-9a-zA-Z\-_]{0,61}[0-9a-zA-Z_]{1}|[*.]{2}[0-9a-zA-Z\-_]{0,60}[0-9a-zA-Z_]{1})\.)*$""".r
val validFQDNRegex: Regex =
"""^(?:([0-9a-zA-Z_]{1,63}|[0-9a-zA-Z_]{1}[0-9a-zA-Z\-\/_]{0,61}[0-9a-zA-Z_]{1}|[*.]{2}[0-9a-zA-Z\-\/_]{0,60}[0-9a-zA-Z_]{1})\.)*$""".r
val validIpv4Regex: Regex =
Expand Down Expand Up @@ -60,6 +64,30 @@ object DomainValidations {
def validateHostName(name: Fqdn): ValidatedNel[DomainValidationError, Fqdn] =
validateHostName(name.fqdn).map(_ => name)

def validateCname(name: Fqdn, isReverse: Boolean): ValidatedNel[DomainValidationError, Fqdn] =
validateCname(name.fqdn, isReverse).map(_ => name)

def validateCname(name: String, isReverse: Boolean): ValidatedNel[DomainValidationError, String] = {
isReverse match {
case true =>
val checkRegex = validReverseZoneFQDNRegex
.findFirstIn(name)
.map(_.validNel)
.getOrElse(InvalidCname(name,isReverse).invalidNel)
val checkLength = validateStringLength(name, Some(HOST_MIN_LENGTH), HOST_MAX_LENGTH)

checkRegex.combine(checkLength).map(_ => name)
case false =>
val checkRegex = validForwardZoneFQDNRegex
.findFirstIn(name)
.map(_.validNel)
.getOrElse(InvalidCname(name,isReverse).invalidNel)
val checkLength = validateStringLength(name, Some(HOST_MIN_LENGTH), HOST_MAX_LENGTH)

checkRegex.combine(checkLength).map(_ => name)
}
}

def validateHostName(name: String): ValidatedNel[DomainValidationError, String] = {
/*
Label rules are as follows (from RFC 952; detailed in RFC 1034):
Expand All @@ -85,6 +113,8 @@ object DomainValidations {
checkRegex.combine(checkLength).map(_ => name)
}



def validateIpv4Address(address: String): ValidatedNel[DomainValidationError, String] =
validIpv4Regex
.findFirstIn(address)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class BatchChangeValidations(
isApproved: Boolean
): SingleValidation[Unit] = {
val validTTL = addChangeInput.ttl.map(validateTTL(_).asUnit).getOrElse(().valid)
val validRecord = validateRecordData(addChangeInput.record)
val validRecord = validateRecordData(addChangeInput.record, addChangeInput)
val validInput = validateInputName(addChangeInput, isApproved)

validTTL |+| validRecord |+| validInput
Expand All @@ -222,19 +222,26 @@ class BatchChangeValidations(
isApproved: Boolean
): SingleValidation[Unit] = {
val validRecord = deleteRRSetChangeInput.record match {
case Some(recordData) => validateRecordData(recordData)
case Some(recordData) => validateRecordData(recordData, deleteRRSetChangeInput)
case None => ().validNel
}
val validInput = validateInputName(deleteRRSetChangeInput, isApproved)

validRecord |+| validInput
}

def validateRecordData(record: RecordData): SingleValidation[Unit] =
def validateRecordData(record: RecordData,change: ChangeInput): SingleValidation[Unit] =
record match {
case a: AData => validateIpv4Address(a.address).asUnit
case aaaa: AAAAData => validateIpv6Address(aaaa.address).asUnit
case cname: CNAMEData => validateHostName(cname.cname).asUnit
case cname: CNAMEData =>
/*
To validate the zone is reverse
*/
val isIPv4: Boolean = change.inputName.toLowerCase.endsWith("in-addr.arpa.")
val isIPv6: Boolean = change.inputName.toLowerCase.endsWith("ip6.arpa.")
val isReverse: Boolean = isIPv4 || isIPv6
validateCname(cname.cname,isReverse).asUnit
case ptr: PTRData => validateHostName(ptr.ptrdname).asUnit
case txt: TXTData => validateTxtTextLength(txt.text).asUnit
case mx: MXData =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1974,7 +1974,7 @@ def test_cname_recordtype_add_checks(shared_zone_test_context):
error_messages=['Invalid TTL: "29", must be a number between 30 and 2147483647.',
f'Invalid domain name: "bad-ttl-and-invalid-name$.{parent_zone_name}", '
"valid domain names must be letters, numbers, underscores, and hyphens, joined by dots, and terminated with a dot.",
'Invalid domain name: "also$bad.name.", valid domain names must be letters, numbers, underscores, and hyphens, '
'Invalid Cname: "also$bad.name.", valid cnames must be letters, numbers, underscores, and hyphens, '
"joined by dots, and terminated with a dot."])
# zone discovery failure
assert_failed_change_in_error_response(response[7], input_name="no.zone.com.", record_type="CNAME",
Expand Down Expand Up @@ -2134,7 +2134,7 @@ def test_cname_recordtype_update_delete_checks(shared_zone_test_context):
error_messages=['Invalid TTL: "20", must be a number between 30 and 2147483647.',
'Invalid domain name: "$another.invalid.host.name.", valid domain names must be letters, numbers, '
'underscores, and hyphens, joined by dots, and terminated with a dot.',
'Invalid domain name: "$another.invalid.cname.", valid domain names must be letters, numbers, '
'Invalid Cname: "$another.invalid.cname.", valid cnames must be letters, numbers, '
'underscores, and hyphens, joined by dots, and terminated with a dot.'])

# zone discovery failure
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ package vinyldns.api.domain
import cats.scalatest.ValidatedMatchers
import org.scalacheck._
import org.scalatestplus.scalacheck.ScalaCheckDrivenPropertyChecks
import org.scalatest._
import org.scalatest.propspec.AnyPropSpec
import org.scalatest.matchers.should.Matchers
import vinyldns.api.ValidationTestImprovements._
import vinyldns.core.domain.{InvalidDomainName, InvalidLength}
import vinyldns.core.domain.{InvalidDomainName, InvalidCname, InvalidLength}

class DomainValidationsSpec
extends AnyPropSpec
Expand Down Expand Up @@ -111,4 +110,52 @@ class DomainValidationsSpec
val invalidDesc = "a" * 256
validateStringLength(Some(invalidDesc), None, 255).failWith[InvalidLength]
}

property("Shortest cname should be valid") {
validateCname("a.",true) shouldBe valid
validateCname("a.",false) shouldBe valid

}

property("Longest cname should be valid") {
val name = ("a" * 50 + ".") * 5
validateCname(name,true) shouldBe valid
validateCname(name,false) shouldBe valid

}

property("Cnames with underscores should pass property-based testing") {
validateCname("_underscore.domain.name.",true).isValid
validateCname("under_score.domain.name.",true).isValid
validateCname("underscore._domain.name.",true).isValid
validateCname("_underscore.domain.name.",false).isValid
validateCname("under_score.domain.name.",false).isValid
validateCname("underscore._domain.name.",false).isValid
}

// For wildcard records. '*' can only be in the beginning followed by '.' and domain name
property("Cnames beginning with asterisk should pass property-based testing") {
validateCname("*.domain.name.",true) shouldBe valid
validateCname("aste*risk.domain.name.",true) shouldBe invalid
validateCname("*asterisk.domain.name.",true) shouldBe invalid
validateCname("asterisk*.domain.name.",true) shouldBe invalid
validateCname("asterisk.*domain.name.",true) shouldBe invalid
validateCname("asterisk.domain*.name.",true) shouldBe invalid
validateCname("*.domain.name.",false) shouldBe valid
validateCname("aste*risk.domain.name.",false) shouldBe invalid
validateCname("*asterisk.domain.name.",false) shouldBe invalid
validateCname("asterisk*.domain.name.",false) shouldBe invalid
validateCname("asterisk.*domain.name.",false) shouldBe invalid
validateCname("asterisk.domain*.name.",false) shouldBe invalid
}
property("Cname names with forward slash should pass with reverse zone") {
validateCname("/slash.cname.name.",true).isValid
validateCname("slash./cname.name.",true).isValid
validateCname("slash.cname./name.",true).isValid
}
property("Cname names with forward slash should fail with forward zone") {
validateCname("/slash.cname.name.",false).failWith[InvalidCname]
validateCname("slash./cname.name.",false).failWith[InvalidCname]
validateCname("slash.cname./name.",false).failWith[InvalidCname]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ class BatchChangeValidationsSpec
)
val result = validateAddChangeInput(change, false)

result should haveInvalid[DomainValidationError](InvalidDomainName(s"$invalidCNAMERecordData."))
result should haveInvalid[DomainValidationError](InvalidCname(s"$invalidCNAMERecordData.",false))
}

property("""validateAddChangeInput: should fail with InvalidLength
Expand Down Expand Up @@ -2666,4 +2666,21 @@ class BatchChangeValidationsSpec
result(3) shouldBe valid
result(4) shouldBe valid
}

property("validateAddChangeInput: should fail for a CNAME addChangeInput with forward slash for forward zone") {
val cnameWithForwardSlash = AddChangeInput("cname.ok.", RecordType.CNAME, ttl, CNAMEData(Fqdn("cname/")))
val result = validateAddChangeInput(cnameWithForwardSlash, false)
result should haveInvalid[DomainValidationError](InvalidCname("cname/.",false))
}
property("validateAddChangeInput: should succeed for a valid CNAME addChangeInput without forward slash for forward zone") {
val cname = AddChangeInput("cname.ok.", RecordType.CNAME, ttl, CNAMEData(Fqdn("cname")))
val result = validateAddChangeInput(cname, false)
result shouldBe valid
}
property("validateAddChangeInput: should succeed for a valid CNAME addChangeInput with forward slash for reverse zone") {
val cnameWithForwardSlash = AddChangeInput("2.0.192.in-addr.arpa.", RecordType.CNAME, ttl, CNAMEData(Fqdn("cname/")))
val result = validateAddChangeInput(cnameWithForwardSlash, true)
result shouldBe valid
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,18 @@ final case class InvalidDomainName(param: String) extends DomainValidationError
"joined by dots, and terminated with a dot."
}

final case class InvalidCname(param: String, isReverseZone: Boolean) extends DomainValidationError {
def message: String =
isReverseZone match {
case true =>
s"""Invalid Cname: "$param", valid cnames must be letters, numbers, slashes, underscores, and hyphens, """ +
"joined by dots, and terminated with a dot."
case false =>
s"""Invalid Cname: "$param", valid cnames must be letters, numbers, underscores, and hyphens, """ +
"joined by dots, and terminated with a dot."
}
}

final case class InvalidLength(param: String, minLengthInclusive: Int, maxLengthInclusive: Int)
extends DomainValidationError {
def message: String =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ object DomainValidationErrorType extends Enumeration {
type DomainValidationErrorType = Value
// NOTE: once defined, an error code type cannot be changed!
val ChangeLimitExceeded, BatchChangeIsEmpty, GroupDoesNotExist, NotAMemberOfOwnerGroup,
InvalidDomainName, InvalidLength, InvalidEmail, InvalidRecordType, InvalidPortNumber,
InvalidDomainName, InvalidCname, InvalidLength, InvalidEmail, InvalidRecordType, InvalidPortNumber,
InvalidIpv4Address, InvalidIpv6Address, InvalidIPAddress, InvalidTTL, InvalidMxPreference,
InvalidBatchRecordType, ZoneDiscoveryError, RecordAlreadyExists, RecordDoesNotExist,
CnameIsNotUniqueError, UserIsNotAuthorized, UserIsNotAuthorizedError, RecordNameNotUniqueInBatch,
Expand All @@ -46,6 +46,7 @@ object DomainValidationErrorType extends Enumeration {
case _: GroupDoesNotExist => GroupDoesNotExist
case _: NotAMemberOfOwnerGroup => NotAMemberOfOwnerGroup
case _: InvalidDomainName => InvalidDomainName
case _: InvalidCname => InvalidCname
case _: InvalidLength => InvalidLength
case _: InvalidEmail => InvalidEmail
case _: InvalidRecordType => InvalidRecordType
Expand Down