From 760185eaa0c349033d8d7bcb7c4e2309e56ba232 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Wed, 7 Dec 2022 10:10:33 +0000 Subject: [PATCH] Make adding auth info to REST responses more robust (#92168) If decoding auth information stored with a background task config fails, this will no longer prevent the entire config serialization from working. For example, if an ML datafeed config document somehow contains corrupt authorization headers then it is now possible to list datafeeds. --- docs/changelog/92168.yaml | 5 +++++ .../AuthenticationContextSerializer.java | 19 ++++++++++++++----- .../core/security/xcontent/XContentUtils.java | 10 ++++++++-- .../security/xcontent/XContentUtilsTests.java | 5 +++++ 4 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 docs/changelog/92168.yaml diff --git a/docs/changelog/92168.yaml b/docs/changelog/92168.yaml new file mode 100644 index 0000000000000..fb758d44cd958 --- /dev/null +++ b/docs/changelog/92168.yaml @@ -0,0 +1,5 @@ +pr: 92168 +summary: Make adding auth info to REST responses more robust +area: Authorization +type: bug +issues: [] diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/AuthenticationContextSerializer.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/AuthenticationContextSerializer.java index b18b6091addef..c44376c45d65b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/AuthenticationContextSerializer.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/support/AuthenticationContextSerializer.java @@ -7,6 +7,8 @@ package org.elasticsearch.xpack.core.security.authc.support; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -23,6 +25,8 @@ */ public class AuthenticationContextSerializer { + private static final Logger logger = LogManager.getLogger(AuthenticationContextSerializer.class); + private final String contextKey; public AuthenticationContextSerializer() { @@ -57,11 +61,16 @@ Authentication deserializeHeaderAndPutInContext(String headerValue, ThreadContex } public static Authentication decode(String header) throws IOException { - byte[] bytes = Base64.getDecoder().decode(header); - StreamInput input = StreamInput.wrap(bytes); - Version version = Version.readVersion(input); - input.setVersion(version); - return new Authentication(input); + try { + byte[] bytes = Base64.getDecoder().decode(header); + StreamInput input = StreamInput.wrap(bytes); + Version version = Version.readVersion(input); + input.setVersion(version); + return new Authentication(input); + } catch (IOException | RuntimeException e) { + logger.warn("Failed to decode authentication [" + header + "]", e); + throw e; + } } public Authentication getAuthentication(ThreadContext context) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtils.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtils.java index ed92541f7c8ad..a81cc3b406b3d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtils.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtils.java @@ -94,8 +94,14 @@ public static void addAuthorizationInfo(final XContentBuilder builder, final Map if (authKey == null) { return; } + Subject authenticationSubject; + try { + authenticationSubject = AuthenticationContextSerializer.decode(authKey).getEffectiveSubject(); + } catch (Exception e) { + // The exception will have been logged by AuthenticationContextSerializer.decode() so don't log it again here. + return; + } builder.startObject("authorization"); - Subject authenticationSubject = AuthenticationContextSerializer.decode(authKey).getEffectiveSubject(); switch (authenticationSubject.getType()) { case USER -> builder.array(User.Fields.ROLES.getPreferredName(), authenticationSubject.getUser().roles()); case API_KEY -> { @@ -103,7 +109,7 @@ public static void addAuthorizationInfo(final XContentBuilder builder, final Map Map metadata = authenticationSubject.getMetadata(); builder.field("id", metadata.get(AuthenticationField.API_KEY_ID_KEY)); Object name = metadata.get(AuthenticationField.API_KEY_NAME_KEY); - if (name != null) { + if (name instanceof String) { builder.field("name", name); } builder.endObject(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtilsTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtilsTests.java index 3134e4e1748ef..03cb4e109a85e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtilsTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/xcontent/XContentUtilsTests.java @@ -67,6 +67,11 @@ public void testAddAuthorizationInfoWithServiceAccount() throws IOException { assertThat(json, equalTo("{\"authorization\":{\"service_account\":\"" + account + "\"}}")); } + public void testAddAuthorizationInfoWithCorruptData() throws IOException { + String json = generateJson(Map.of(AuthenticationField.AUTHENTICATION_KEY, "corrupt")); + assertThat(json, equalTo("{}")); + } + private String generateJson(Map headers) throws IOException { try (XContentBuilder builder = JsonXContent.contentBuilder()) { builder.startObject();