Skip to content

Commit 2fc6bfc

Browse files
jfredenjfreden
committed
fixup! Reduce scope
1 parent 3b4bd9f commit 2fc6bfc

File tree

6 files changed

+40
-82
lines changed

6 files changed

+40
-82
lines changed

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/CrossClusterApiKeySignerIntegTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public void testSignWithPemKeyConfig() {
3030
);
3131
final String[] testHeaders = randomArray(5, String[]::new, () -> randomAlphanumericOfLength(randomInt(20)));
3232

33-
X509CertificateSignature signature = signer.signHeadersForCluster(STATIC_TEST_CLUSTER_ALIAS, testHeaders);
33+
X509CertificateSignature signature = signer.sign(STATIC_TEST_CLUSTER_ALIAS, testHeaders);
3434
signature.certificate().getPublicKey();
3535

3636
var keyConfig = new PemKeyConfig(
@@ -51,7 +51,7 @@ public void testSignUnknownClusterAlias() {
5151
);
5252
final String[] testHeaders = randomArray(5, String[]::new, () -> randomAlphanumericOfLength(randomInt(20)));
5353

54-
X509CertificateSignature signature = signer.signHeadersForCluster("unknowncluster", testHeaders);
54+
X509CertificateSignature signature = signer.sign("unknowncluster", testHeaders);
5555
assertNull(signature);
5656
}
5757

@@ -72,7 +72,7 @@ public void testSeveralKeyStoreAliases() {
7272
);
7373

