Merge remote-tracking branch 'es/main' into mergeSortedNumericField_3

martijnvg · martijnvg · commit b5f332bc713e · 2025-04-08T12:26:30.000+02:00
diff --git a/docs/changelog/126310.yaml b/docs/changelog/126310.yaml
@@ -0,0 +1,6 @@
+pr: 126310
+summary: Add Issuer to failed SAML Signature validation logs when available
+area: Security
+type: enhancement
+issues:
+ - 111022
diff --git a/muted-tests.yml b/muted-tests.yml
@@ -357,9 +357,6 @@ tests:
 - class: org.elasticsearch.search.basic.SearchWithRandomDisconnectsIT
   method: testSearchWithRandomDisconnects
   issue: https://github.com/elastic/elasticsearch/issues/122707
-- class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeIT
-  method: test
-  issue: https://github.com/elastic/elasticsearch/issues/126139
 - class: org.elasticsearch.snapshots.SharedClusterSnapshotRestoreIT
   method: testDeletionOfFailingToRecoverIndexShouldStopRestore
   issue: https://github.com/elastic/elasticsearch/issues/126204
diff --git a/server/src/main/java/org/elasticsearch/common/lucene/search/TopDocsAndMaxScore.java b/server/src/main/java/org/elasticsearch/common/lucene/search/TopDocsAndMaxScore.java
@@ -14,7 +14,6 @@
 /**
  * Wrapper around a {@link TopDocs} instance and the maximum score.
  */
-// TODO: Remove this class when https://github.com/elastic/elasticsearch/issues/32981 is addressed.
 public final class TopDocsAndMaxScore {
 
     public final TopDocs topDocs;
@@ -24,5 +23,4 @@ public TopDocsAndMaxScore(TopDocs topDocs, float maxScore) {
         this.topDocs = topDocs;
         this.maxScore = maxScore;
     }
-
 }
diff --git a/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java b/x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/generative/GenerativeRestTest.java
@@ -41,6 +41,7 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
         "Unbounded sort not supported yet",
         "The field names are too complex to process", // field_caps problem
         "must be \\[any type except counter types\\]", // TODO refine the generation of count()
+        "mismatched input .* expecting", // identifier generator needs to be refined, this happens when an identifier is a reserved keyword
 
         // warnings
         "Field '.*' shadowed by field at line .*",
