ENT-1151: Rework unicode block validation rules (#2125)

* Redo legal name validation rules so that direction change chars are rejected
* Split name validation into minimal rules that all nodes can require, plus extended rules that the Doorman will apply (and we may need to change, without updating the entire network).
* Break down name validation rule sets to better match expectations
* Add test for nulls in Corda names

core/src/main/kotlin/net/corda/core/identity/CordaX500Name.kt

@@ -45,7 +45,7 @@ data class CordaX500Name(val commonName: String?,
 
     init {
         // Legal name checks.
-        LegalNameValidator.validateOrganization(organisation)
+        LegalNameValidator.validateOrganization(organisation, LegalNameValidator.Validation.MINIMAL)
 
         // Attribute data width checks.
         require(country.length == LENGTH_COUNTRY) { "Invalid country '$country' Country code must be $LENGTH_COUNTRY letters ISO code " }

core/src/main/kotlin/net/corda/core/internal/LegalNameValidator.kt

@@ -1,18 +1,27 @@
 package net.corda.core.internal
 
-import java.lang.Character.UnicodeScript.*
+import net.corda.core.internal.LegalNameValidator.normalize
 import java.text.Normalizer
-import java.util.regex.Pattern
 import javax.security.auth.x500.X500Principal
 
 object LegalNameValidator {
+    enum class Validation {
+        MINIMAL,
+        FULL
+    }
+
     @Deprecated("Use validateOrganization instead", replaceWith = ReplaceWith("validateOrganization(normalizedLegalName)"))
-    fun validateLegalName(normalizedLegalName: String) = validateOrganization(normalizedLegalName)
+    fun validateLegalName(normalizedLegalName: String) = validateOrganization(normalizedLegalName, Validation.FULL)
 
     /**
      * The validation function validates a string for use as part of a legal name. It applies the following rules:
      *
-     * - No blacklisted words like "node", "server".
+     * - Does not contain the null character
+     * - Must be normalized (as per the [normalize] function).
+     * - Length must be 255 characters or shorter.
+     *
+     * Full validation (typically this is only done for names the Doorman approves) adds:
+     *
      * - Restrict names to Latin scripts for now to avoid right-to-left issues, debugging issues when we can't pronounce
      *   names over the phone, and character confusability attacks.
      * - No commas or equals signs.
@@ -20,25 +29,37 @@ object LegalNameValidator {
      *
      * @throws IllegalArgumentException if the name does not meet the required rules. The message indicates why not.
      */
-    fun validateNameAttribute(normalizedNameAttribute: String) {
+    fun validateNameAttribute(normalizedNameAttribute: String, validation: Validation) {
-        Rule.baseNameRules.forEach { it.validate(normalizedNameAttribute) }
+        when (validation) {
+            Validation.MINIMAL -> Rule.attributeRules.forEach { it.validate(normalizedNameAttribute) }
+            Validation.FULL -> Rule.attributeFullRules.forEach { it.validate(normalizedNameAttribute) }
+        }
     }
 
     /**
      * The validation function validates a string for use as the organization attribute of a name, which includes additional
-     * constraints over basic name attribute checks. It applies the following rules:
+     * constraints over basic name attribute checks. It applies the following additional rules:
      *
+     * - Must be normalized (as per the [normalize] function).
+     * - Length must be 255 characters or shorter.
      * - No blacklisted words like "node", "server".
+     * - Must consist of at least three letters.
+     *
+     * Full validation (typically this is only done for names the Doorman approves) adds:
+     *
      * - Restrict names to Latin scripts for now to avoid right-to-left issues, debugging issues when we can't pronounce
      *   names over the phone, and character confusability attacks.
-     * - Must consist of at least three letters and should start with a capital letter.
+     * - Must start with a capital letter.
      * - No commas or equals signs.
      * - No dollars or quote marks, we might need to relax the quote mark constraint in future to handle Irish company names.
      *
      * @throws IllegalArgumentException if the name does not meet the required rules. The message indicates why not.
      */
-    fun validateOrganization(normalizedOrganization: String) {
+    fun validateOrganization(normalizedOrganization: String, validation: Validation) {
-        Rule.legalNameRules.forEach { it.validate(normalizedOrganization) }
+        when (validation) {
+            Validation.MINIMAL -> Rule.legalNameRules.forEach { it.validate(normalizedOrganization) }
+            Validation.FULL -> Rule.legalNameFullRules.forEach { it.validate(normalizedOrganization) }
+        }
     }
 
     @Deprecated("Use normalize instead", replaceWith = ReplaceWith("normalize(legalName)"))
@@ -57,18 +78,27 @@ object LegalNameValidator {
 
     sealed class Rule<in T> {
         companion object {
-            val baseNameRules: List<Rule<String>> = listOf(
+            val attributeRules: List<Rule<String>> = listOf(
                     UnicodeNormalizationRule(),
-                    CharacterRule(',', '=', '$', '"', '\'', '\\'),
-                    WordRule("node", "server"),
                     LengthRule(maxLength = 255),
+                    MustHaveAtLeastTwoLettersRule(),
+                    CharacterRule('\u0000') // Ban null
+            )
+            val attributeFullRules: List<Rule<String>> = attributeRules + listOf(
+                    CharacterRule(',', '=', '$', '"', '\'', '\\'),
                     // TODO: Implement confusable character detection if we add more scripts.
-                    UnicodeRangeRule(LATIN, COMMON, INHERITED),
+                    UnicodeRangeRule(Character.UnicodeBlock.BASIC_LATIN),
+                    CapitalLetterRule()
+            )
+            val legalNameRules: List<Rule<String>> = attributeRules + listOf(
+                    WordRule("node", "server"),
                     X500NameRule()
             )
-            val legalNameRules: List<Rule<String>> = baseNameRules + listOf(
+            val legalNameFullRules: List<Rule<String>> = legalNameRules + listOf(
-                    CapitalLetterRule(),
+                    CharacterRule(',', '=', '$', '"', '\'', '\\'),
-                    MustHaveAtLeastTwoLettersRule()
+                    // TODO: Implement confusable character detection if we add more scripts.
+                    UnicodeRangeRule(Character.UnicodeBlock.BASIC_LATIN),
+                    CapitalLetterRule()
             )
         }
 
@@ -80,18 +110,13 @@ object LegalNameValidator {
             }
         }
 
-        private class UnicodeRangeRule(vararg supportScripts: Character.UnicodeScript) : Rule<String>() {
+        private class UnicodeRangeRule(vararg supportScripts: Character.UnicodeBlock) : Rule<String>() {
-            private val pattern = supportScripts.map { "\\p{Is$it}" }.joinToString(separator = "", prefix = "[", postfix = "]*").let { Pattern.compile(it) }
+            val supportScriptsSet = supportScripts.toSet()
 
             override fun validate(legalName: String) {
-                require(pattern.matcher(legalName).matches()) {
+                val illegalChars = legalName.toCharArray().filter { Character.UnicodeBlock.of(it) !in supportScriptsSet }.size
-                    val illegalChars = legalName.replace(pattern.toRegex(), "").toSet()
+                // We don't expose the characters or the legal name, for security reasons
-                    if (illegalChars.size > 1) {
+                require (illegalChars == 0) { "$illegalChars forbidden characters in legal name." }
-                        "Forbidden characters $illegalChars in \"$legalName\"."
-                    } else {
-                        "Forbidden character $illegalChars in \"$legalName\"."
-                    }
-                }
             }
         }
 

core/src/test/kotlin/net/corda/core/internal/LegalNameValidatorTest.kt

@@ -8,55 +8,94 @@ class LegalNameValidatorTest {
     @Test
     fun `no double spaces`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Test Legal  Name")
+            LegalNameValidator.validateOrganization("Test Legal  Name", LegalNameValidator.Validation.FULL)
         }
-        LegalNameValidator.validateOrganization(LegalNameValidator.normalize("Test Legal  Name"))
+        LegalNameValidator.validateOrganization(LegalNameValidator.normalize("Test Legal  Name"), LegalNameValidator.Validation.FULL)
     }
 
     @Test
     fun `no trailing white space`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Test ")
+            LegalNameValidator.validateOrganization("Test ", LegalNameValidator.Validation.FULL)
         }
     }
 
     @Test
     fun `no prefixed white space`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization(" Test")
+            LegalNameValidator.validateOrganization(" Test", LegalNameValidator.Validation.FULL)
         }
     }
 
     @Test
     fun `blacklisted words`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Test Server")
+            LegalNameValidator.validateOrganization("Test Server", LegalNameValidator.Validation.FULL)
         }
     }
 
     @Test
     fun `blacklisted characters`() {
-        LegalNameValidator.validateOrganization("Test")
+        LegalNameValidator.validateOrganization("Test", LegalNameValidator.Validation.FULL)
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("\$Test")
+            LegalNameValidator.validateOrganization("\$Test", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("\"Test")
+            LegalNameValidator.validateOrganization("\"Test", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("\'Test")
+            LegalNameValidator.validateOrganization("\'Test", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("=Test")
+            LegalNameValidator.validateOrganization("=Test", LegalNameValidator.Validation.FULL)
         }
     }
 
     @Test
-    fun `unicode range`() {
+    fun `unicode range in organization`() {
-        LegalNameValidator.validateOrganization("Test A")
+        LegalNameValidator.validateOrganization("The quick brown fox jumped over the lazy dog.1234567890", LegalNameValidator.Validation.FULL)
+        assertFailsWith(IllegalArgumentException::class) {
+            // Null
+            LegalNameValidator.validateOrganization("\u0000R3 Null", LegalNameValidator.Validation.FULL)
+        }
+        assertFailsWith(IllegalArgumentException::class) {
+            // Right to left direction override
+            LegalNameValidator.validateOrganization("\u202EdtL 3R", LegalNameValidator.Validation.FULL)
+        }
         assertFailsWith(IllegalArgumentException::class) {
             // Greek letter A.
-            LegalNameValidator.validateOrganization("Test Α")
+            LegalNameValidator.validateOrganization("Test \u0391", LegalNameValidator.Validation.FULL)
+        }
+        // Latin capital letter turned m
+        assertFailsWith<IllegalArgumentException> {
+            LegalNameValidator.validateOrganization( "Test\u019CLtd", LegalNameValidator.Validation.FULL)
+        }
+        // Latin small letter turned e
+        assertFailsWith<IllegalArgumentException> {
+            LegalNameValidator.validateOrganization("Test\u01ddLtd", LegalNameValidator.Validation.FULL)
+        }
+    }
+
+    @Test
+    fun `unicode range in general attributes`() {
+        LegalNameValidator.validateNameAttribute("The quick brown fox jumped over the lazy dog.1234567890", LegalNameValidator.Validation.FULL)
+        assertFailsWith(IllegalArgumentException::class) {
+            // Right to left direction override
+            LegalNameValidator.validateNameAttribute("\u202EdtL 3R", LegalNameValidator.Validation.FULL)
+        }
+        // Right to left direction override is okay with minimal validation though
+        LegalNameValidator.validateNameAttribute("\u202EdtL 3R", LegalNameValidator.Validation.MINIMAL)
+        assertFailsWith(IllegalArgumentException::class) {
+            // Greek letter A.
+            LegalNameValidator.validateNameAttribute("Test \u0391", LegalNameValidator.Validation.FULL)
+        }
+        // Latin capital letter turned m
+        assertFailsWith<IllegalArgumentException> {
+            LegalNameValidator.validateNameAttribute( "Test\u019CLtd", LegalNameValidator.Validation.FULL)
+        }
+        // Latin small letter turned e
+        assertFailsWith<IllegalArgumentException> {
+            LegalNameValidator.validateNameAttribute("Test\u01ddLtd", LegalNameValidator.Validation.FULL)
         }
     }
 
@@ -66,21 +105,21 @@ class LegalNameValidatorTest {
         while (longLegalName.length < 255) {
             longLegalName.append("A")
         }
-        LegalNameValidator.validateOrganization(longLegalName.toString())
+        LegalNameValidator.validateOrganization(longLegalName.toString(), LegalNameValidator.Validation.FULL)
 
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization(longLegalName.append("A").toString())
+            LegalNameValidator.validateOrganization(longLegalName.append("A").toString(), LegalNameValidator.Validation.FULL)
         }
     }
 
     @Test
     fun `legal name should be capitalized`() {
-        LegalNameValidator.validateOrganization("Good legal name")
+        LegalNameValidator.validateOrganization("Good legal name", LegalNameValidator.Validation.FULL)
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("bad name")
+            LegalNameValidator.validateOrganization("bad name", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("bad Name")
+            LegalNameValidator.validateOrganization("bad Name", LegalNameValidator.Validation.FULL)
         }
     }
 
@@ -90,13 +129,13 @@ class LegalNameValidatorTest {
         assertEquals("Legal Name With Unicode Whitespaces", LegalNameValidator.normalize("Legal Name\u2004With\u0009Unicode\u0020Whitespaces"))
         assertEquals("Legal Name With Line Breaks", LegalNameValidator.normalize("Legal Name With\n\rLine\nBreaks"))
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Legal Name With\tTab")
+            LegalNameValidator.validateOrganization("Legal Name With\tTab", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Legal Name\u2004With\u0009Unicode\u0020Whitespaces")
+            LegalNameValidator.validateOrganization("Legal Name\u2004With\u0009Unicode\u0020Whitespaces", LegalNameValidator.Validation.FULL)
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateOrganization("Legal Name With\n\rLine\nBreaks")
+            LegalNameValidator.validateOrganization("Legal Name With\n\rLine\nBreaks", LegalNameValidator.Validation.FULL)
         }
     }
 }

docs/source/generating-a-node.rst

@@ -50,17 +50,21 @@ The name must also obey the following constraints:
 
 * The country attribute is a valid ISO 3166-1 two letter code in upper-case
 
-* The organisation field of the name obeys the following constraints:
+* All attributes must obey the following constraints:
 
     * Upper-case first letter
     * Has at least two letters
     * No leading or trailing whitespace
-    * No double-spacing
-    * Does not contain the words "node" or "server"
     * Does not include the following characters: ``,`` , ``=`` , ``$`` , ``"`` , ``'`` , ``\``
     * Is in NFKC normalization form
+    * Does not contain the null character
     * Only the latin, common and inherited unicode scripts are supported
 
+* The organisation field of the name also obeys the following constraints:
+
+    * No double-spacing
+    * Does not contain the words "node" or "server"
+
         * This is to avoid right-to-left issues, debugging issues when we can't pronounce names over the phone, and
           character confusability attacks
 
@@ -149,4 +153,4 @@ in the ``deployNodes`` task, plus a ``runnodes`` shell script (or batch file on
 for testing and development purposes. If you make any changes to your CorDapp source or ``deployNodes`` task, you will
 need to re-run the task to see the changes take effect.
 
-You can now run the nodes by following the instructions in :doc:`Running a node <running-a-node>`.
+You can now run the nodes by following the instructions in :doc:`Running a node <running-a-node>`.

tools/demobench/src/main/kotlin/net/corda/demobench/views/NodeTabView.kt

@@ -204,14 +204,15 @@ class NodeTabView : Fragment() {
 
     private fun Pane.nodeNameField() = textfield(model.legalName) {
         minWidth = textWidth
-        validator {
+        validator { rawName ->
-            if (it == null) {
+            val normalizedName: String? = rawName?.let(LegalNameValidator::normalize)
+            if (normalizedName == null) {
                 error("Node name is required")
-            } else if (nodeController.nameExists(LegalNameValidator.normalize(it))) {
+            } else if (nodeController.nameExists(normalizedName)) {
                 error("Node with this name already exists")
             } else {
                 try {
-                    LegalNameValidator.validateOrganization(LegalNameValidator.normalize(it))
+                    LegalNameValidator.validateOrganization(normalizedName, LegalNameValidator.Validation.MINIMAL)
                     null
                 } catch (e: IllegalArgumentException) {
                     error(e.message)