7474
{
75-
X509CertificateSignature signature = signer.signHeadersForCluster(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
75+
X509CertificateSignature signature = signer.sign(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
7676
assertNull(signature);
7777
}
7878

@@ -82,7 +82,7 @@ public void testSeveralKeyStoreAliases() {
8282
.put(SIGNING_KEYSTORE_ALIAS.getConcreteSettingForNamespace(DYNAMIC_TEST_CLUSTER_ALIAS).getKey(), "wholelottakey")
8383
);
8484
{
85-
X509CertificateSignature signature = signer.signHeadersForCluster(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
85+
X509CertificateSignature signature = signer.sign(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
8686
assertNotNull(signature);
8787
}
8888

@@ -92,7 +92,7 @@ public void testSeveralKeyStoreAliases() {
9292
.put(SIGNING_KEYSTORE_ALIAS.getConcreteSettingForNamespace(DYNAMIC_TEST_CLUSTER_ALIAS).getKey(), "idonotexist")
9393
);
9494
{
95-
X509CertificateSignature signature = signer.signHeadersForCluster(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
95+
X509CertificateSignature signature = signer.sign(DYNAMIC_TEST_CLUSTER_ALIAS, "test", "test");
9696
assertNull(signature);
9797
}
9898
} finally {

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/transport/CrossClusterSigningConfigurationReloaderIntegTests.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ public void testDependentKeyConfigFilesUpdated() throws Exception {
109109
Map.of(SIGNING_KEY_SECURE_PASSPHRASE.getConcreteSettingForNamespace(testClusterAlias).getKey(), "marshall".toCharArray())
110110
);
111111

112-
assertNull(signer.signHeadersForCluster(testClusterAlias, "a_header"));
112+
assertNull(signer.sign(testClusterAlias, "a_header"));
113113
Path tempDir = createTempDir();
114114
Path signingCert = tempDir.resolve("signing.crt");
115115
Files.copy(getDataPath("/org/elasticsearch/xpack/security/signature/signing_rsa.crt"), signingCert);
@@ -129,14 +129,14 @@ public void testDependentKeyConfigFilesUpdated() throws Exception {
129129
);
130130

131131
// Make sure a signature can be created
132-
var signatureBefore = signer.signHeadersForCluster(testClusterAlias, "test", "test");
132+
var signatureBefore = signer.sign(testClusterAlias, "test", "test");
133133
assertNotNull(signatureBefore);
134134

135135
Files.move(updatedSigningCert, signingCert, StandardCopyOption.REPLACE_EXISTING);
136136
Files.move(updatedSigningKey, signingKey, StandardCopyOption.REPLACE_EXISTING);
137137

138138
assertBusy(() -> {
139-
var signatureAfter = signer.signHeadersForCluster(testClusterAlias, "test", "test");
139+
var signatureAfter = signer.sign(testClusterAlias, "test", "test");
140140
assertNotNull(signatureAfter);
141141
assertNotEquals(signatureAfter, signatureBefore);
142142
});
@@ -157,7 +157,7 @@ public void testRemoveFileWithConfig() throws Exception {
157157
internalCluster().getRandomNodeName()
158158
);
159159

160-
assertNull(signer.signHeadersForCluster("test_cluster", "a_header"));
160+
assertNull(signer.sign("test_cluster", "a_header"));
161161
Path tempDir = createTempDir();
162162
Path signingCert = tempDir.resolve("signing.crt");
163163
Files.copy(getDataPath("/org/elasticsearch/xpack/security/signature/signing_rsa.crt"), signingCert);
@@ -172,14 +172,14 @@ public void testRemoveFileWithConfig() throws Exception {
172172
);
173173

174174
// Make sure a signature can be created
175-
var signatureBefore = signer.signHeadersForCluster("test_cluster", "test", "test");
175+
var signatureBefore = signer.sign("test_cluster", "test", "test");
176176
assertNotNull(signatureBefore);
177177

178178
// This should just fail the update, not remove any actual configs
179179
Files.delete(signingCert);
180180
Files.delete(signingKey);
181181

182-
var signatureAfter = signer.signHeadersForCluster("test_cluster", "test", "test");
182+
var signatureAfter = signer.sign("test_cluster", "test", "test");
183183
assertNotNull(signatureAfter);
184184
assertEquals(signatureAfter, signatureBefore);
185185
} finally {
@@ -206,15 +206,15 @@ private void addAndRemoveClusterConfigsRuntime(
206206
try {
207207
for (var clusterAlias : clusterAliases) {
208208
// Try to create a signature for a remote cluster that doesn't exist
209-
assertNull(signer.signHeadersForCluster(clusterAlias, testHeaders));
209+
assertNull(signer.sign(clusterAlias, testHeaders));
210210
clusterCreator.accept(clusterAlias);
211211
// Make sure a signature can be created
212-
assertNotNull(signer.signHeadersForCluster(clusterAlias, testHeaders));
212+
assertNotNull(signer.sign(clusterAlias, testHeaders));
213213
}
214214
for (var clusterAlias : clusterAliases) {
215215
clusterRemover.accept(clusterAlias);
216216
// Make sure no signature was created
217-
assertBusy(() -> assertNull(signer.signHeadersForCluster(clusterAlias, testHeaders)));
217+
assertBusy(() -> assertNull(signer.sign(clusterAlias, testHeaders)));
218218
}
219219
} finally {
220220
var builder = Settings.builder();
@@ -223,7 +223,9 @@ private void addAndRemoveClusterConfigsRuntime(
223223
builder.putNull(setting.getConcreteSettingForNamespace(clusterAlias).getKey());
224224
});
225225
}
226-
updateClusterSettings(builder.setSecureSettings(new MockSecureSettings()));
226+
if (clusterAliases.isEmpty() == false) {
227+
updateClusterSettings(builder.setSecureSettings(new MockSecureSettings()));
228+
}
227229
}
228230
}
229231

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/CrossClusterAccessHeaders.java‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,44 +8,27 @@
88
package org.elasticsearch.xpack.security.authc;
99

1010
import org.elasticsearch.common.util.concurrent.ThreadContext;
11-
import org.elasticsearch.core.Nullable;
1211
import org.elasticsearch.xpack.core.security.action.apikey.ApiKey;
1312
import org.elasticsearch.xpack.core.security.authc.CrossClusterAccessSubjectInfo;
14-
import org.elasticsearch.xpack.security.transport.X509CertificateSignature;
1513

1614
import java.io.IOException;
1715
import java.util.Objects;
1816

1917
public final class CrossClusterAccessHeaders {
2018

2119
public static final String CROSS_CLUSTER_ACCESS_CREDENTIALS_HEADER_KEY = "_cross_cluster_access_credentials";
22-
public static final String CROSS_CLUSTER_ACCESS_SIGNATURE_HEADER_KEY = "_cross_cluster_access_signature";
23-
2420
private final String credentialsHeader;
2521
private final CrossClusterAccessSubjectInfo crossClusterAccessSubjectInfo;
26-
private final X509CertificateSignature signature;
2722

2823
public CrossClusterAccessHeaders(String credentialsHeader, CrossClusterAccessSubjectInfo crossClusterAccessSubjectInfo) {
29-
this(credentialsHeader, crossClusterAccessSubjectInfo, null);
30-
}
31-
32-
public CrossClusterAccessHeaders(
33-
String credentialsHeader,
34-
CrossClusterAccessSubjectInfo crossClusterAccessSubjectInfo,
35-
@Nullable X509CertificateSignature signature
36-
) {
3724
assert credentialsHeader.startsWith("ApiKey ") : "credentials header must start with [ApiKey ]";
3825
this.credentialsHeader = credentialsHeader;
3926
this.crossClusterAccessSubjectInfo = crossClusterAccessSubjectInfo;
40-
this.signature = signature;
4127
}
4228

4329
public void writeToContext(final ThreadContext ctx) throws IOException {
4430
ctx.putHeader(CROSS_CLUSTER_ACCESS_CREDENTIALS_HEADER_KEY, credentialsHeader);
4531
crossClusterAccessSubjectInfo.writeToContext(ctx);
46-
if (signature != null) {
47-
ctx.putHeader(CROSS_CLUSTER_ACCESS_SIGNATURE_HEADER_KEY, signature.encodeToString());
48-
}
4932
}
5033

5134
public static CrossClusterAccessHeaders readFromContext(final ThreadContext ctx) throws IOException {
@@ -58,7 +41,7 @@ public static CrossClusterAccessHeaders readFromContext(final ThreadContext ctx)
5841
// Invoke parsing logic to validate that the header decodes to a valid API key credential
5942
// Call `close` since the returned value is an auto-closable
6043
parseCredentialsHeader(credentialsHeader).close();
61-
return new CrossClusterAccessHeaders(credentialsHeader, CrossClusterAccessSubjectInfo.readFromContext(ctx), null);
44+
return new CrossClusterAccessHeaders(credentialsHeader, CrossClusterAccessSubjectInfo.readFromContext(ctx));
6245
}
6346

6447
public ApiKeyService.ApiKeyCredentials credentials() {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/CrossClusterApiKeySigner.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public Map<Path, Set<String>> getDependentFilesToClusterAliases() {
6363

6464
public void loadSigningConfig(String clusterAlias, @Nullable Settings settings, boolean updateSecureSettings) {
6565
signingConfigByClusterAlias.compute(clusterAlias, (key, currentSigningConfig) -> {
66-
var effectiveSettings = buildEffectiveSettingsForSigningConfig(
66+
var effectiveSettings = buildEffectiveSettings(
6767
currentSigningConfig != null ? currentSigningConfig.settings : null,
6868
settings,
6969
updateSecureSettings
@@ -95,7 +95,7 @@ public void loadSigningConfig(String clusterAlias, @Nullable Settings settings,
9595
});
9696
}
9797

98-
public X509CertificateSignature signHeadersForCluster(String clusterAlias, String... headers) {
98+
public X509CertificateSignature sign(String clusterAlias, String... headers) {
9999
SigningConfig signingConfig = signingConfigByClusterAlias.get(clusterAlias);
100100
if (signingConfig == null || signingConfig.keyPair() == null) {
101101
return null;
@@ -121,12 +121,13 @@ private void loadSigningConfigs() {
121121
}
122122

123123
/**
124-
* Build the actual remote cluster settings to use
124+
* Build the effective remote cluster settings by merging the currently configured (if any) and new/updated settings
125+
* <p>
125126
* - If newSettings is null - use existing settings, used to refresh the dependent files
126127
* - If updateSecureSettings is true - merge secure settings from newSettings with current settings, used by secure settings refresh
127128
* - If updateSecureSettings is false - merge new settings with existing secure settings, used for regular settings update
128129
*/
129-
private Settings buildEffectiveSettingsForSigningConfig(
130+
private Settings buildEffectiveSettings(
130131
@Nullable Settings currentSettings,
131132
@Nullable Settings newSettings,
132133
boolean updateSecureSettings
@@ -148,7 +149,7 @@ private Settings buildEffectiveSettingsForSigningConfig(
148149
return Settings.builder().put(settingsSource, false).setSecureSettings(secureSettings).build();
149150
}
150151

151-
void reloadFilesForClusters(Set<String> clusterAliases) {
152+
void reloadSigningConfigs(Set<String> clusterAliases) {
152153
clusterAliases.forEach(alias -> loadSigningConfig(alias, null, true));
153154
}
154155

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/CrossClusterApiKeySignerReloader.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,14 @@ public CrossClusterApiKeySignerReloader(
5151
logger.info("Updating cross cluster signing configuration for key [{}] with settings [{}]", key, val);
5252
apiKeySigner.loadSigningConfig(key, val.getByPrefix(RemoteClusterSettings.REMOTE_CLUSTER_SETTINGS_PREFIX + key + "."), false);
5353
watchDependentFilesForClusterAliases(
54-
apiKeySigner::reloadFilesForClusters,
54+
apiKeySigner::reloadSigningConfigs,
5555
resourceWatcherService,
5656
apiKeySigner.getDependentFilesToClusterAliases()
5757
);
5858
});
5959

6060
watchDependentFilesForClusterAliases(
61-
apiKeySigner::reloadFilesForClusters,
61+
apiKeySigner::reloadSigningConfigs,
6262
resourceWatcherService,
6363
apiKeySigner.getDependentFilesToClusterAliases()
6464
);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java‎

Lines changed: 14 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import org.apache.logging.log4j.LogManager;
1010
import org.apache.logging.log4j.Logger;
11-
import org.elasticsearch.ElasticsearchSecurityException;
1211
import org.elasticsearch.TransportVersion;
1312
import org.elasticsearch.action.ActionListener;
1413
import org.elasticsearch.action.admin.cluster.state.ClusterStateAction;
@@ -60,7 +59,6 @@
6059
import org.elasticsearch.xpack.security.authz.AuthorizationUtils;
6160
import org.elasticsearch.xpack.security.authz.PreAuthorizationUtils;
6261

63-
import java.io.IOException;
6462
import java.util.Collections;
6563
import java.util.Map;
6664
import java.util.Optional;
@@ -372,38 +370,21 @@ private <T extends TransportResponse> void sendWithCrossClusterAccessHeaders(
372370
)
373371
);
374372
}
375-
CrossClusterAccessSubjectInfo subjectInfo = SystemUser.crossClusterAccessSubjectInfo(
376-
authentication.getEffectiveSubject().getTransportVersion(),
377-
authentication.getEffectiveSubject().getRealm().getNodeName()
373+
final var crossClusterAccessHeaders = new CrossClusterAccessHeaders(
374+
remoteClusterCredentials.credentials(),
375+
SystemUser.crossClusterAccessSubjectInfo(
376+
authentication.getEffectiveSubject().getTransportVersion(),
377+
authentication.getEffectiveSubject().getRealm().getNodeName()
378+
)
378379
);
379-
try {
380-
final var crossClusterAccessHeaders = new CrossClusterAccessHeaders(
381-
remoteClusterCredentials.credentials(),
382-
subjectInfo,
383-
crossClusterApiKeySigner.signHeadersForCluster(
384-
remoteClusterAlias,
385-
remoteClusterCredentials.credentials(),
386-
subjectInfo.encode()
387-
)
388-
);
389-
// To be able to enforce index-level privileges under the new remote cluster security model,
390-
// we switch from old-style internal actions to their new equivalent indices actions so that
391-
// they will be checked for index privileges against the index specified in the requests
392-
final String effectiveAction = RCS_INTERNAL_ACTIONS_REPLACEMENTS.getOrDefault(action, action);
393-
if (false == effectiveAction.equals(action)) {
394-
logger.trace("switching internal action from [{}] to [{}]", action, effectiveAction);
395-
}
396-
sendWithCrossClusterAccessHeaders(
397-
crossClusterAccessHeaders,
398-
connection,
399-
effectiveAction,
400-
request,
401-
options,
402-
handler
403-
);
404-
} catch (IOException e) {
405-
throw new ElasticsearchSecurityException("Failed to sign cross cluster headers", e);
380+
// To be able to enforce index-level privileges under the new remote cluster security model,
381+
// we switch from old-style internal actions to their new equivalent indices actions so that
382+
// they will be checked for index privileges against the index specified in the requests
383+
final String effectiveAction = RCS_INTERNAL_ACTIONS_REPLACEMENTS.getOrDefault(action, action);
384+
if (false == effectiveAction.equals(action)) {
385+
logger.trace("switching internal action from [{}] to [{}]", action, effectiveAction);
406386
}
387+
sendWithCrossClusterAccessHeaders(crossClusterAccessHeaders, connection, effectiveAction, request, options, handler);
407388
} else {
408389
assert false == action.startsWith("internal:") : "internal action must be sent with system user";
409390
authzService.getRoleDescriptorsIntersectionForRemoteCluster(
@@ -427,18 +408,9 @@ private <T extends TransportResponse> void sendWithCrossClusterAccessHeaders(
427408
remoteClusterAlias
428409
);
429410
}
430-
CrossClusterAccessSubjectInfo subjectInfo = new CrossClusterAccessSubjectInfo(
431-
authentication,
432-
roleDescriptorsIntersection
433-
);
434411
final var crossClusterAccessHeaders = new CrossClusterAccessHeaders(
435412
remoteClusterCredentials.credentials(),
436-
subjectInfo,
437-
crossClusterApiKeySigner.signHeadersForCluster(
438-
remoteClusterAlias,
439-
remoteClusterCredentials.credentials(),
440-
subjectInfo.encode()
441-
)
413+
new CrossClusterAccessSubjectInfo(authentication, roleDescriptorsIntersection)
442414
);
443415
sendWithCrossClusterAccessHeaders(crossClusterAccessHeaders, connection, action, request, options, handler);
444416
}, // it's safe to not use a context restore handler here since `getRoleDescriptorsIntersectionForRemoteCluster`

0 commit comments

Comments
 (0)