Limit the use of extensions term in the codebase (opensearch-project#…

…2748)

* Limit the use of extensions term in the codebase

Extensions are a big part of what we are building into OpenSearch and
Security.  While we need these features we should try to build them as
generically as possible, this checkstyle scan will help enforce this and
also allow for working around the term usage with comments as we do have
places where extensions need to be treated differently.

Signed-off-by: Peter Nied <petern@amazon.com>

checkstyle/sun_checks.xml

@@ -204,14 +204,21 @@
     <property name="severity" value="error"/>
   </module>
 
+  <module name="RegexpSingleline">
+    <property name="format" value="extension"/>
+    <property name="ignoreCase" value="true"/>
+    <property name="message" value="Extension should only be used sparingly to keep implementations as generic as possible" />
+    <property name="severity" value="error"/>
+  </module>
+
   <module name="SuppressWithPlainTextCommentFilter">
     <property name="offCommentFormat" value="CS-SUPPRESS-ALL: .+"/> <!-- Require an explaination after surpressing -->
     <property name="onCommentFormat" value="CS-ENFORCE-ALL"/>
   </module>
 
   <module name="SuppressWithPlainTextCommentFilter">
     <property name="offCommentFormat" value="CS-SUPPRESS-SINGLE\: ([\w\|]+) .+"/> <!-- Require an explaination after surpressing -->
-    <property name="onCommentFormat" value="CS-ENFORCE-SINGLE"/>
+    <property name="onCommentFormat" value="CS-ENFORCE-SINGLE()"/>
     <property name="checkFormat" value="$1"/>
   </module>
 

src/integrationTest/java/org/opensearch/security/SnapshotSteps.java

@@ -41,7 +41,7 @@ public SnapshotSteps(RestHighLevelClient restHighLevelClient) {
 
 	// CS-SUPPRESS-SINGLE: RegexpSingleline It is not possible to use phrase "cluster manager" instead of master here
 	public org.opensearch.action.support.master.AcknowledgedResponse createSnapshotRepository(String repositoryName, String snapshotDirPath, String type)
-	//CS-ENFORCE-SINGLE
+	// CS-ENFORCE-SINGLE
 		throws IOException {
 		PutRepositoryRequest createRepositoryRequest = new PutRepositoryRequest().name(repositoryName).type(type)
 			.settings(Map.of("location", snapshotDirPath));
@@ -70,7 +70,7 @@ public org.opensearch.action.support.master.AcknowledgedResponse deleteSnapshotR
 		return snapshotClient.deleteRepository(request, DEFAULT);
 	}
 
-	//CS-SUPPRESS-SINGLE: RegexpSingleline: It is not possible to use phrase "cluster manager" instead of master here
+	//CS-SUPPRESS-SINGLE: RegexpSingleline It is not possible to use phrase "cluster manager" instead of master here
 	public org.opensearch.action.support.master.AcknowledgedResponse deleteSnapshot(String repositoryName, String snapshotName) throws IOException {
 	//CS-ENFORCE-SINGLE
 		return snapshotClient.delete(new DeleteSnapshotRequest(repositoryName, snapshotName), DEFAULT);

src/integrationTest/java/org/opensearch/test/framework/certificate/CertificateMetadata.java

@@ -9,7 +9,7 @@
 */
 package org.opensearch.test.framework.certificate;
 
-
+// CS-SUPPRESS-SINGLE: RegexpSingleline Extension is used to refer to certificate extensions, keeping this rule disable for the whole file
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -218,3 +218,4 @@ ExtendedKeyUsage getExtendedKeyUsage() {
 		return new ExtendedKeyUsage(usages);
 	}
 }
+// CS-ENFORCE-SINGLE

src/integrationTest/java/org/opensearch/test/framework/certificate/CertificatesIssuer.java

@@ -26,6 +26,7 @@
 
 package org.opensearch.test.framework.certificate;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline Extension is used to refer to certificate extensions, keeping this rule disable for the whole file
 import java.math.BigInteger;
 import java.security.KeyPair;
 import java.security.NoSuchAlgorithmException;
@@ -222,3 +223,4 @@ private BigInteger generateNextCertificateSerialNumber() {
 		return BigInteger.valueOf(ID_COUNTER.incrementAndGet());
 	}
 }
+// CS-ENFORCE-SINGLE

