Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.io.IOException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;

import javax.crypto.KeyGenerator;
import javax.crypto.Mac;
Expand All @@ -32,8 +31,6 @@

import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;
import org.apache.hadoop.ipc.RetriableException;
import org.apache.hadoop.ipc.StandbyException;

Expand Down Expand Up @@ -115,61 +112,29 @@ public void checkAvailableForRead() throws StandbyException {
// Default to being available for read.
}

private static final String SELECTED_ALGORITHM;
private static final int SELECTED_LENGTH;

static {
Configuration conf = new Configuration();
String algorithm = conf.get(
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_KEY,
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_DEFAULT);
LOG.debug("Selected hash algorithm: {}", algorithm);
SELECTED_ALGORITHM = algorithm;
int length = conf.getInt(
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_KEY,
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_DEFAULT);
LOG.debug("Selected hash key length:{}", length);
SELECTED_LENGTH = length;
}

/**
* A thread local store for the Macs.
*/
private static final ThreadLocal<Mac> threadLocalMac =
new ThreadLocal<Mac>(){
@Override
protected Mac initialValue() {
try {
return Mac.getInstance(SELECTED_ALGORITHM);
} catch (NoSuchAlgorithmException nsa) {
throw new IllegalArgumentException("Can't find " + SELECTED_ALGORITHM, nsa);
}
}
};
ThreadLocal.withInitial(SecretManagerConfig::createMac);

/**
* Key generator to use.
*/
private final KeyGenerator keyGen;
{
try {
keyGen = KeyGenerator.getInstance(SELECTED_ALGORITHM);
keyGen.init(SELECTED_LENGTH);
} catch (NoSuchAlgorithmException nsa) {
throw new IllegalArgumentException("Can't find " + SELECTED_ALGORITHM, nsa);
}
}
private volatile KeyGenerator keyGen;
private final Object keyGenLock = new Object();