@@ -52,10 +53,9 @@ public abstract class GenerativeRestTest extends ESRestTestCase {
         "only supports KEYWORD or TEXT values, found expression", // https://github.com/elastic/elasticsearch/issues/126017
         "token recognition error at: '``", // https://github.com/elastic/elasticsearch/issues/125870
         "Unknown column \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126026
-        "Expected \\[.*\\] but was \\[.*\\]", // https://github.com/elastic/elasticsearch/issues/126030
-        "trying to encode an unsupported data type value for TopN", // still https://github.com/elastic/elasticsearch/issues/126030 probably
-        "Block cannot be cast to", // https://github.com/elastic/elasticsearch/issues/126036
         "optimized incorrectly due to missing references", // https://github.com/elastic/elasticsearch/issues/116781
+        "No matches found for pattern", // https://github.com/elastic/elasticsearch/issues/126418
+        "JOIN left field .* is incompatible with right field", // https://github.com/elastic/elasticsearch/issues/126419
         "The incoming YAML document exceeds the limit:" // still to investigate, but it seems to be specific to the test framework
     );
 
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java
@@ -117,7 +117,7 @@ private static List<NamedExpression> combineProjections(List<? extends NamedExpr
         // collect named expressions declaration in the lower list
         AttributeMap.Builder<NamedExpression> namedExpressionsBuilder = AttributeMap.builder();
         // while also collecting the alias map for resolving the source (f1 = 1, f2 = f1, etc..)
-        AttributeMap.Builder<Expression> aliasesBuilder = AttributeMap.builder();
+        AttributeMap.Builder<Expression> aliasesBuilder = AttributeMap.builder(lower.size());
         for (NamedExpression ne : lower) {
             // record the alias
             aliasesBuilder.put(ne.toAttribute(), Alias.unwrap(ne));
@@ -128,7 +128,7 @@ private static List<NamedExpression> combineProjections(List<? extends NamedExpr
                 namedExpressionsBuilder.put(ne.toAttribute(), as.replaceChild(aliasesBuilder.build().resolve(child, child)));
             }
         }
-        List<NamedExpression> replaced = new ArrayList<>();
+        List<NamedExpression> replaced = new ArrayList<>(upper.size());
         var namedExpressions = namedExpressionsBuilder.build();
 
         // replace any matching attribute with a lower alias (if there's a match)
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/Layout.java
@@ -7,14 +7,14 @@
 
 package org.elasticsearch.xpack.esql.planner;
 
+import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.xpack.esql.core.expression.NameId;
 import org.elasticsearch.xpack.esql.core.expression.NamedExpression;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -65,7 +65,7 @@ class Builder {
         private final List<ChannelSet> channels;
 
         public Builder() {
-            channels = new ArrayList<>();
+            this(new ArrayList<>());
         }
 
         Builder(List<ChannelSet> channels) {
@@ -76,7 +76,7 @@ public Builder() {
          * Appends a new channel to the layout. The channel is mapped to one or more attribute ids.
          */
         public Builder append(ChannelSet set) {
-            if (set.nameIds.size() < 1) {
+            if (set.nameIds.isEmpty()) {
                 throw new IllegalArgumentException("Channel must be mapped to at least one id.");
             }
             channels.add(set);
@@ -104,7 +104,11 @@ public Builder append(Collection<? extends NamedExpression> attributes) {
          * Build a new {@link Layout}.
          */
         public Layout build() {
-            Map<NameId, ChannelAndType> layout = new HashMap<>();
+            int size = 0;
+            for (ChannelSet channel : channels) {
+                size += channel.nameIds.size();
+            }
+            Map<NameId, ChannelAndType> layout = Maps.newHashMapWithExpectedSize(size);
             int numberOfChannels = 0;
             for (ChannelSet set : channels) {
                 int channel = numberOfChannels++;
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/LocalExecutionPlanner.java
@@ -11,6 +11,7 @@
 import org.elasticsearch.common.lucene.BytesRefs;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.BigArrays;
+import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.compute.Describable;
 import org.elasticsearch.compute.aggregation.AggregatorMode;
 import org.elasticsearch.compute.data.Block;
@@ -117,9 +118,6 @@
 import org.elasticsearch.xpack.esql.session.Configuration;
 
 import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -501,8 +499,8 @@ private PhysicalOperation planGrok(GrokExec grok, LocalExecutionPlannerContext c
         Layout.Builder layoutBuilder = source.layout.builder();
         List<Attribute> extractedFields = grok.extractedFields();
         layoutBuilder.append(extractedFields);
-        Map<String, Integer> fieldToPos = new HashMap<>(extractedFields.size());
-        Map<String, ElementType> fieldToType = new HashMap<>(extractedFields.size());
+        Map<String, Integer> fieldToPos = Maps.newHashMapWithExpectedSize(extractedFields.size());
+        Map<String, ElementType> fieldToType = Maps.newHashMapWithExpectedSize(extractedFields.size());
         ElementType[] types = new ElementType[extractedFields.size()];
         List<Attribute> extractedFieldsFromPattern = grok.pattern().extractedFields();
         for (int i = 0; i < extractedFields.size(); i++) {
@@ -559,7 +557,8 @@ private PhysicalOperation planEnrich(EnrichExec enrich, LocalExecutionPlannerCon
     private PhysicalOperation planRerank(RerankExec rerank, LocalExecutionPlannerContext context) {
         PhysicalOperation source = plan(rerank.child(), context);
 
-        Map<ColumnInfoImpl, EvalOperator.ExpressionEvaluator.Factory> rerankFieldsEvaluatorSuppliers = new LinkedHashMap<>();
+        Map<ColumnInfoImpl, EvalOperator.ExpressionEvaluator.Factory> rerankFieldsEvaluatorSuppliers = Maps
+            .newLinkedHashMapWithExpectedSize(rerank.rerankFields().size());
 
         for (var rerankField : rerank.rerankFields()) {
             rerankFieldsEvaluatorSuppliers.put(
@@ -734,31 +733,13 @@ private PhysicalOperation planProject(ProjectExec project, LocalExecutionPlanner
         List<Integer> projectionList = new ArrayList<>(projections.size());
 
         Layout.Builder layout = new Layout.Builder();
-        Map<Integer, Layout.ChannelSet> inputChannelToOutputIds = new HashMap<>();
-        for (int index = 0, size = projections.size(); index < size; index++) {
-            NamedExpression ne = projections.get(index);
-
-            NameId inputId = null;
-            if (ne instanceof Alias a) {
-                inputId = ((NamedExpression) a.child()).id();
-            } else {
-                inputId = ne.id();
-            }
+        for (NamedExpression ne : projections) {
+            NameId inputId = ne instanceof Alias a ? ((NamedExpression) a.child()).id() : ne.id();
             Layout.ChannelAndType input = source.layout.get(inputId);
             if (input == null) {
                 throw new IllegalStateException("can't find input for [" + ne + "]");
             }
-            Layout.ChannelSet channelSet = inputChannelToOutputIds.get(input.channel());
-            if (channelSet == null) {
-                channelSet = new Layout.ChannelSet(new HashSet<>(), input.type());
-                channelSet.nameIds().add(ne.id());
-                layout.append(channelSet);
-            } else {
-                channelSet.nameIds().add(ne.id());
-            }
-            if (channelSet.type() != input.type()) {
-                throw new IllegalArgumentException("type mismatch for aliases");
-            }
+            layout.append(ne);
             projectionList.add(input.channel());
         }
 
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticator.java
@@ -93,7 +93,7 @@ private SamlAttributes authenticateResponse(Element element, Collection<String>
         }
         final boolean requireSignedAssertions;
         if (response.isSigned()) {
-            validateSignature(response.getSignature());
+            validateSignature(response.getSignature(), response.getIssuer());
             requireSignedAssertions = false;
         } else {
             requireSignedAssertions = true;
@@ -199,7 +199,7 @@ private List<Attribute> processAssertion(Assertion assertion, boolean requireSig
         }
         // Do not further process unsigned Assertions
         if (assertion.isSigned()) {
-            validateSignature(assertion.getSignature());
+            validateSignature(assertion.getSignature(), assertion.getIssuer());
         } else if (requireSignature) {
             throw samlException("Assertion [{}] is not signed, but a signature is required", assertion.getElementQName());
         }
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutRequestHandler.java
@@ -73,7 +73,7 @@ private Result parseLogout(LogoutRequest logoutRequest, boolean requireSignature
                 throw samlException("Logout request is not signed");
             }
         } else {
-            validateSignature(signature);
+            validateSignature(signature, logoutRequest.getIssuer());
         }
 
         checkIssuer(logoutRequest.getIssuer(), logoutRequest);
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlLogoutResponseHandler.java
@@ -45,7 +45,7 @@ public void handle(boolean httpRedirect, String payload, Collection<String> allo
                 if (logoutResponse.getSignature() == null) {
                     throw samlException("LogoutResponse is not signed, but is required for HTTP-Post binding");
                 }
-                validateSignature(logoutResponse.getSignature());
+                validateSignature(logoutResponse.getSignature(), logoutResponse.getIssuer());
             }
             checkInResponseTo(logoutResponse, allowedSamlRequestIds);
             checkStatus(logoutResponse.getStatus());
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlObjectHandler.java
@@ -95,6 +95,8 @@ public class SamlObjectHandler {
         }
     });
 
+    private static final int ISSUER_VALUE_MAX_LENGTH = 512;
+
     protected final Logger logger = LogManager.getLogger(getClass());
 
     @Nullable
@@ -161,13 +163,13 @@ protected static String describe(Collection<X509Credential> credentials) {
         return credentials.stream().map(credential -> describe(credential.getEntityCertificate())).collect(Collectors.joining(","));
     }
 
-    void validateSignature(Signature signature) {
+    void validateSignature(Signature signature, @Nullable Issuer issuer) {
         final String signatureText = text(signature, 32);
         SAMLSignatureProfileValidator profileValidator = new SAMLSignatureProfileValidator();
         try {
             profileValidator.validate(signature);
         } catch (SignatureException e) {
-            throw samlSignatureException(idp.getSigningCredentials(), signatureText, e);
+            throw samlSignatureException(issuer, idp.getSigningCredentials(), signatureText, e);
         }
 
         checkIdpSignature(credential -> {
@@ -200,21 +202,21 @@ void validateSignature(Signature signature) {
                         );
                         return true;
                     } catch (PrivilegedActionException e) {
-                        logger.warn("SecurityException while attempting to validate SAML signature", e);
+                        logger.warn("SecurityException while attempting to validate SAML signature." + describeIssuer(issuer), e);
                         return false;
                     }
                 });
             } catch (PrivilegedActionException e) {
                 throw new SecurityException("SecurityException while attempting to validate SAML signature", e);
             }
-        }, signatureText);
+        }, signatureText, issuer);
     }
 
     /**
      * Tests whether the provided function returns {@code true} for any of the IdP's signing credentials.
-     * @throws ElasticsearchSecurityException - A SAML exception if not matching credential is found.
+     * @throws ElasticsearchSecurityException - A SAML exception if no matching credential is found.
      */
-    protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception> check, String signatureText) {
+    protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception> check, String signatureText, @Nullable Issuer issuer) {
         final Predicate<Credential> predicate = credential -> {
             try {
                 return check.apply(credential);
@@ -231,35 +233,52 @@ protected void checkIdpSignature(CheckedFunction<Credential, Boolean, Exception>
                 logger.trace("SAML Signature failure caused by", e);
                 return false;
             } catch (Exception e) {
-                logger.warn("Exception while attempting to validate SAML Signature", e);
+                logger.warn("Exception while attempting to validate SAML Signature." + describeIssuer(issuer), e);
                 return false;
             }
         };
         final List<Credential> credentials = idp.getSigningCredentials();
         if (credentials.stream().anyMatch(predicate) == false) {
-            throw samlSignatureException(credentials, signatureText);
+            throw samlSignatureException(issuer, credentials, signatureText);
         }
     }
 
     /**
      * Constructs a SAML specific exception with a consistent message regarding SAML Signature validation failures
      */
-    private ElasticsearchSecurityException samlSignatureException(List<Credential> credentials, String signature, Exception cause) {
+    private ElasticsearchSecurityException samlSignatureException(
+        @Nullable Issuer issuer,
+        List<Credential> credentials,
+        String signature,
+        Exception cause
+    ) {
         logger.warn(
-            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML"
-                + "metadata file/URL for this Identity Provider"
+            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML "
+                + "metadata file/URL for this Identity Provider.{}",
+            describeIssuer(issuer)
         );
         final String msg = "SAML Signature [{}] could not be validated against [{}]";
-        return samlException(msg, cause, signature, describeCredentials(credentials));
+        if (cause != null) {
+            return samlException(msg, cause, signature, describeCredentials(credentials));
+        } else {
+            return samlException(msg, signature, describeCredentials(credentials));
+        }
     }
 
-    private ElasticsearchSecurityException samlSignatureException(List<Credential> credentials, String signature) {
-        logger.warn(
-            "The XML Signature of this SAML message cannot be validated. Please verify that the saml realm uses the correct SAML"
-                + "metadata file/URL for this Identity Provider"
-        );
-        final String msg = "SAML Signature [{}] could not be validated against [{}]";
-        return samlException(msg, signature, describeCredentials(credentials));
+    private ElasticsearchSecurityException samlSignatureException(Issuer issuer, List<Credential> credentials, String signature) {
+        return samlSignatureException(issuer, credentials, signature, null);
+    }
+
+    // package private for testing
+    static String describeIssuer(@Nullable Issuer issuer) {
+        if (issuer == null || issuer.getValue() == null) {
+            return "";
+        }
+        final String msg = " The issuer included in the SAML message was [%s]";
+        if (issuer.getValue().length() > ISSUER_VALUE_MAX_LENGTH) {
+            return Strings.format(msg + "...", Strings.cleanTruncate(issuer.getValue(), ISSUER_VALUE_MAX_LENGTH));
+        }
+        return Strings.format(msg, issuer.getValue());
     }
 
     private static String describeCredentials(List<Credential> credentials) {
@@ -423,7 +442,7 @@ private void validateSignature(String inputString, String signatureAlgorithm, St
                 );
                 return false;
             }
-        }, signatureText);
+        }, signatureText, null);
     }
 
     protected byte[] decodeBase64(String content) {
diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/saml/SamlAuthenticatorTests.java

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,6 @@`
`14`	`14`	`/**`
`15`	`15`	`* Wrapper around a {@link TopDocs} instance and the maximum score.`
`16`	`16`	`*/`
`17`		`-// TODO: Remove this class when https://github.com/elastic/elasticsearch/issues/32981 is addressed.`
`18`	`17`	`public final class TopDocsAndMaxScore {`
`19`	`18`
`20`	`19`	`public final TopDocs topDocs;`
`@@ -24,5 +23,4 @@ public TopDocsAndMaxScore(TopDocs topDocs, float maxScore) {`
`24`	`23`	`this.topDocs = topDocs;`
`25`	`24`	`this.maxScore = maxScore;`
`26`	`25`	`}`
`27`		`-`
`28`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -93,7 +93,7 @@ private SamlAttributes authenticateResponse(Element element, Collection<String>`
`93`	`93`	`}`
`94`	`94`	`final boolean requireSignedAssertions;`
`95`	`95`	`if (response.isSigned()) {`
`96`		`- validateSignature(response.getSignature());`
	`96`	`+ validateSignature(response.getSignature(), response.getIssuer());`
`97`	`97`	`requireSignedAssertions = false;`
`98`	`98`	`} else {`
`99`	`99`	`requireSignedAssertions = true;`
`@@ -199,7 +199,7 @@ private List<Attribute> processAssertion(Assertion assertion, boolean requireSig`
`199`	`199`	`}`
`200`	`200`	`// Do not further process unsigned Assertions`
`201`	`201`	`if (assertion.isSigned()) {`
`202`		`- validateSignature(assertion.getSignature());`
	`202`	`+ validateSignature(assertion.getSignature(), assertion.getIssuer());`
`203`	`203`	`} else if (requireSignature) {`
`204`	`204`	`throw samlException("Assertion [{}] is not signed, but a signature is required", assertion.getElementQName());`
`205`	`205`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ private Result parseLogout(LogoutRequest logoutRequest, boolean requireSignature`
`73`	`73`	`throw samlException("Logout request is not signed");`
`74`	`74`	`}`
`75`	`75`	`} else {`
`76`		`- validateSignature(signature);`
	`76`	`+ validateSignature(signature, logoutRequest.getIssuer());`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`checkIssuer(logoutRequest.getIssuer(), logoutRequest);`
Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ public void handle(boolean httpRedirect, String payload, Collection<String> allo`
`45`	`45`	`if (logoutResponse.getSignature() == null) {`
`46`	`46`	`throw samlException("LogoutResponse is not signed, but is required for HTTP-Post binding");`
`47`	`47`	`}`
`48`		`- validateSignature(logoutResponse.getSignature());`
	`48`	`+ validateSignature(logoutResponse.getSignature(), logoutResponse.getIssuer());`
`49`	`49`	`}`
`50`	`50`	`checkInResponseTo(logoutResponse, allowedSamlRequestIds);`
`51`	`51`	`checkStatus(logoutResponse.getStatus());`