src/integrationTest/java/org/opensearch/test/framework/certificate/PublicKeyUsage.java

@@ -14,6 +14,7 @@
 import org.bouncycastle.asn1.x509.KeyPurposeId;
 import org.bouncycastle.asn1.x509.KeyUsage;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline Extension is used to refer to certificate extensions
 /**
 * The class is associated with certificate extensions related to key usages. These extensions are defined by
 * <a href="https://www.rfc-editor.org/rfc/rfc5280.html">RFC 5280</a> and describes allowed usage of public kay which is embedded in
@@ -25,6 +26,7 @@
 *
 * @see <a href="https://www.rfc-editor.org/rfc/rfc5280.html">RFC 5280</a>
 */
+// CS-ENFORCE-SINGLE
 enum PublicKeyUsage {
 	DIGITAL_SIGNATURE(KeyUsage.digitalSignature),
 	KEY_CERT_SIGN(KeyUsage.keyCertSign),

src/integrationTest/java/org/opensearch/test/framework/certificate/TestCertificates.java

@@ -60,8 +60,8 @@ public class TestCertificates {
 	private static final String CA_SUBJECT = "DC=com,DC=example,O=Example Com Inc.,OU=Example Com Inc. Root CA,CN=Example Com Inc. Root CA";
 	private static final String ADMIN_DN = "CN=kirk,OU=client,O=client,L=test,C=de";
 	private static final int CERTIFICATE_VALIDITY_DAYS = 365;
-	private static final String CERTIFICATE_FILE_EXTENSION = ".cert";
-	private static final String KEY_FILE_EXTENSION = ".key";
+	private static final String CERTIFICATE_FILE_EXT = ".cert";
+	private static final String KEY_FILE_EXT = ".key";
 	private final CertificateData caCertificate;
 	private final CertificateData adminCertificate;
 	private final List<CertificateData> nodeCertificates;
@@ -106,7 +106,7 @@ public CertificateData createSelfSignedCertificate(String distinguishedName) {
 	* @return file which contains certificate in PEM format, defined by <a href="https://www.rfc-editor.org/rfc/rfc1421.txt">RFC 1421</a>
 	*/
 	public File getRootCertificate() {
-		return createTempFile("root", CERTIFICATE_FILE_EXTENSION, caCertificate.certificateInPemFormat());
+		return createTempFile("root", CERTIFICATE_FILE_EXT, caCertificate.certificateInPemFormat());
 	}
 
 	public CertificateData getRootCertificateData() {
@@ -120,7 +120,7 @@ public CertificateData getRootCertificateData() {
 	*/
 	public File getNodeCertificate(int node) {
 		CertificateData certificateData = getNodeCertificateData(node);
-		return createTempFile("node-" + node, CERTIFICATE_FILE_EXTENSION, certificateData.certificateInPemFormat());
+		return createTempFile("node-" + node, CERTIFICATE_FILE_EXT, certificateData.certificateInPemFormat());
 	}
 
 	public CertificateData getNodeCertificateData(int node) {
@@ -178,7 +178,7 @@ public CertificateData getLdapCertificateData() {
 	*/
 	public File getNodeKey(int node, String privateKeyPassword) {
 		CertificateData certificateData = nodeCertificates.get(node);
-		return createTempFile("node-" + node, KEY_FILE_EXTENSION, certificateData.privateKeyInPemFormat(privateKeyPassword));
+		return createTempFile("node-" + node, KEY_FILE_EXT, certificateData.privateKeyInPemFormat(privateKeyPassword));
 	}
 
 	/**
@@ -187,7 +187,7 @@ public File getNodeKey(int node, String privateKeyPassword) {
 	* @return file which contains certificate in PEM format, defined by <a href="https://www.rfc-editor.org/rfc/rfc1421.txt">RFC 1421</a>
 	*/
 	public File getAdminCertificate() {
-		return createTempFile("admin", CERTIFICATE_FILE_EXTENSION, adminCertificate.certificateInPemFormat());
+		return createTempFile("admin", CERTIFICATE_FILE_EXT, adminCertificate.certificateInPemFormat());
 	}
 
 	public CertificateData getAdminCertificateData() {
@@ -202,7 +202,7 @@ public CertificateData getAdminCertificateData() {
 	* by <a href="https://www.rfc-editor.org/rfc/rfc1421.txt">RFC 1421</a>
 	*/
 	public File getAdminKey(String privateKeyPassword) {
-		return createTempFile("admin", KEY_FILE_EXTENSION, adminCertificate.privateKeyInPemFormat(privateKeyPassword));
+		return createTempFile("admin", KEY_FILE_EXT, adminCertificate.privateKeyInPemFormat(privateKeyPassword));
 	}
 
 	public String[] getAdminDNs() {

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -26,6 +26,7 @@
 
 package org.opensearch.security;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
@@ -192,6 +193,7 @@
 import org.opensearch.transport.TransportResponseHandler;
 import org.opensearch.transport.TransportService;
 import org.opensearch.watcher.ResourceWatcherService;
+// CS-ENFORCE-SINGLE
 
 public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin implements ClusterPlugin, MapperPlugin {
 
@@ -1188,6 +1190,7 @@ public static class GuiceHolder implements LifecycleComponent {
         private static IndicesService indicesService;
         private static PitService pitService;
 
+        // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
         private static ExtensionsManager extensionsManager;
 
         @Inject
@@ -1199,6 +1202,7 @@ public GuiceHolder(final RepositoriesService repositoriesService,
             GuiceHolder.pitService = pitService;
             GuiceHolder.extensionsManager = extensionsManager;
         }
+        // CS-ENFORCE-SINGLE
 
         public static RepositoriesService getRepositoriesService() {
             return repositoriesService;
@@ -1214,7 +1218,9 @@ public static IndicesService getIndicesService() {
 
         public static PitService getPitService() { return pitService; }
 
+        // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
         public static ExtensionsManager getExtensionsManager() { return extensionsManager; }
+        // CS-ENFORCE-SINGLE
 
 
         @Override

src/main/java/org/opensearch/security/configuration/DlsFilterLevelActionHandler.java

@@ -142,7 +142,7 @@ private boolean handle() {
             threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_FILTER_LEVEL_DLS_DONE, request.toString());
 
             try {
-                if (!createQueryExtension()) {
+                if (!modifyQuery()) {
                     return true;
                 }
 
@@ -186,7 +186,7 @@ private boolean handle(SearchRequest searchRequest, StoredContext ctx) {
 
         if (localClusterAlias != null) {
             try {
-                createQueryExtension(localClusterAlias);
+                modifyQuery(localClusterAlias);
             } catch (Exception e) {
                 log.error("Unable to handle filter level DLS", e);
                 listener.onFailure(new OpenSearchSecurityException("Unable to handle filter level DLS", e));
@@ -387,11 +387,11 @@ private GetResult searchHitToGetResult(SearchHit hit) {
                 documentFields, metadataFields);
     }
 
-    private boolean createQueryExtension() throws IOException {
-        return createQueryExtension(null);
+    private boolean modifyQuery() throws IOException {
+        return modifyQuery(null);
     }
 
-    private boolean createQueryExtension(String localClusterAlias) throws IOException {
+    private boolean modifyQuery(String localClusterAlias) throws IOException {
         Map<String, Set<String>> filterLevelQueries = evaluatedDlsFlsConfig.getDlsQueriesByIndex();
 
         BoolQueryBuilder dlsQueryBuilder = QueryBuilders.boolQuery().minimumShouldMatch(1);

src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java

@@ -110,8 +110,10 @@ private void printJCEWarnings() {
             final int aesMaxKeyLength = Cipher.getMaxAllowedKeyLength("AES");
 
             if (aesMaxKeyLength < 256) {
+	            // CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions
                 log.info("AES-256 not supported, max key length for AES is {} bit."
                     + " (This is not an issue, it just limits possible encryption strength. To enable AES 256, install 'Java Cryptography Extension (JCE) Unlimited Strength Jurisdiction Policy Files')", aesMaxKeyLength);
+	            // CS-ENFORCE-SINGLE
             }
         } catch (final NoSuchAlgorithmException e) {
             log.error("AES encryption not supported (SG 1). ", e);

src/main/java/org/opensearch/security/ssl/util/CertificateValidator.java

@@ -52,6 +52,7 @@
 import java.util.HashSet;
 import java.util.Set;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline certification extensions is unrelated to OpenSearch extensions
 /**
  * Convenience class to handle validation of certificates, aliases and keystores
  *
@@ -62,6 +63,7 @@
  * IMPORTANT: at least one of the above mechanisms *MUST* be configured and
  * operational, otherwise certificate validation *WILL FAIL* unconditionally.
  */
+// CS-ENFORCE-SINGLE
 public class CertificateValidator
 {
 

src/main/java/org/opensearch/security/ssl/util/TLSUtil.java

@@ -21,7 +21,9 @@ public class TLSUtil {
     private static final int SSL_CONTENT_TYPE_ALERT = 21;
     private static final int SSL_CONTENT_TYPE_HANDSHAKE = 22;
     private static final int SSL_CONTENT_TYPE_APPLICATION_DATA = 23;
+    // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions heartbeat needs special handling by security extension
     private static final int SSL_CONTENT_TYPE_EXTENSION_HEARTBEAT = 24;
+    // CS-ENFORCE-SINGLE
     private static final int SSL_RECORD_HEADER_LENGTH = 5;
 
     private TLSUtil() {
@@ -39,9 +41,11 @@ public static boolean isTLS(ByteBuf buffer) {
             case SSL_CONTENT_TYPE_ALERT:
             case SSL_CONTENT_TYPE_HANDSHAKE:
             case SSL_CONTENT_TYPE_APPLICATION_DATA:
+            // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions heartbeat needs special handling by security extension
             case SSL_CONTENT_TYPE_EXTENSION_HEARTBEAT:
                 tls = true;
                 break;
+            // CS-ENFORCE-SINGLE
             default:
                 // SSLv2 or bad data
                 tls = false;

src/main/java/org/opensearch/security/support/ConfigConstants.java

@@ -97,8 +97,9 @@ public class ConfigConstants {
 
     public static final String OPENDISTRO_SECURITY_SSL_TRANSPORT_TRUSTED_CLUSTER_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX+"ssl_transport_trustedcluster_request";
 
+    // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
     public static final String OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST = OPENDISTRO_SECURITY_CONFIG_PREFIX+"ssl_transport_extension_request";
-
+    // CS-ENFORCE-SINGLE
 
     /**
      * Set by the SSL plugin, this is the peer node certificate on the transport layer

src/main/java/org/opensearch/security/support/HeaderHelper.java

@@ -45,10 +45,11 @@ public static boolean isDirectRequest(final ThreadContext context) {
                   || context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE) == null;
     }
 
+    // CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions
     public static boolean isExtensionRequest(final ThreadContext context) {
         return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST) == Boolean.TRUE;
     }
-
+    // CS-ENFORCE-SINGLE
 
     public static String getSafeFromHeader(final ThreadContext context, final String headerName) {
 

src/main/java/org/opensearch/security/tools/SecurityAdmin.java

@@ -210,8 +210,10 @@ public static int execute(final String[] args) throws Exception {
         options.addOption( "nhnv", "disable-host-name-verification", false, "Disable hostname verification" );
         options.addOption(Option.builder("ts").longOpt("truststore").hasArg().argName("file").desc("Path to truststore (JKS/PKCS12 format)").build());
         options.addOption(Option.builder("ks").longOpt("keystore").hasArg().argName("file").desc("Path to keystore (JKS/PKCS12 format").build());
+        // CS-SUPPRESS-SINGLE: RegexpSingleline file extensions is unrelated to OpenSearch extensions
         options.addOption(Option.builder("tst").longOpt("truststore-type").hasArg().argName("type").desc("JKS or PKCS12, if not given we use the file extension to dectect the type").build());
         options.addOption(Option.builder("kst").longOpt("keystore-type").hasArg().argName("type").desc("JKS or PKCS12, if not given we use the file extension to dectect the type").build());
+        // CS-ENFORCE-SINGLE
         options.addOption(Option.builder("tspass").longOpt("truststore-password").hasArg().argName("password").desc("Truststore password").build());
         options.addOption(Option.builder("kspass").longOpt("keystore-password").hasArg().argName("password").desc("Keystore password").build());
         options.addOption(Option.builder("cd").longOpt("configdir").hasArg().argName("directory").desc("Directory for config files").build());

src/main/java/org/opensearch/security/transport/OIDClusterRequestEvaluator.java

@@ -33,11 +33,13 @@
 import org.opensearch.security.support.ConfigConstants;
 import org.opensearch.transport.TransportRequest;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions
 /**
  * Implementation to evaluate a certificate extension with a given OID
  * and value to the same value found on the peer certificate
  *
  */
+// CS-ENFORCE-SINGLE
 public final class OIDClusterRequestEvaluator implements InterClusterRequestEvaluator {
     private final String certOid;
 
@@ -49,8 +51,10 @@ public OIDClusterRequestEvaluator(final Settings settings) {
     public boolean isInterClusterRequest(TransportRequest request, X509Certificate[] localCerts, X509Certificate[] peerCerts,
             final String principal) {
         if (localCerts != null && localCerts.length > 0 && peerCerts != null && peerCerts.length > 0) {
+            // CS-SUPPRESS-SINGLE: RegexpSingleline Java Cryptography Extension is unrelated to OpenSearch extensions
             final byte[] localValue = localCerts[0].getExtensionValue(certOid);
             final byte[] peerValue = peerCerts[0].getExtensionValue(certOid);
+            // CS-ENFORCE-SINGLE
             if (localValue != null && peerValue != null) {
                 return Arrays.equals(localValue, peerValue);
             }

src/main/java/org/opensearch/security/transport/SecurityRequestHandler.java

@@ -26,6 +26,7 @@
 
 package org.opensearch.security.transport;
 
+// CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
 import java.net.InetSocketAddress;
 import java.security.cert.X509Certificate;
 import java.util.Objects;
@@ -62,6 +63,7 @@
 import org.opensearch.transport.TransportRequestHandler;
 
 import static org.opensearch.security.OpenSearchSecurityPlugin.isActionTraceEnabled;
+// CS-ENFORCE-SINGLE
 
 public class SecurityRequestHandler<T extends TransportRequest> extends SecuritySSLRequestHandler<T> {
 
@@ -195,11 +197,13 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
             //if the incoming request is an internal:* or a shard request allow only if request was sent by a server node
             //if transport channel is not a netty channel but a direct or local channel (e.g. send via network) then allow it (regardless of beeing a internal: or shard request)
             //also allow when issued from a remote cluster for cross cluster search
+            // CS-SUPPRESS-SINGLE: RegexpSingleline Used to allow/disallow TLS connections to extensions
             if ( !HeaderHelper.isInterClusterRequest(getThreadContext())
                     && !HeaderHelper.isTrustedClusterRequest(getThreadContext())
                     && !HeaderHelper.isExtensionRequest(getThreadContext())
                     && !task.getAction().equals("internal:transport/handshake")
                     && (task.getAction().startsWith("internal:") || task.getAction().contains("["))) {
+                // CS-ENFORCE-SINGLE
 
                 auditLog.logMissingPrivileges(task.getAction(), request, task);
                 log.error("Internal or shard requests ("+task.getAction()+") not allowed from a non-server node for transport type "+transportChannel.getChannelType());
@@ -224,9 +228,11 @@ else if(!Strings.isNullOrEmpty(injectedUserHeader)) {
                 }
 
                 //network intercluster request or cross search cluster request
+                // CS-SUPPRESS-SINGLE: RegexpSingleline Used to allow/disallow TLS connections to extensions
                 if(HeaderHelper.isInterClusterRequest(getThreadContext())
                         || HeaderHelper.isTrustedClusterRequest(getThreadContext())
                         || HeaderHelper.isExtensionRequest(getThreadContext())) {
+                    // CS-ENFORCE-SINGLE
 
                     final String userHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_HEADER);
                     final String injectedRolesHeader = getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_HEADER);
@@ -328,13 +334,15 @@ protected void addAdditionalContextValues(final String action, final TransportRe
             }
         }
 
+        // CS-SUPPRESS-SINGLE: RegexpSingleline Extensions manager used to allow/disallow TLS connections to extensions
         String extensionUniqueId = getThreadContext().getHeader("extension_unique_id");
         if (extensionUniqueId != null) {
             ExtensionsManager extManager = OpenSearchSecurityPlugin.GuiceHolder.getExtensionsManager();
             if (extManager.lookupInitializedExtensionById(extensionUniqueId).isPresent()) {
                 getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST, Boolean.TRUE);
             }
         }
+        // CS-ENFORCE-SINGLE
 
         super.addAdditionalContextValues(action, request, localCerts, peerCerts, principal);
     }