/**
* Generate a new random secret key.
* @return the new key
*/
protected SecretKey generateSecret() {
SecretKey key;
synchronized (keyGen) {
key = keyGen.generateKey();
synchronized (keyGenLock) {
if (keyGen == null) {
keyGen = SecretManagerConfig.createKeyGenerator();
}
return keyGen.generateKey();
}
return key;
}

/**
Expand Down Expand Up @@ -197,6 +162,6 @@ public static byte[] createPassword(byte[] identifier,
* @return the secret key
*/
protected static SecretKey createSecretKey(byte[] key) {
return new SecretKeySpec(key, SELECTED_ALGORITHM);
return new SecretKeySpec(key, SecretManagerConfig.getSelectedAlgorithm());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.security.token;

import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.crypto.KeyGenerator;
import javax.crypto.Mac;
import java.security.NoSuchAlgorithmException;
import java.util.Map;
import java.util.WeakHashMap;

/**
* Provides configuration and utility methods for managing cryptographic key generation
* and message authentication code (MAC) generation using specified algorithms and key lengths.
* <p>
* This class supports static access to the selected cryptographic algorithm and key length,
* and provides methods to create configured {@link javax.crypto.KeyGenerator}
* and {@link javax.crypto.Mac} instances.
* The configuration is initialized statically from a provided {@link Configuration} object.
* <p>
* The {@link SecretManager} has some static method, so static configuration is required
*/
@InterfaceAudience.Public
@InterfaceStability.Evolving
public final class SecretManagerConfig {
private static final Logger LOG = LoggerFactory.getLogger(SecretManagerConfig.class);
private static String selectedAlgorithm;
private static int selectedLength;

private static final Map<Thread, KeyGenerator> KEYGENS = new WeakHashMap<>();

Choose a reason for hiding this comment

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

KeyGenerator is not threadlocal, there will be a single global instance, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If everything works as expected yes, but no there is not granted some one will not call this method again ...
you right this should be in SecretManager not in an other class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just rechecked the code, seems like Keygenerator is a local variable for every SecretManager instance.
Maybe this could be thread local, and maybe could improve the performance, but i would rather not touch this for sake of the stability.

public static final Map<Thread, Mac> MACS = new WeakHashMap<>();

static {
update(new Configuration());
}

private SecretManagerConfig() {
}

/**
* Updates the selected cryptographic algorithm and key length using the provided
* Hadoop {@link Configuration}. This method reads the values for
* {@code HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_KEY} and
* {@code HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_KEY}, or uses default values if not set.
*
* @param conf the configuration object containing cryptographic settings
*/
public static synchronized void update(Configuration conf) {
if (!KEYGENS.isEmpty()) {
LOG.warn("Keygen was already initialized with older config, those will not be updated." +
"Hint: If you turn on debug log you can see when it happened. Keygens: {}", KEYGENS);
}
if (!MACS.isEmpty()) {
LOG.warn("Mac was already initialized with older config, those will not be updated." +

Choose a reason for hiding this comment

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

I don't think we need to store and log all the macs, I would be satisfied with logging if there is any in the current thread, acquired by:

threadLocalMac.get()

assuming that the update() happens on the same thread as the usage, this can be a useful logging message (without logging a whole MACS collection)

"Hint: If you turn on debug log you can see when it happened. Macs: {}", MACS);
}
selectedAlgorithm = conf.get(
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_KEY,
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_DEFAULT);
LOG.debug("Selected hash algorithm: {}", selectedAlgorithm);
selectedLength = conf.getInt(
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_KEY,
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_DEFAULT);
LOG.debug("Selected hash key length: {}", selectedLength);
}

/**
* Returns the currently selected cryptographic algorithm.
*
* @return the name of the selected algorithm
*/
public static synchronized String getSelectedAlgorithm() {
return selectedAlgorithm;
}

/**
* Returns the currently selected key length in bits.
*
* @return the selected key length
*/
public static synchronized int getSelectedLength() {
return selectedLength;
}

/**
* Creates a new {@link KeyGenerator} instance configured with the currently selected
* algorithm and key length.
*
* @return a new {@code KeyGenerator} instance
* @throws IllegalArgumentException if the specified algorithm is not available
*/
public static synchronized KeyGenerator createKeyGenerator() {
LOG.debug("Creating key generator instance {} - {} bit with thread {}",
selectedAlgorithm, selectedLength, Thread.currentThread());
try {
KeyGenerator keyGen = KeyGenerator.getInstance(selectedAlgorithm);
keyGen.init(selectedLength);
KEYGENS.put(Thread.currentThread(), keyGen);
return keyGen;
} catch (NoSuchAlgorithmException nsa) {
throw new IllegalArgumentException("Can't find " + selectedAlgorithm, nsa);
}
}

/**
* Creates a new {@link Mac} instance using the currently selected algorithm.
*
* @return a new {@code Mac} instance
* @throws IllegalArgumentException if the specified algorithm is not available
*/
public static synchronized Mac createMac() {
LOG.debug("Creating mac instance {} with thread {}", selectedAlgorithm, Thread.currentThread());
try {
Mac mac = Mac.getInstance(selectedAlgorithm);
MACS.put(Thread.currentThread(), mac);
return mac;
} catch (NoSuchAlgorithmException nsa) {
throw new IllegalArgumentException("Can't find " + selectedAlgorithm, nsa);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.hadoop.security.token;

import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.CommonConfigurationKeysPublic;

import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

import javax.crypto.KeyGenerator;
import javax.crypto.Mac;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class TestSecurityManagerConfig {

private final String defaultAlgorithm =
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_DEFAULT;
private final int defaultLength =
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_DEFAULT;
private final String strongAlgorithm = "HmacSHA256";
private final int strongLength = 256;

@Test
public void testDefaults() {
assertEquals(defaultAlgorithm, SecretManagerConfig.getSelectedAlgorithm());
assertEquals(defaultLength, SecretManagerConfig.getSelectedLength());
}

@Test
public void testUpdateByConfig() {
SecretManagerConfig.update(createConfiguration(strongAlgorithm, strongLength));
assertEquals(strongAlgorithm, SecretManagerConfig.getSelectedAlgorithm());
assertEquals(strongLength, SecretManagerConfig.getSelectedLength());
}

@Test
public void testMacCreation() {
SecretManagerConfig.update(createConfiguration(strongAlgorithm, strongLength));
Mac mac = SecretManagerConfig.createMac();
assertEquals(strongAlgorithm, mac.getAlgorithm());
}

@Test
public void testMacCreationUnknownAlgorithm() {
SecretManagerConfig.update(
createConfiguration("testMacCreationUnknownAlgorithm_NO_ALG", defaultLength));
assertThrows(IllegalArgumentException.class, SecretManagerConfig::createMac);
}

@Test
public void testKeygenCreation() {
SecretManagerConfig.update(createConfiguration(strongAlgorithm, strongLength));
KeyGenerator keyGenerator = SecretManagerConfig.createKeyGenerator();
assertEquals(strongAlgorithm, keyGenerator.getAlgorithm());
}

@Test
public void testKeygenCreationUnknownAlgorithm() {
SecretManagerConfig.update(
createConfiguration("testKeygenCreationUnknownAlgorithm_NO_ALG", defaultLength));
assertThrows(IllegalArgumentException.class, SecretManagerConfig::createKeyGenerator);
}

@Test
public void testConfigUpdateAfterKeygenCreation() {
SecretManagerConfig.update(createConfiguration(strongAlgorithm, strongLength));
KeyGenerator keyGenerator = SecretManagerConfig.createKeyGenerator();
SecretManagerConfig.update(createConfiguration(defaultAlgorithm, defaultLength));
assertEquals(strongAlgorithm, keyGenerator.getAlgorithm());
}

@AfterEach
public void tearDown() {
SecretManagerConfig.update(createConfiguration(defaultAlgorithm, defaultLength));
}

private Configuration createConfiguration(String algorithm, int length) {
Configuration conf = new Configuration();
conf.set(
CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_GENERATOR_ALGORITHM_KEY,
algorithm);
conf.setInt(CommonConfigurationKeysPublic.HADOOP_SECURITY_SECRET_MANAGER_KEY_LENGTH_KEY,
length);
return conf;
}
}