Add X500 name constraints for non-organisation attributes (#2108)

Enforce X500 name constraints consistently across all attributes

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

@@ -9,7 +9,7 @@ import org.bouncycastle.asn1.ASN1ObjectIdentifier
 import org.bouncycastle.asn1.x500.AttributeTypeAndValue
 import org.bouncycastle.asn1.x500.X500Name
 import org.bouncycastle.asn1.x500.style.BCStyle
-import java.util.Locale
+import java.util.*
 import javax.security.auth.x500.X500Principal
 
 /**
@@ -45,7 +45,7 @@ data class CordaX500Name(val commonName: String?,
 
     init {
         // Legal name checks.
-        LegalNameValidator.validateLegalName(organisation)
+        LegalNameValidator.validateOrganization(organisation)
 
         // 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

@@ -6,8 +6,27 @@ import java.util.regex.Pattern
 import javax.security.auth.x500.X500Principal
 
 object LegalNameValidator {
+    @Deprecated("Use validateOrganization instead", replaceWith = ReplaceWith("validateOrganization(normalizedLegalName)"))
+    fun validateLegalName(normalizedLegalName: String) = validateOrganization(normalizedLegalName)
+
     /**
-     * The validation function will validate the input string using the following rules:
+     * 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".
+     * - 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.
+     * - 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 validateNameAttribute(normalizedNameAttribute: String) {
+        Rule.baseNameRules.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:
      *
      * - No blacklisted words like "node", "server".
      * - Restrict names to Latin scripts for now to avoid right-to-left issues, debugging issues when we can't pronounce
@@ -18,16 +37,19 @@ object LegalNameValidator {
      *
      * @throws IllegalArgumentException if the name does not meet the required rules. The message indicates why not.
      */
-    fun validateLegalName(normalizedLegalName: String) {
+    fun validateOrganization(normalizedOrganization: String) {
-        Rule.legalNameRules.forEach { it.validate(normalizedLegalName) }
+        Rule.legalNameRules.forEach { it.validate(normalizedOrganization) }
     }
 
+    @Deprecated("Use normalize instead", replaceWith = ReplaceWith("normalize(legalName)"))
+    fun normalizeLegalName(legalName: String): String = normalize(legalName)
+
     /**
      * The normalize function will trim the input string, replace any multiple spaces with a single space,
      * and normalize the string according to NFKC normalization form.
      */
-    fun normaliseLegalName(legalName: String): String {
+    fun normalize(nameAttribute: String): String {
-        val trimmedLegalName = legalName.trim().replace(WHITESPACE, " ")
+        val trimmedLegalName = nameAttribute.trim().replace(WHITESPACE, " ")
         return Normalizer.normalize(trimmedLegalName, Normalizer.Form.NFKC)
     }
 
@@ -35,15 +57,17 @@ object LegalNameValidator {
 
     sealed class Rule<in T> {
         companion object {
-            val legalNameRules: List<Rule<String>> = listOf(
+            val baseNameRules: List<Rule<String>> = listOf(
                     UnicodeNormalizationRule(),
                     CharacterRule(',', '=', '$', '"', '\'', '\\'),
                     WordRule("node", "server"),
                     LengthRule(maxLength = 255),
                     // TODO: Implement confusable character detection if we add more scripts.
                     UnicodeRangeRule(LATIN, COMMON, INHERITED),
+                    X500NameRule()
+            )
+            val legalNameRules: List<Rule<String>> = baseNameRules + listOf(
                     CapitalLetterRule(),
-                    X500NameRule(),
                     MustHaveAtLeastTwoLettersRule()
             )
         }
@@ -52,7 +76,7 @@ object LegalNameValidator {
 
         private class UnicodeNormalizationRule : Rule<String>() {
             override fun validate(legalName: String) {
-                require(legalName == normaliseLegalName(legalName)) { "Legal name must be normalized. Please use 'normaliseLegalName' to normalize the legal name before validation." }
+                require(legalName == normalize(legalName)) { "Legal name must be normalized. Please use 'normalize' to normalize the legal name before validation." }
             }
         }
 

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

@@ -8,55 +8,55 @@ class LegalNameValidatorTest {
     @Test
     fun `no double spaces`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Test Legal  Name")
+            LegalNameValidator.validateOrganization("Test Legal  Name")
         }
-        LegalNameValidator.validateLegalName(LegalNameValidator.normaliseLegalName("Test Legal  Name"))
+        LegalNameValidator.validateOrganization(LegalNameValidator.normalize("Test Legal  Name"))
     }
 
     @Test
     fun `no trailing white space`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Test ")
+            LegalNameValidator.validateOrganization("Test ")
         }
     }
 
     @Test
     fun `no prefixed white space`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName(" Test")
+            LegalNameValidator.validateOrganization(" Test")
         }
     }
 
     @Test
     fun `blacklisted words`() {
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Test Server")
+            LegalNameValidator.validateOrganization("Test Server")
         }
     }
 
     @Test
     fun `blacklisted characters`() {
-        LegalNameValidator.validateLegalName("Test")
+        LegalNameValidator.validateOrganization("Test")
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("\$Test")
+            LegalNameValidator.validateOrganization("\$Test")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("\"Test")
+            LegalNameValidator.validateOrganization("\"Test")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("\'Test")
+            LegalNameValidator.validateOrganization("\'Test")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("=Test")
+            LegalNameValidator.validateOrganization("=Test")
         }
     }
 
     @Test
     fun `unicode range`() {
-        LegalNameValidator.validateLegalName("Test A")
+        LegalNameValidator.validateOrganization("Test A")
         assertFailsWith(IllegalArgumentException::class) {
             // Greek letter A.
-            LegalNameValidator.validateLegalName("Test Α")
+            LegalNameValidator.validateOrganization("Test Α")
         }
     }
 
