From f197bbb9dc6fad5916c6734c7bebf2c4e9699255 Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Tue, 17 Dec 2024 13:08:01 +0100 Subject: [PATCH 1/7] Code cleanup (#21177) Co-authored-by: Matthias Oesterheld <33032967+moesterheld@users.noreply.github.com> --- .../graylog/datanode/configuration/DatanodeConfiguration.java | 2 -- .../datanode/configuration/DatanodeConfigurationProvider.java | 1 - .../beans/impl/OpensearchCommonConfigurationBean.java | 4 ++-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfiguration.java b/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfiguration.java index 4a2ef38e0605..3b78df4747ea 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfiguration.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfiguration.java @@ -16,7 +16,6 @@ */ package org.graylog.datanode.configuration; -import org.graylog.datanode.OpensearchDistribution; import org.graylog2.security.IndexerJwtAuthTokenProvider; /** @@ -27,7 +26,6 @@ public record DatanodeConfiguration( OpensearchDistributionProvider opensearchDistributionProvider, DatanodeDirectories datanodeDirectories, int processLogsBufferSize, - String opensearchHeap, IndexerJwtAuthTokenProvider indexerJwtAuthTokenProvider ) { } diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfigurationProvider.java b/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfigurationProvider.java index ae4f7a606179..3cfc53f0d411 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfigurationProvider.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/DatanodeConfigurationProvider.java @@ -40,7 +40,6 @@ public DatanodeConfigurationProvider( opensearchDistributionProvider, DatanodeDirectories.fromConfiguration(localConfiguration, nodeId), localConfiguration.getProcessLogsBufferSize(), - localConfiguration.getOpensearchHeap(), jwtTokenProvider ); } diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchCommonConfigurationBean.java b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchCommonConfigurationBean.java index 6705e69244fa..a51bb0a1112d 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchCommonConfigurationBean.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchCommonConfigurationBean.java @@ -43,8 +43,8 @@ public DatanodeConfigurationPart buildConfigurationPart(OpensearchConfigurationP return DatanodeConfigurationPart.builder() .properties(commonOpensearchConfig(buildParams)) .nodeRoles(localConfiguration.getNodeRoles()) - .javaOpt("-Xms%s".formatted(datanodeConfiguration.opensearchHeap())) - .javaOpt("-Xmx%s".formatted(datanodeConfiguration.opensearchHeap())) + .javaOpt("-Xms%s".formatted(localConfiguration.getOpensearchHeap())) + .javaOpt("-Xmx%s".formatted(localConfiguration.getOpensearchHeap())) .javaOpt("-Dopensearch.transport.cname_in_publish_address=true") .build(); } From b9a8df381e05e61c2b3315c11920cedf6ddfc62d Mon Sep 17 00:00:00 2001 From: Anton Ebel Date: Tue, 17 Dec 2024 15:01:17 +0100 Subject: [PATCH 2/7] add javadoc for decodeSafe method (#21207) --- .../main/java/org/graylog2/plugin/inputs/codecs/Codec.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/graylog2-server/src/main/java/org/graylog2/plugin/inputs/codecs/Codec.java b/graylog2-server/src/main/java/org/graylog2/plugin/inputs/codecs/Codec.java index b99f13a94cae..0f2591b35228 100644 --- a/graylog2-server/src/main/java/org/graylog2/plugin/inputs/codecs/Codec.java +++ b/graylog2-server/src/main/java/org/graylog2/plugin/inputs/codecs/Codec.java @@ -28,6 +28,12 @@ public interface Codec { + /** + * @param rawMessage + * @return an empty Optional if RawMessage contains payload data that should not be decoded. + * Otherwise, an Optional with a Message that can be processed further. + * @throws org.graylog2.plugin.inputs.failure.InputProcessingException if an error occurs during decoding. + */ default Optional decodeSafe(@Nonnull RawMessage rawMessage) { return Optional.ofNullable(decode(rawMessage)); } From f4a129171ad40ecaa63bd89768d3e7608f2c9950 Mon Sep 17 00:00:00 2001 From: Linus Pahl <46300478+linuspahl@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:25:43 +0100 Subject: [PATCH 3/7] Create `ClipboardIconButton`. (#21135) * Create `ClipboardIconButton`. * Add tests. * Define fixed icon name. * Use regular icon type. * Update test * Fix linter hints --- .../common/ClipboardButton.test.tsx | 38 +++++++++++ .../src/components/common/ClipboardButton.tsx | 68 ++++++------------- .../components/common/ClipboardContainer.tsx | 63 +++++++++++++++++ .../common/ClipboardIconButton.test.tsx | 38 +++++++++++ .../components/common/ClipboardIconButton.tsx | 51 ++++++++++++++ .../src/components/common/index.tsx | 1 + 6 files changed, 212 insertions(+), 47 deletions(-) create mode 100644 graylog2-web-interface/src/components/common/ClipboardButton.test.tsx create mode 100644 graylog2-web-interface/src/components/common/ClipboardContainer.tsx create mode 100644 graylog2-web-interface/src/components/common/ClipboardIconButton.test.tsx create mode 100644 graylog2-web-interface/src/components/common/ClipboardIconButton.tsx diff --git a/graylog2-web-interface/src/components/common/ClipboardButton.test.tsx b/graylog2-web-interface/src/components/common/ClipboardButton.test.tsx new file mode 100644 index 000000000000..fa1d62cc31e4 --- /dev/null +++ b/graylog2-web-interface/src/components/common/ClipboardButton.test.tsx @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +import React from 'react'; +import { render, screen } from 'wrappedTestingLibrary'; +import userEvent from '@testing-library/user-event'; + +import copyToClipboard from 'util/copyToClipboard'; + +import ClipboardButton from './ClipboardButton'; + +jest.mock('util/copyToClipboard', () => jest.fn(() => Promise.resolve())); + +describe('ClipboardButton', () => { + it('should copy provided text to clipboard', async () => { + const text = 'Text to copy'; + render(); + + userEvent.click(await screen.findByRole('button', { name: /click here to copy/i })); + + expect(copyToClipboard).toHaveBeenCalledWith(text); + + await screen.findByText('Copied!'); + }); +}); diff --git a/graylog2-web-interface/src/components/common/ClipboardButton.tsx b/graylog2-web-interface/src/components/common/ClipboardButton.tsx index 6c184726247d..cbaa7609d04a 100644 --- a/graylog2-web-interface/src/components/common/ClipboardButton.tsx +++ b/graylog2-web-interface/src/components/common/ClipboardButton.tsx @@ -15,14 +15,11 @@ * . */ import * as React from 'react'; -import { useCallback, useState } from 'react'; -import { useTimeout } from '@mantine/hooks'; import { Button } from 'components/bootstrap'; import type { BsSize } from 'components/bootstrap/types'; import type { StyleProps } from 'components/bootstrap/Button'; -import copyToClipboard from 'util/copyToClipboard'; -import Tooltip from 'components/common/Tooltip'; +import ClipboardContainer from 'components/common/ClipboardContainer'; /** * Component that renders a button to copy some text in the clipboard when pressed. @@ -40,48 +37,25 @@ type Props = { title: React.ReactNode, } -type Args = { - copied: boolean, - copy: () => void, -} -type CopyButtonProps = { - value: string, - timeout: number, - children: (args: Args) => React.ReactElement, -}; - -const CopyButton = ({ children, value, timeout }: CopyButtonProps) => { - const [copied, setCopied] = useState(false); - const { start } = useTimeout(() => setCopied(false), timeout); - const copy = useCallback(() => copyToClipboard(value).then(() => { setCopied(true); start(); }), [start, value]); - - return children({ copied, copy }); -}; - -const ClipboardButton = ({ bsSize, bsStyle, buttonTitle, className, disabled, onSuccess, text, title }: Props) => { - const button = (copy: () => void) => ( - - ); - - return ( - - {({ copied, copy }) => (copied ? ( - - {button(copy)} - - ) : button(copy))} - - ); -}; +const ClipboardButton = ({ + bsSize = undefined, bsStyle = undefined, buttonTitle = undefined, className = undefined, + disabled = undefined, onSuccess = undefined, text, title, +}: Props) => ( + + {({ copy }) => ( + + )} + +); export default ClipboardButton; diff --git a/graylog2-web-interface/src/components/common/ClipboardContainer.tsx b/graylog2-web-interface/src/components/common/ClipboardContainer.tsx new file mode 100644 index 000000000000..67c08cec4f61 --- /dev/null +++ b/graylog2-web-interface/src/components/common/ClipboardContainer.tsx @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +import * as React from 'react'; +import { useCallback, useState } from 'react'; +import { useTimeout } from '@mantine/hooks'; + +import copyToClipboard from 'util/copyToClipboard'; +import Tooltip from 'components/common/Tooltip'; + +/** + * This component can be used as a wrapper for other components. When users click on the children, + * the provided text will be copied to the clipboard, and they see a tooltip for visual feedback. + */ + +type Props = { + children: (props: { copy: () => void }) => JSX.Element, + text: string, +} + +type Args = { + copied: boolean, + copy: () => void, +} + +type CopyProps = { + value: string, + timeout: number, + children: (args: Args) => React.ReactElement, +}; + +const Copy = ({ children, value, timeout }: CopyProps) => { + const [copied, setCopied] = useState(false); + const { start } = useTimeout(() => setCopied(false), timeout); + const copy = useCallback(() => copyToClipboard(value).then(() => { setCopied(true); start(); }), [start, value]); + + return children({ copied, copy }); +}; + +const ClipboardContainer = ({ children, text }: Props) => ( + + {({ copied, copy }) => (copied ? ( + + {children({ copy })} + + ) : children({ copy }))} + +); + +export default ClipboardContainer; diff --git a/graylog2-web-interface/src/components/common/ClipboardIconButton.test.tsx b/graylog2-web-interface/src/components/common/ClipboardIconButton.test.tsx new file mode 100644 index 000000000000..f340b104a3b5 --- /dev/null +++ b/graylog2-web-interface/src/components/common/ClipboardIconButton.test.tsx @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +import React from 'react'; +import { render, screen } from 'wrappedTestingLibrary'; +import userEvent from '@testing-library/user-event'; + +import copyToClipboard from 'util/copyToClipboard'; + +import ClipboardIconButton from './ClipboardIconButton'; + +jest.mock('util/copyToClipboard', () => jest.fn(() => Promise.resolve())); + +describe('ClipboardIconButton', () => { + it('should copy provided text to clipboard', async () => { + const text = 'Text to copy'; + render(); + + userEvent.click(await screen.findByRole('button', { name: /click here to copy/i })); + + expect(copyToClipboard).toHaveBeenCalledWith(text); + + await screen.findByText('Copied!'); + }); +}); diff --git a/graylog2-web-interface/src/components/common/ClipboardIconButton.tsx b/graylog2-web-interface/src/components/common/ClipboardIconButton.tsx new file mode 100644 index 000000000000..25992835bfa4 --- /dev/null +++ b/graylog2-web-interface/src/components/common/ClipboardIconButton.tsx @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2020 Graylog, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Server Side Public License, version 1, + * as published by MongoDB, Inc. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Server Side Public License for more details. + * + * You should have received a copy of the Server Side Public License + * along with this program. If not, see + * . + */ +import * as React from 'react'; + +import ClipboardContainer from 'components/common/ClipboardContainer'; +import { IconButton } from 'components/common'; + +/** + * Component that renders an icon button to copy some text in the clipboard when pressed. + * The text to be copied can be given in the `text` prop, or in an external element through a CSS selector in the `target` prop. + */ + +type Props = { + buttonTitle?: string, + className?: string, + disabled?: boolean, + onSuccess?: () => void, + text: string, +} + +const ClipboardIconButton = ({ buttonTitle = undefined, className = undefined, disabled = undefined, onSuccess = undefined, text }: Props) => ( + + {({ copy }) => ( + { + copy(); + onSuccess?.(); + }} /> + )} + +); + +export default ClipboardIconButton; diff --git a/graylog2-web-interface/src/components/common/index.tsx b/graylog2-web-interface/src/components/common/index.tsx index dcb875a56952..748f36f5c8ad 100644 --- a/graylog2-web-interface/src/components/common/index.tsx +++ b/graylog2-web-interface/src/components/common/index.tsx @@ -29,6 +29,7 @@ export { default as Card } from './Card'; export { default as Center } from './Center'; export { default as Carousel } from './Carousel'; export { default as ClipboardButton } from './ClipboardButton'; +export { default as ClipboardIconButton } from './ClipboardIconButton'; export { default as ColorPicker } from './ColorPicker'; export { default as ColorPickerPopover } from './ColorPickerPopover'; export { default as ConfirmDialog } from './ConfirmDialog'; From b5d905c6f068638b111f13697795879ffb0ef401 Mon Sep 17 00:00:00 2001 From: Linus Pahl <46300478+linuspahl@users.noreply.github.com> Date: Tue, 17 Dec 2024 19:51:09 +0100 Subject: [PATCH 4/7] Enable defining required permissions for `navigation` web interface plugins. (#21205) * Enable defining permissions for `navigation` web interface plugins. * Adding changelog. --- changelog/unreleased/pr-21205.toml | 8 ++++++++ .../src/@types/graylog-web-plugin/index.d.ts | 1 + 2 files changed, 9 insertions(+) create mode 100644 changelog/unreleased/pr-21205.toml diff --git a/changelog/unreleased/pr-21205.toml b/changelog/unreleased/pr-21205.toml new file mode 100644 index 000000000000..44cd47bce32c --- /dev/null +++ b/changelog/unreleased/pr-21205.toml @@ -0,0 +1,8 @@ +type = "a" +message = "Enable defining required permissions for navigation web interface plugin." + +pulls = ["21205"] +details.user = """ +Before it was only possible to define required permissions for a navigation dropdown item. +""" + diff --git a/graylog2-web-interface/src/@types/graylog-web-plugin/index.d.ts b/graylog2-web-interface/src/@types/graylog-web-plugin/index.d.ts index dd5da4d02769..07551e8da009 100644 --- a/graylog2-web-interface/src/@types/graylog-web-plugin/index.d.ts +++ b/graylog2-web-interface/src/@types/graylog-web-plugin/index.d.ts @@ -56,6 +56,7 @@ type PluginNavigation = { perspective?: string; BadgeComponent?: React.ComponentType<{ text: string }>; position?: 'last' | undefined, + permissions?: string | Array, useIsValidLicense?: () => boolean, } & (PluginNavigationLink | PluginNavigationDropdown) From 7845ab99117250085170ffede89de0e8ae84de7f Mon Sep 17 00:00:00 2001 From: Florian Petersen <188503754+fpetersen-gl@users.noreply.github.com> Date: Wed, 18 Dec 2024 08:21:37 +0100 Subject: [PATCH 5/7] Improve performance for working with indices (#21195) * Issue #18563: Don't iterate over all IndexSets for a given indexName, instead use `getForIndex(String)` directly. Also converted some loops to streaming. * Add changelog * Review comments * Review comments part two --- changelog/unreleased/issue-18563.toml | 5 + .../indexer/MongoIndexSetRegistry.java | 83 ++++++---------- .../indexer/MongoIndexSetRegistryTest.java | 97 +++++++++++++++++++ 3 files changed, 130 insertions(+), 55 deletions(-) create mode 100644 changelog/unreleased/issue-18563.toml diff --git a/changelog/unreleased/issue-18563.toml b/changelog/unreleased/issue-18563.toml new file mode 100644 index 000000000000..7298d46d501d --- /dev/null +++ b/changelog/unreleased/issue-18563.toml @@ -0,0 +1,5 @@ +type = "fixed" +message = "Improve performance of close and delete actions when working with a lot of index sets." + +issues = ["18563"] +pulls = ["21195"] diff --git a/graylog2-server/src/main/java/org/graylog2/indexer/MongoIndexSetRegistry.java b/graylog2-server/src/main/java/org/graylog2/indexer/MongoIndexSetRegistry.java index f601a0aaa717..14a96ed5a560 100644 --- a/graylog2-server/src/main/java/org/graylog2/indexer/MongoIndexSetRegistry.java +++ b/graylog2-server/src/main/java/org/graylog2/indexer/MongoIndexSetRegistry.java @@ -22,15 +22,15 @@ import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; +import jakarta.annotation.Nonnull; +import jakarta.inject.Inject; +import jakarta.inject.Singleton; import org.graylog2.indexer.indexset.IndexSetConfig; import org.graylog2.indexer.indexset.IndexSetService; import org.graylog2.indexer.indexset.events.IndexSetCreatedEvent; import org.graylog2.indexer.indexset.events.IndexSetDeletedEvent; import org.graylog2.indexer.indices.TooManyAliasesException; -import jakarta.inject.Inject; -import jakarta.inject.Singleton; - import java.util.Collection; import java.util.Collections; import java.util.Iterator; @@ -42,6 +42,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import static java.util.Objects.requireNonNull; @@ -52,7 +53,7 @@ public class MongoIndexSetRegistry implements IndexSetRegistry { static class IndexSetsCache { private final IndexSetService indexSetService; - private AtomicReference>> indexSetConfigs; + private final AtomicReference>> indexSetConfigs; @Inject IndexSetsCache(IndexSetService indexSetService, @@ -143,12 +144,9 @@ public Set getForIndices(Collection indices) { @Override public Set getFromIndexConfig(Collection indexSetConfigs) { - final ImmutableSet.Builder mongoIndexSets = ImmutableSet.builder(); - for (IndexSetConfig config : indexSetConfigs) { - final MongoIndexSet mongoIndexSet = mongoIndexSetFactory.create(config); - mongoIndexSets.add(mongoIndexSet); - } - return ImmutableSet.copyOf(mongoIndexSets.build()); + return indexSetConfigs.stream() + .map(mongoIndexSetFactory::create) + .collect(Collectors.toUnmodifiableSet()); } @Override @@ -158,13 +156,9 @@ public IndexSet getDefault() { @Override public String[] getManagedIndices() { - final ImmutableSet.Builder indexNamesBuilder = ImmutableSet.builder(); - for (MongoIndexSet indexSet : findAllMongoIndexSets()) { - indexNamesBuilder.add(indexSet.getManagedIndices()); - } - - final ImmutableSet indexNames = indexNamesBuilder.build(); - return indexNames.toArray(new String[0]); + return findAllMongoIndexSets().stream() + .flatMap(indexSet -> Stream.of(indexSet.getManagedIndices())) + .toArray(String[]::new); } @Override @@ -180,38 +174,25 @@ public Map isManagedIndex(Collection indices) { } private boolean isManagedIndex(Collection indexSets, String index) { - for (IndexSet indexSet : indexSets) { - if (indexSet.isManagedIndex(index)) { - return true; - } - } - return false; + return indexSets.stream() + .anyMatch(indexSet -> indexSet.isManagedIndex(index)); + } + + private String[] doWithWritableIndices(Function fn) { + return findAllMongoIndexSets().stream() + .filter(indexSet -> indexSet.getConfig().isWritable()) + .map(fn) + .toArray(String[]::new); } @Override public String[] getIndexWildcards() { - final ImmutableSet.Builder wildcardsBuilder = ImmutableSet.builder(); - for (MongoIndexSet indexSet : findAllMongoIndexSets()) { - if (indexSet.getConfig().isWritable()) { - wildcardsBuilder.add(indexSet.getIndexWildcard()); - } - } - - final ImmutableSet wildcards = wildcardsBuilder.build(); - return wildcards.toArray(new String[0]); + return doWithWritableIndices(MongoIndexSet::getIndexWildcard); } @Override public String[] getWriteIndexAliases() { - final ImmutableSet.Builder indexNamesBuilder = ImmutableSet.builder(); - for (MongoIndexSet indexSet : findAllMongoIndexSets()) { - if (indexSet.getConfig().isWritable()) { - indexNamesBuilder.add(indexSet.getWriteIndexAlias()); - } - } - - final ImmutableSet indexNames = indexNamesBuilder.build(); - return indexNames.toArray(new String[0]); + return doWithWritableIndices(MongoIndexSet::getWriteIndexAlias); } @Override @@ -223,26 +204,18 @@ public boolean isUp() { @Override public boolean isCurrentWriteIndexAlias(String indexName) { - for (MongoIndexSet indexSet : findAllMongoIndexSets()) { - if (indexSet.isWriteIndexAlias(indexName)) { - return true; - } - } - - return false; + return findAllMongoIndexSets().stream() + .anyMatch(indexSet -> indexSet.isWriteIndexAlias(indexName)); } @Override public boolean isCurrentWriteIndex(String indexName) throws TooManyAliasesException { - for (MongoIndexSet indexSet : findAllMongoIndexSets()) { - if (indexSet.getActiveWriteIndex() != null && indexSet.getActiveWriteIndex().equals(indexName)) { - return true; - } - } - - return false; + return getForIndex(indexName) + .map(indexSet -> Objects.equals(indexSet.getActiveWriteIndex(), indexName)) + .orElse(false); } + @Nonnull @Override public Iterator iterator() { return getAll().iterator(); diff --git a/graylog2-server/src/test/java/org/graylog2/indexer/MongoIndexSetRegistryTest.java b/graylog2-server/src/test/java/org/graylog2/indexer/MongoIndexSetRegistryTest.java index 80349e8ea274..dbb842c943f6 100644 --- a/graylog2-server/src/test/java/org/graylog2/indexer/MongoIndexSetRegistryTest.java +++ b/graylog2-server/src/test/java/org/graylog2/indexer/MongoIndexSetRegistryTest.java @@ -16,6 +16,7 @@ */ package org.graylog2.indexer; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.EventBus; import org.graylog2.indexer.indexset.IndexSetConfig; @@ -28,15 +29,22 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; +import org.mockito.stubbing.Answer; +import java.time.ZonedDateTime; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; public class MongoIndexSetRegistryTest { @@ -266,4 +274,93 @@ public void isManagedIndexWithUnmanagedIndexReturnsFalse() { assertThat(indexSetRegistry.isManagedIndex("index")).isFalse(); } + + @Test + public void isCurrentWriteIndexSingleIndexSetConfigHappyPath() { + final String idxName = "index"; + final IndexSetConfig indexSetConfig = mock(IndexSetConfig.class); + final List indexSetConfigs = Collections.singletonList(indexSetConfig); + final MongoIndexSet indexSet = mock(MongoIndexSet.class); + when(mongoIndexSetFactory.create(indexSetConfig)).thenReturn(indexSet); + when(indexSetService.findAll()).thenReturn(indexSetConfigs); + when(indexSet.isManagedIndex(idxName)).thenReturn(true); + when(indexSet.getActiveWriteIndex()).thenReturn(idxName); + assertThat(indexSetRegistry.isCurrentWriteIndex(idxName)).isTrue(); + + verify(indexSet).isManagedIndex(idxName); + verify(indexSet).getActiveWriteIndex(); + verifyNoMoreInteractions(indexSetConfig, indexSet); + } + + @Test + public void isCurrentWriteIndexSingleIndexSetConfigNoneFound() { + final String idxName = "index"; + final IndexSetConfig indexSetConfig = mock(IndexSetConfig.class); + final List indexSetConfigs = Collections.singletonList(indexSetConfig); + final MongoIndexSet indexSet = mock(MongoIndexSet.class); + when(mongoIndexSetFactory.create(indexSetConfig)).thenReturn(indexSet); + when(indexSetService.findAll()).thenReturn(indexSetConfigs); + lenient().when(indexSet.getActiveWriteIndex()).thenReturn("non_existing_index"); + assertThat(indexSetRegistry.isCurrentWriteIndex(idxName)).isFalse(); + + verify(indexSet).isManagedIndex(idxName); + verify(indexSetService).findAll(); + verifyNoMoreInteractions(indexSetConfig, indexSet); + } + + @Test + public void isCurrentWriteIndexOnManyIndexSetConfigsNoneFoundDueToMismatchingIndexName() { + final String idxName = "index"; + final int noOfIndices = 2; + // The following constructs are necessary, because internally the IndexSetConfigs are handled as Set. + // Simply mocking a gazillion of them would de-duplicate, rendering the entire test useless. + final List indexSetConfigs = mkIndexSetConfigs(noOfIndices); + final ImmutableSet.Builder mockedMongosBuilder = ImmutableSet.builder(); + final AtomicReference matchingMongoMock = new AtomicReference<>(); + when(mongoIndexSetFactory.create(any(IndexSetConfig.class))) + .thenAnswer((Answer) invocation -> { + Object[] args = invocation.getArguments(); + final IndexSetConfig cfg = (IndexSetConfig) args[0]; + final MongoIndexSet mockedIndexSet = mock(MongoIndexSet.class); + lenient().when(mockedIndexSet.getConfig()).thenReturn(cfg); + final int currentIndex = Integer.parseInt(Objects.requireNonNull(cfg.id())); + if (currentIndex == noOfIndices - 1) { + when(mockedIndexSet.getActiveWriteIndex()).thenReturn(cfg.indexPrefix() + "_0"); + //Let the last MongoIndexSet be the one responsible to the index we're looking for: + when(mockedIndexSet.isManagedIndex(idxName)).thenReturn(true); + matchingMongoMock.getAndSet(mockedIndexSet); + } + mockedMongosBuilder.add(mockedIndexSet); + return mockedIndexSet; + } + ); + when(indexSetService.findAll()).thenReturn(indexSetConfigs); + assertThat(indexSetRegistry.isCurrentWriteIndex(idxName)).isFalse(); + + final ImmutableSet mockedMongos = mockedMongosBuilder.build(); + mockedMongos.forEach(mockedMongo -> verify(mockedMongo).isManagedIndex(idxName)); + verify(matchingMongoMock.get()).getActiveWriteIndex(); + verifyNoMoreInteractions(mockedMongos.toArray(new Object[0])); + } + + private List mkIndexSetConfigs(final int howMany) { + final ImmutableList.Builder configs = ImmutableList.builder(); + for (int i = 0; i < howMany; i++) { + configs.add(IndexSetConfig.builder() + .id(String.valueOf(i)) + .title("title " + i) + .indexPrefix("index" + i) + .shards(1) + .replicas(0) + .creationDate(ZonedDateTime.now()) + .indexOptimizationMaxNumSegments(1) + .indexOptimizationDisabled(false) + .indexAnalyzer("any") + .indexTemplateName("template" + i) + .build() + ); + + } + return configs.build(); + } } From 8e3941d8a038f392839799586570da568509f53f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Dec 2024 13:11:14 +0100 Subject: [PATCH 6/7] Bump io.netty:netty-bom from 4.1.115.Final to 4.1.116.Final (#21211) Bumps [io.netty:netty-bom](https://github.com/netty/netty) from 4.1.115.Final to 4.1.116.Final. - [Commits](https://github.com/netty/netty/compare/netty-4.1.115.Final...netty-4.1.116.Final) --- updated-dependencies: - dependency-name: io.netty:netty-bom dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 320907e4f1c0..ab2d666c4f6f 100644 --- a/pom.xml +++ b/pom.xml @@ -154,7 +154,7 @@ 5.2.1 4.11.0 0.13 - 4.1.115.Final + 4.1.116.Final 2.0.69.Final 4.12.0 2.3 From 45e5c07e4caf62045f327ff2ee9dfb6b17762b56 Mon Sep 17 00:00:00 2001 From: Tomas Dvorak Date: Wed, 18 Dec 2024 14:29:10 +0100 Subject: [PATCH 7/7] Better handling of intermediate CAs in datanode truststore (#21062) * TruststoreCreator adds the whole certchain, code cleanup * further truststore simplifications, unique aliases to prevent overriding already added certs * added changelog --- changelog/unreleased/pr-21062.toml | 4 + .../configuration/TruststoreCreator.java | 53 +++++++-- .../OpensearchSecurityConfigurationBean.java | 13 +-- .../configuration/TruststoreCreatorTest.java | 102 ++++++++++++++---- .../DatanodeSecurityTestUtils.java | 30 ++---- .../csr/FilesystemKeystoreInformation.java | 6 +- .../csr/InMemoryKeystoreInformation.java | 2 +- .../certutil/csr/KeystoreInformation.java | 4 +- 8 files changed, 152 insertions(+), 62 deletions(-) create mode 100644 changelog/unreleased/pr-21062.toml diff --git a/changelog/unreleased/pr-21062.toml b/changelog/unreleased/pr-21062.toml new file mode 100644 index 000000000000..0df01e48c0a3 --- /dev/null +++ b/changelog/unreleased/pr-21062.toml @@ -0,0 +1,4 @@ +type = "c" +message = "Better handling of intermediate CAs in datanode truststore" + +pulls = ["21062"] diff --git a/data-node/src/main/java/org/graylog/datanode/configuration/TruststoreCreator.java b/data-node/src/main/java/org/graylog/datanode/configuration/TruststoreCreator.java index 57950d5dea11..5cbb5b1f07c3 100644 --- a/data-node/src/main/java/org/graylog/datanode/configuration/TruststoreCreator.java +++ b/data-node/src/main/java/org/graylog/datanode/configuration/TruststoreCreator.java @@ -22,6 +22,7 @@ import org.graylog.security.certutil.csr.InMemoryKeystoreInformation; import org.graylog.security.certutil.csr.KeystoreInformation; +import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; import java.nio.file.Path; @@ -34,6 +35,7 @@ import java.security.cert.X509Certificate; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; public class TruststoreCreator { @@ -59,22 +61,33 @@ public static TruststoreCreator newEmpty() { } } - public TruststoreCreator addRootCert(final String name, KeystoreInformation keystoreInformation, - final String alias) throws GeneralSecurityException { - final X509Certificate rootCert; - try { - rootCert = findRootCert(keystoreInformation, alias); - } catch (Exception e) { - throw new RuntimeException(e); - } - this.truststore.setCertificateEntry(name, rootCert); - return this; + /** + * Originally we added only the root(=selfsigned) certificate to the truststore. But this causes problems with + * usage of intermediate CAs. There is nothing wrong adding the whole cert chain to the truststore. + * + * @param keystoreInformation access to the keystore, to obtain certificate chains by the given alias + * @param alias which certificate chain should we extract from the provided keystore + */ + public TruststoreCreator addFromKeystore(KeystoreInformation keystoreInformation, + final String alias) throws IOException, GeneralSecurityException { + final KeyStore keystore = keystoreInformation.loadKeystore(); + final Certificate[] chain = keystore.getCertificateChain(alias); + final List x509Certs = toX509Certs(chain); + return addCertificates(x509Certs); + } + + @Nonnull + private static List toX509Certs(Certificate[] certs) { + return Arrays.stream(certs) + .filter(c -> c instanceof X509Certificate) + .map(c -> (X509Certificate) c) + .toList(); } public TruststoreCreator addCertificates(List trustedCertificates) { trustedCertificates.forEach(cert -> { try { - this.truststore.setCertificateEntry(cert.getSubjectX500Principal().getName(), cert); + this.truststore.setCertificateEntry(generateAlias(this.truststore, cert), cert); } catch (KeyStoreException e) { throw new RuntimeException(e); } @@ -82,6 +95,24 @@ public TruststoreCreator addCertificates(List trustedCertificat return this; } + /** + * Alias has no meaning for the trust and validation purposes in the truststore. It's there only for managing + * the truststore content. We just need to make sure that we are using unique aliases, otherwise the + * truststore would override already present certificates. + * + * If there is no collision, we use the cname as given in the cert. In case of collisions, we'll append _i, + * where is index an incremented till it's unique in the truststore. + */ + private static String generateAlias(KeyStore truststore, X509Certificate cert) throws KeyStoreException { + AtomicInteger counter = new AtomicInteger(1); + final String cname = cert.getSubjectX500Principal().getName(); + String alias = cname; + while (truststore.containsAlias(alias)) { + alias = cname + "_" + counter.getAndIncrement(); + } + return alias; + } + public FilesystemKeystoreInformation persist(final Path truststorePath, final char[] truststorePassword) throws IOException, GeneralSecurityException { try (final FileOutputStream fileOutputStream = new FileOutputStream(truststorePath.toFile())) { diff --git a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchSecurityConfigurationBean.java b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchSecurityConfigurationBean.java index f026d2bda49f..b48bbddd446e 100644 --- a/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchSecurityConfigurationBean.java +++ b/data-node/src/main/java/org/graylog/datanode/opensearch/configuration/beans/impl/OpensearchSecurityConfigurationBean.java @@ -36,6 +36,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Path; import java.security.GeneralSecurityException; import java.security.KeyStore; @@ -112,10 +113,10 @@ public DatanodeConfigurationPart buildConfigurationPart(OpensearchConfigurationP try { configurationBuilder.httpCertificate(cert); configurationBuilder.withConfigFile(new KeystoreConfigFile(Path.of(TARGET_DATANODE_HTTP_KEYSTORE_FILENAME), cert)); - truststoreCreator.addRootCert("http-cert", cert, CertConstants.DATANODE_KEY_ALIAS); + truststoreCreator.addFromKeystore(cert, CertConstants.DATANODE_KEY_ALIAS); logCertificateInformation("HTTP certificate", cert); - } catch (GeneralSecurityException e) { - throw new RuntimeException(e); + } catch (GeneralSecurityException | IOException e) { + throw new OpensearchConfigurationException(e); } }); @@ -123,10 +124,10 @@ public DatanodeConfigurationPart buildConfigurationPart(OpensearchConfigurationP try { configurationBuilder.transportCertificate(cert); configurationBuilder.withConfigFile(new KeystoreConfigFile(Path.of(TARGET_DATANODE_TRANSPORT_KEYSTORE_FILENAME), cert)); - truststoreCreator.addRootCert("transport-cert", cert, CertConstants.DATANODE_KEY_ALIAS); + truststoreCreator.addFromKeystore(cert, CertConstants.DATANODE_KEY_ALIAS); logCertificateInformation("Transport certificate", cert); - } catch (GeneralSecurityException e) { - throw new RuntimeException(e); + } catch (GeneralSecurityException | IOException e) { + throw new OpensearchConfigurationException(e); } }); diff --git a/data-node/src/test/java/org/graylog/datanode/configuration/TruststoreCreatorTest.java b/data-node/src/test/java/org/graylog/datanode/configuration/TruststoreCreatorTest.java index 4520a7a0602e..715b7410b596 100644 --- a/data-node/src/test/java/org/graylog/datanode/configuration/TruststoreCreatorTest.java +++ b/data-node/src/test/java/org/graylog/datanode/configuration/TruststoreCreatorTest.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; +import jakarta.annotation.Nonnull; import org.apache.commons.lang3.RandomStringUtils; import org.assertj.core.api.Assertions; import org.bouncycastle.asn1.x500.X500Name; @@ -28,12 +29,19 @@ import org.bouncycastle.operator.OperatorCreationException; import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder; import org.graylog.security.certutil.CertConstants; +import org.graylog.security.certutil.CertRequest; +import org.graylog.security.certutil.CertificateGenerator; +import org.graylog.security.certutil.KeyPair; import org.graylog.security.certutil.csr.FilesystemKeystoreInformation; +import org.graylog.security.certutil.csr.InMemoryKeystoreInformation; +import org.graylog.security.certutil.csr.KeystoreInformation; import org.graylog.security.certutil.keystore.storage.KeystoreFileStorage; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import java.io.FileOutputStream; +import javax.net.ssl.TrustManager; +import javax.net.ssl.TrustManagerFactory; +import javax.net.ssl.X509TrustManager; import java.io.IOException; import java.math.BigInteger; import java.nio.file.Path; @@ -41,11 +49,15 @@ import java.security.KeyPairGenerator; import java.security.KeyStore; import java.security.KeyStoreException; +import java.security.NoSuchAlgorithmException; import java.security.cert.Certificate; +import java.security.cert.CertificateException; import java.security.cert.X509Certificate; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Optional; @@ -61,12 +73,12 @@ public class TruststoreCreatorTest { @Test void testTrustStoreCreation(@TempDir Path tempDir) throws Exception { - final FilesystemKeystoreInformation root = createKeystore(tempDir.resolve("root.p12"), "root", "CN=ROOT", BigInteger.ONE); - final FilesystemKeystoreInformation boot = createKeystore(tempDir.resolve("boot.p12"), "boot", "CN=BOOT", BigInteger.TWO); + final KeystoreInformation root = createKeystore(tempDir.resolve("root.p12"), "root", "CN=ROOT", BigInteger.ONE); + final KeystoreInformation boot = createKeystore(tempDir.resolve("boot.p12"), "boot", "CN=BOOT", BigInteger.TWO); final FilesystemKeystoreInformation truststore = TruststoreCreator.newEmpty() - .addRootCert("root", root, "root") - .addRootCert("boot", boot, "boot") + .addFromKeystore(root, "root") + .addFromKeystore(boot, "boot") .persist(tempDir.resolve("truststore.sec"), "caramba! caramba!".toCharArray()); @@ -79,11 +91,11 @@ void testTrustStoreCreation(@TempDir Path tempDir) throws Exception { final KeyStore keyStore = keyStoreOptional.get(); assertThat(ImmutableList.copyOf(keyStore.aliases().asIterator())) - .containsOnly("root", "boot"); + .containsOnly("cn=root", "cn=boot"); - final Certificate rootCert = keyStore.getCertificate("root"); + final Certificate rootCert = keyStore.getCertificate("cn=root"); verifyCertificate(rootCert, "CN=ROOT", BigInteger.ONE); - final Certificate bootCert = keyStore.getCertificate("boot"); + final Certificate bootCert = keyStore.getCertificate("cn=boot"); verifyCertificate(bootCert, "CN=BOOT", BigInteger.TWO); } @@ -98,7 +110,7 @@ void testDefaultJvm() throws KeyStoreException { @Test void testAdditionalCertificates(@TempDir Path tempDir) throws GeneralSecurityException, IOException, OperatorCreationException { - final FilesystemKeystoreInformation root = createKeystore(tempDir.resolve("root.p12"), "something-unknown", "CN=ROOT", BigInteger.ONE); + final KeystoreInformation root = createKeystore(tempDir.resolve("root.p12"), "something-unknown", "CN=ROOT", BigInteger.ONE); final X509Certificate cert = (X509Certificate) root.loadKeystore().getCertificate("something-unknown"); final FilesystemKeystoreInformation truststore = TruststoreCreator.newEmpty() @@ -111,7 +123,64 @@ void testAdditionalCertificates(@TempDir Path tempDir) throws GeneralSecurityExc Assertions.assertThat(alias) .isNotNull() .isEqualTo("cn=root"); + } + + @Test + void testIntermediateCa() throws Exception { + final KeyPair ca = CertificateGenerator.generate(CertRequest.selfSigned("my-ca").isCA(true).validity(Duration.ofDays(100))); + final KeyPair intermediateCa = CertificateGenerator.generate(CertRequest.signed("intermediate", ca).isCA(true).validity(Duration.ofDays(100))); + final KeyPair nodeKeys = CertificateGenerator.generate(CertRequest.signed("my-node", intermediateCa).isCA(false).validity(Duration.ofDays(100))); + + + final InMemoryKeystoreInformation keystoreInformation = createInMemoryKeystore(nodeKeys, intermediateCa); + final KeyStore truststore = TruststoreCreator.newEmpty() + .addFromKeystore(keystoreInformation, "my-node") + .getTruststore(); + + final X509TrustManager defaultTrustManager = createTrustManager(truststore); + + Assertions.assertThatNoException().isThrownBy(() -> defaultTrustManager.checkServerTrusted(new X509Certificate[]{nodeKeys.certificate()}, "RSA")); + + final KeyPair fakeNodeKeys = CertificateGenerator.generate(CertRequest.selfSigned("my-fake-node").isCA(false).validity(Duration.ofDays(100))); + Assertions.assertThatThrownBy(() -> defaultTrustManager.checkServerTrusted(new X509Certificate[]{fakeNodeKeys.certificate()}, "RSA")) + .isInstanceOf(CertificateException.class); + } + + @Test + void testDuplicateCname() throws Exception { + final KeyPair ca1 = CertificateGenerator.generate(CertRequest.selfSigned("my-ca").isCA(true).validity(Duration.ofDays(90))); + final KeyPair ca2 = CertificateGenerator.generate(CertRequest.selfSigned("my-ca").isCA(true).validity(Duration.ofDays(90))); + final KeyPair ca3 = CertificateGenerator.generate(CertRequest.selfSigned("my-ca").isCA(true).validity(Duration.ofDays(90))); + + final KeyStore truststore = TruststoreCreator.newEmpty() + .addCertificates(List.of(ca1.certificate())) + .addCertificates(List.of(ca2.certificate())) + .addCertificates(List.of(ca3.certificate())) + .getTruststore(); + + Assertions.assertThat(Collections.list(truststore.aliases())) + .hasSize(3) + .contains("cn=my-ca") + .contains("cn=my-ca_1") + .contains("cn=my-ca_2"); + } + + private static X509TrustManager createTrustManager(KeyStore caTruststore) throws NoSuchAlgorithmException, KeyStoreException { + final TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); + tmf.init(caTruststore); + final TrustManager[] trustManagers = tmf.getTrustManagers(); + return (X509TrustManager) trustManagers[0]; + } + + @SuppressWarnings("deprecation") + @Nonnull + private static InMemoryKeystoreInformation createInMemoryKeystore(KeyPair nodeKeys, KeyPair intermediate) throws KeyStoreException, IOException, NoSuchAlgorithmException, CertificateException { + final char[] password = RandomStringUtils.randomAlphabetic(256).toCharArray(); + KeyStore keystore = KeyStore.getInstance(CertConstants.PKCS12); + keystore.load(null, null); + keystore.setKeyEntry("my-node", nodeKeys.privateKey(), password, new Certificate[]{nodeKeys.certificate(), intermediate.certificate()}); + return new InMemoryKeystoreInformation(keystore, password); } private void verifyCertificate(final Certificate rootCert, final String cnName, final BigInteger serialNumber) { @@ -124,7 +193,8 @@ private void verifyCertificate(final Certificate rootCert, final String cnName, assertEquals(cnName, x509Certificate.getIssuerX500Principal().getName()); } - private FilesystemKeystoreInformation createKeystore(Path path, String alias, final String cnName, final BigInteger serialNumber) throws GeneralSecurityException, OperatorCreationException, IOException { + @SuppressWarnings("deprecation") + private KeystoreInformation createKeystore(Path path, String alias, final String cnName, final BigInteger serialNumber) throws GeneralSecurityException, OperatorCreationException, IOException { KeyPairGenerator keyGen = KeyPairGenerator.getInstance(KEY_GENERATION_ALGORITHM); java.security.KeyPair certKeyPair = keyGen.generateKeyPair(); X500Name name = new X500Name(cnName); @@ -142,17 +212,13 @@ private FilesystemKeystoreInformation createKeystore(Path path, String alias, fi final X509Certificate signedCert = new JcaX509CertificateConverter().getCertificate(certHolder); - KeyStore trustStore = KeyStore.getInstance(CertConstants.PKCS12); - trustStore.load(null, null); + KeyStore keyStore = KeyStore.getInstance(CertConstants.PKCS12); + keyStore.load(null, null); final char[] password = RandomStringUtils.randomAlphabetic(256).toCharArray(); - trustStore.setKeyEntry(alias, certKeyPair.getPrivate(), password, new Certificate[]{signedCert}); - + keyStore.setKeyEntry(alias, certKeyPair.getPrivate(), password, new Certificate[]{signedCert}); - try (final FileOutputStream fileOutputStream = new FileOutputStream(path.toFile())) { - trustStore.store(fileOutputStream, password); - } - return new FilesystemKeystoreInformation(path, password); + return new InMemoryKeystoreInformation(keyStore, password); } } diff --git a/data-node/src/test/java/org/graylog/datanode/integration/DatanodeSecurityTestUtils.java b/data-node/src/test/java/org/graylog/datanode/integration/DatanodeSecurityTestUtils.java index 8bd3bf09ce6f..b4237d5a60ad 100644 --- a/data-node/src/test/java/org/graylog/datanode/integration/DatanodeSecurityTestUtils.java +++ b/data-node/src/test/java/org/graylog/datanode/integration/DatanodeSecurityTestUtils.java @@ -17,41 +17,29 @@ package org.graylog.datanode.integration; import org.apache.commons.lang3.RandomStringUtils; +import org.graylog.datanode.configuration.TruststoreCreator; import org.graylog.security.certutil.CertutilCa; import org.graylog.security.certutil.CertutilCert; import org.graylog.security.certutil.CertutilHttp; import org.graylog.security.certutil.console.TestableConsole; import org.graylog.security.certutil.csr.FilesystemKeystoreInformation; +import org.graylog.security.certutil.csr.KeystoreInformation; -import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Path; import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.cert.Certificate; -import java.security.cert.X509Certificate; import java.util.Enumeration; public class DatanodeSecurityTestUtils { - public static KeyStore buildTruststore(FilesystemKeystoreInformation ca) throws IOException, GeneralSecurityException { - try (FileInputStream fis = new FileInputStream(ca.location().toFile())) { - - KeyStore caKeystore = KeyStore.getInstance("PKCS12"); - caKeystore.load(fis, ca.password()); - - KeyStore trustStore = KeyStore.getInstance("PKCS12"); - trustStore.load(null, null); - - final Enumeration aliases = caKeystore.aliases(); - while (aliases.hasMoreElements()) { - final String alias = aliases.nextElement(); - final Certificate cert = caKeystore.getCertificate(alias); - if (cert instanceof final X509Certificate x509Certificate) { - trustStore.setCertificateEntry(alias, x509Certificate); - } - } - return trustStore; + public static KeyStore buildTruststore(KeystoreInformation ca) throws IOException, GeneralSecurityException { + final TruststoreCreator truststoreCreator = TruststoreCreator.newEmpty(); + final Enumeration aliases = ca.loadKeystore().aliases(); + while (aliases.hasMoreElements()) { + final String alias = aliases.nextElement(); + truststoreCreator.addFromKeystore(ca, alias); } + return truststoreCreator.getTruststore(); } public static FilesystemKeystoreInformation generateCa(Path dir) { diff --git a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/FilesystemKeystoreInformation.java b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/FilesystemKeystoreInformation.java index f46d7127bb5f..2fe48db6beff 100644 --- a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/FilesystemKeystoreInformation.java +++ b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/FilesystemKeystoreInformation.java @@ -19,10 +19,8 @@ import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Path; +import java.security.GeneralSecurityException; import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.cert.CertificateException; import java.util.Arrays; import java.util.Objects; @@ -38,7 +36,7 @@ public FilesystemKeystoreInformation(Path location, char[] password) { } @Override - public KeyStore loadKeystore() throws KeyStoreException, IOException, CertificateException, NoSuchAlgorithmException { + public KeyStore loadKeystore() throws IOException, GeneralSecurityException { KeyStore keyStore = KeyStore.getInstance(PKCS12); try (FileInputStream fis = new FileInputStream(location.toFile())) { keyStore.load(fis, password); diff --git a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/InMemoryKeystoreInformation.java b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/InMemoryKeystoreInformation.java index f4f216ddaf3b..dad8af8b0a9d 100644 --- a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/InMemoryKeystoreInformation.java +++ b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/InMemoryKeystoreInformation.java @@ -29,7 +29,7 @@ public InMemoryKeystoreInformation(KeyStore keyStore, char[] password) { } @Override - public KeyStore loadKeystore() throws Exception { + public KeyStore loadKeystore() { return keyStore; } diff --git a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/KeystoreInformation.java b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/KeystoreInformation.java index 746eb4ef773d..424d94309e30 100644 --- a/graylog2-server/src/main/java/org/graylog/security/certutil/csr/KeystoreInformation.java +++ b/graylog2-server/src/main/java/org/graylog/security/certutil/csr/KeystoreInformation.java @@ -16,11 +16,13 @@ */ package org.graylog.security.certutil.csr; +import java.io.IOException; +import java.security.GeneralSecurityException; import java.security.KeyStore; public interface KeystoreInformation { - KeyStore loadKeystore() throws Exception; + KeyStore loadKeystore() throws IOException, GeneralSecurityException; char[] password(); }