Skip to content

Add X-Elastic-Product header on all http responses #73434

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

Merged
merged 7 commits into from
Jun 1, 2021

Conversation

jbaiera
Copy link
Member

@jbaiera jbaiera commented May 26, 2021

This PR adds a header to all Elasticsearch responses that confirms the type of service operating on the other end of the connection. This will aid in validating client-server connections to ensure protocol compatibility between requestors and the server.

The header returns from all requests (except unauthenticated ones) and its format is as follows:

X-elastic-product: Elasticsearch

resolves #73424

edit: Updated the default casing of the header, updated that it is not returned on unauthenticated requests.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

@jbaiera jbaiera changed the title Add product header to all responses Add X-Elastic-Product header on all http responses May 26, 2021
@@ -57,6 +57,8 @@

private static final Logger logger = LogManager.getLogger(RestController.class);
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestController.class);
private static final String ELASTIC_PRODUCT_HTTP_HEADER = "X-Elastic-Product";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be lowercase here? I noticed that many of the HTTP Headers we operate with are not consistently cased.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the HTTP spec, header names are case-insensitive. We use lowercase for x-elastic-product-origin but it shouldn't* matter.

* famous last words

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending green CI. One non-blocking comment below for you to consider or decline as you prefer.

@@ -57,6 +57,8 @@

private static final Logger logger = LogManager.getLogger(RestController.class);
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestController.class);
private static final String ELASTIC_PRODUCT_HTTP_HEADER = "X-Elastic-Product";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the HTTP spec, header names are case-insensitive. We use lowercase for x-elastic-product-origin but it shouldn't* matter.

* famous last words

@@ -142,6 +143,9 @@ public MethodHandlers next() {
assertEquals("true", threadContext.getHeader("header.1"));
assertEquals("true", threadContext.getHeader("header.2"));
assertNull(threadContext.getHeader("header.3"));
List<String> expectedProductResponseHeader = new ArrayList<>();
expectedProductResponseHeader.add("Elasticsearch");
assertEquals(expectedProductResponseHeader, threadContext.getResponseHeaders().getOrDefault("X-Elastic-Product", null));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make the ELASTIC_PRODUCT_HTTP_HEADER variable package private so it can be referenced here instead of the string literal?

@jbaiera
Copy link
Member Author

jbaiera commented May 27, 2021

Failing test was because security expects only 3 headers to be output when a request is not authenticated: Content-Type, Content-Encoding, and Content-Length. We should probably not output the product header on an unauthenticated request - else we leak that this Elasticsearch is behind this port.

@jbaiera
Copy link
Member Author

jbaiera commented Jun 1, 2021

@elasticmachine update branch

@jbaiera
Copy link
Member Author

jbaiera commented Jun 1, 2021

@elasticmachine update branch

@jbaiera jbaiera merged commit c13384c into elastic:master Jun 1, 2021
@jbaiera jbaiera deleted the add-product-response-header branch June 1, 2021 20:46
@swallez
Copy link
Member

swallez commented Jun 8, 2021

@jbaiera this should be backported to 7.x as the header should be present starting with 7.14. Shall I do it?

@jbaiera
Copy link
Member Author

jbaiera commented Jun 8, 2021

@swallez thanks for the reminder, backport is in flight

jbaiera added a commit that referenced this pull request Jun 8, 2021
Backports the following commits to 7.x:

Add X-Elastic-Product header on all http responses
swallez added a commit to swallez/elasticsearch that referenced this pull request Jun 16, 2021
Follow-up to elastic#73434

Ensures that High Level Rest Client is running against a verified
Elasticsearch. When the first request is send on HLRC, a request to the
info endpoint is made first to verify the product identification and
version.
swallez added a commit that referenced this pull request Jun 17, 2021
Follow-up to #73434

Ensures that High Level Rest Client is running against a verified
Elasticsearch. When the first request is send on HLRC, a request to the
info endpoint is made first to verify the product identification and
version.
swallez added a commit that referenced this pull request Jun 18, 2021
Follow-up to #73434

Ensures that High Level Rest Client is running against a verified
Elasticsearch. When the first request is send on HLRC, a request to the
info endpoint is made first to verify the product identification and
version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add X-Elastic-Product header on all http responses
5 participants