@@ -66,37 +66,37 @@ class LegalNameValidatorTest {
         while (longLegalName.length < 255) {
             longLegalName.append("A")
         }
-        LegalNameValidator.validateLegalName(longLegalName.toString())
+        LegalNameValidator.validateOrganization(longLegalName.toString())
 
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName(longLegalName.append("A").toString())
+            LegalNameValidator.validateOrganization(longLegalName.append("A").toString())
         }
     }
 
     @Test
     fun `legal name should be capitalized`() {
-        LegalNameValidator.validateLegalName("Good legal name")
+        LegalNameValidator.validateOrganization("Good legal name")
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("bad name")
+            LegalNameValidator.validateOrganization("bad name")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("bad Name")
+            LegalNameValidator.validateOrganization("bad Name")
         }
     }
 
     @Test
     fun `correctly handle whitespaces`() {
-        assertEquals("Legal Name With Tab", LegalNameValidator.normaliseLegalName("Legal Name With\tTab"))
+        assertEquals("Legal Name With Tab", LegalNameValidator.normalize("Legal Name With\tTab"))
-        assertEquals("Legal Name With Unicode Whitespaces", LegalNameValidator.normaliseLegalName("Legal Name\u2004With\u0009Unicode\u0020Whitespaces"))
+        assertEquals("Legal Name With Unicode Whitespaces", LegalNameValidator.normalize("Legal Name\u2004With\u0009Unicode\u0020Whitespaces"))
-        assertEquals("Legal Name With Line Breaks", LegalNameValidator.normaliseLegalName("Legal Name With\n\rLine\nBreaks"))
+        assertEquals("Legal Name With Line Breaks", LegalNameValidator.normalize("Legal Name With\n\rLine\nBreaks"))
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Legal Name With\tTab")
+            LegalNameValidator.validateOrganization("Legal Name With\tTab")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Legal Name\u2004With\u0009Unicode\u0020Whitespaces")
+            LegalNameValidator.validateOrganization("Legal Name\u2004With\u0009Unicode\u0020Whitespaces")
         }
         assertFailsWith(IllegalArgumentException::class) {
-            LegalNameValidator.validateLegalName("Legal Name With\n\rLine\nBreaks")
+            LegalNameValidator.validateOrganization("Legal Name With\n\rLine\nBreaks")
         }
     }
 }

docs/source/key-concepts-identity.rst

@@ -49,8 +49,17 @@ the minimum supported set for X.509 certificates (specified in RFC 3280), plus t
 * common name (CN) - used only for service identities
 
 The organisation, locality and country attributes are required, while state, organisational-unit and common name are
-optional. Attributes cannot be be present more than once in the name. The "country" code is strictly restricted to valid
+optional. Attributes cannot be be present more than once in the name.
-ISO 3166-1 two letter codes.
+
+All of these attributes have the following set of constraints applied for security reasons:
+
+ - No blacklisted words (currently "node" and "server").
+ - 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.
+ - No dollars or quote marks.
+
+Additionally the "organisation" attribute must consist of at least three letters and starting with a capital letter,
+and "country code" is strictly restricted to valid ISO 3166-1 two letter codes.
 
 Certificates
 ------------

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

@@ -207,11 +207,11 @@ class NodeTabView : Fragment() {
         validator {
             if (it == null) {
                 error("Node name is required")
-            } else if (nodeController.nameExists(LegalNameValidator.normaliseLegalName(it))) {
+            } else if (nodeController.nameExists(LegalNameValidator.normalize(it))) {
                 error("Node with this name already exists")
             } else {
                 try {
-                    LegalNameValidator.validateLegalName(LegalNameValidator.normaliseLegalName(it))
+                    LegalNameValidator.validateOrganization(LegalNameValidator.normalize(it))
                     null
                 } catch (e: IllegalArgumentException) {
                     error(e.message)