Generate default password during the plugin's init step During the plugin's init step one can either rely on that the default password (16 bytes long) under '$site/data/@PLUGIN@/default.key' file is being generated, provide own configuration or add a new one under new 'key-id' (that will be marked as default). Bug: Issue 16192 Change-Id: I99e4024ecf19d828deb2d84b40fe1dd7e1abe23d
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java index a826886..7ef81d1 100644 --- a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfig.java
@@ -13,6 +13,8 @@ // limitations under the License. package com.googlesource.gerrit.plugins.github.oauth; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL; + import com.google.common.base.CharMatcher; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; @@ -38,13 +40,9 @@ import javax.servlet.http.HttpServletRequest; import lombok.Getter; import org.eclipse.jgit.lib.Config; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Singleton public class GitHubOAuthConfig { - private static Logger logger = LoggerFactory.getLogger(GitHubOAuthConfig.class); - private final Config config; private final CanonicalWebUrl canonicalWebUrl; @@ -138,25 +136,16 @@ config.getSubsections(CONF_KEY_SECTION).stream() .map(KeyConfig::new) .collect(Collectors.toMap(KeyConfig::getKeyId, Function.identity())); - - if (configuredKeyConfig.isEmpty()) { - logger.warn( - "No configured '{}' sections found. Default configuration should NOT be used in production code.", - CONF_KEY_SECTION); - currentKeyConfig = new KeyConfig(); - keyConfigMap = Map.of(currentKeyConfig.getKeyId(), currentKeyConfig); - } else { - keyConfigMap = configuredKeyConfig; - List<KeyConfig> currentKeyConfigs = - keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList()); - if (currentKeyConfigs.size() != 1) { - throw new IllegalStateException( - String.format( - "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found", - CONF_KEY_SECTION, currentKeyConfigs.size())); - } - currentKeyConfig = currentKeyConfigs.get(0); + keyConfigMap = configuredKeyConfig; + List<KeyConfig> currentKeyConfigs = + keyConfigMap.values().stream().filter(KeyConfig::isCurrent).collect(Collectors.toList()); + if (currentKeyConfigs.size() != 1) { + throw new IllegalStateException( + String.format( + "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found", + CONF_KEY_SECTION, currentKeyConfigs.size())); } + currentKeyConfig = currentKeyConfigs.get(0); } public String getOAuthFinalRedirectUrl(HttpServletRequest req) { @@ -220,7 +209,6 @@ public class KeyConfig { - public static final String PASSWORD_DEVICE_DEFAULT = "/dev/zero"; public static final int PASSWORD_LENGTH_DEFAULT = 16; public static final String CIPHER_ALGORITHM_DEFAULT = "AES/ECB/PKCS5Padding"; public static final String SECRET_KEY_ALGORITHM_DEFAULT = "AES"; @@ -251,11 +239,7 @@ CONF_KEY_SECTION, keyId, KEY_DELIMITER)); } - this.passwordDevice = - trimTrailingSlash( - MoreObjects.firstNonNull( - config.getString(CONF_KEY_SECTION, keyId, PASSWORD_DEVICE_CONFIG_LABEL), - PASSWORD_DEVICE_DEFAULT)); + this.passwordDevice = trimTrailingSlash(getPasswordDeviceOrThrow(config, keyId)); this.passwordLength = config.getInt( CONF_KEY_SECTION, keyId, PASSWORD_LENGTH_CONFIG_LABEL, PASSWORD_LENGTH_DEFAULT); @@ -274,15 +258,6 @@ this.keyId = keyId; } - private KeyConfig() { - passwordDevice = PASSWORD_DEVICE_DEFAULT; - passwordLength = PASSWORD_LENGTH_DEFAULT; - isCurrent = true; - cipherAlgorithm = CIPHER_ALGORITHM_DEFAULT; - keyId = KEY_ID_DEFAULT; - secretKeyAlgorithm = SECRET_KEY_ALGORITHM_DEFAULT; - } - public byte[] readPassword() throws IOException { Path devicePath = Paths.get(passwordDevice); try (FileInputStream in = new FileInputStream(devicePath.toFile())) { @@ -311,4 +286,22 @@ return keyId; } } + + /** + * Method returns the password device value for a given {@code keyId}. + * + * @throws {@link IllegalStateException} when password device is not configured for {@code keyId} + */ + private static String getPasswordDeviceOrThrow(Config config, String keyId) { + String passwordDevice = + config.getString(CONF_KEY_SECTION, keyId, KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL); + if (Strings.isNullOrEmpty(passwordDevice)) { + throw new IllegalStateException( + String.format( + "Configuration error. Missing %s.%s for key-id '%s'", + CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, keyId)); + } + + return passwordDevice; + } }
diff --git a/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java new file mode 100644 index 0000000..2f36425 --- /dev/null +++ b/github-oauth/src/main/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGenerator.java
@@ -0,0 +1,99 @@ +// Copyright (C) 2022 The Android Open Source Project +// +// Licensed 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 +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// 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 com.googlesource.gerrit.plugins.github.oauth; + +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT; +import static java.util.Objects.requireNonNull; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.SecureRandom; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class PasswordGenerator { + private static final Logger logger = LoggerFactory.getLogger(PasswordGenerator.class); + public static final String DEFAULT_PASSWORD_FILE = "default.key"; + + /** + * Generates default password and stores under given {@code Path}. Note that if password already + * exists it is not regenerated. + * + * @param passwordFilePath path that should contain the default password; cannot be {@code null} + * @throws {@link IllegalStateException} when file denoted by given {@code Path} is a directory, + * cannot be read, has invalid length or doesn't exist and cannot be created + * @return {@code true} if password was generated, {@code false} if it already exists + */ + public boolean generate(Path passwordFilePath) { + requireNonNull(passwordFilePath); + + File passwordFile = passwordFilePath.toFile(); + + if (passwordFile.isDirectory()) { + throw logErrorAndCreateRuntimeException( + "'%s' is directory whilst a regular file was expected.", passwordFilePath); + } + + if (passwordFile.isFile()) { + if (!passwordFile.canRead()) { + throw logErrorAndCreateRuntimeException( + "'%s' password file exists, but cannot be read.", passwordFilePath); + } + + long length = passwordFile.length(); + if (length != PASSWORD_LENGTH_DEFAULT) { + throw logErrorAndCreateRuntimeException( + "'%s' password file exists but has an invalid length of %d bytes. The expected length is %d bytes.", + passwordFilePath, length, PASSWORD_LENGTH_DEFAULT); + } + return false; + } + + byte[] token = generateToken(); + try { + Files.write(passwordFilePath, token); + logger.info("Password was stored in {} file", passwordFilePath); + return true; + } catch (IOException e) { + throw logErrorAndCreateRuntimeException(e, "Password generation has failed"); + } + } + + private byte[] generateToken() { + SecureRandom random = new SecureRandom(); + byte[] token = new byte[PASSWORD_LENGTH_DEFAULT]; + random.nextBytes(token); + return token; + } + + private IllegalStateException logErrorAndCreateRuntimeException( + String msg, Object... parameters) { + return logErrorAndCreateRuntimeException(null, msg, parameters); + } + + private IllegalStateException logErrorAndCreateRuntimeException( + Exception e, String msg, Object... parameters) { + String log = String.format(msg, parameters); + if (e != null) { + logger.error(log, e); + return new IllegalStateException(log, e); + } + + logger.error(log); + return new IllegalStateException(log); + } +}
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java index 94d1a43..a5bf767 100644 --- a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java +++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/GitHubOAuthConfigTest.java
@@ -15,12 +15,10 @@ import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_KEY_SECTION; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_SECTION; -import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGORITHM_DEFAULT; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_DELIMITER; -import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT; -import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_ALGORITHM_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; @@ -38,6 +36,7 @@ CanonicalWebUrl canonicalWebUrl; Config config; + private static final String testPasswordDevice = "/dev/zero"; @Before public void setUp() { @@ -61,28 +60,13 @@ } @Test - public void shouldDefaultKeyConfigWhenNoSpecificConfigurationIsSet() { - GitHubOAuthConfig objectUnderTest = objectUnderTest(); - assertEquals(objectUnderTest.getCurrentKeyConfig().isCurrent(), true); - assertEquals( - objectUnderTest.getCurrentKeyConfig().getCipherAlgorithm(), CIPHER_ALGORITHM_DEFAULT); - assertEquals( - objectUnderTest.getCurrentKeyConfig().getSecretKeyAlgorithm(), - SECRET_KEY_ALGORITHM_DEFAULT); - assertEquals(objectUnderTest.getCurrentKeyConfig().getKeyId(), KEY_ID_DEFAULT); - } - - @Test - public void shouldDBeAbleToLookupDefaultKeyWhenNoSpecificConfigurationIsSet() { - assertEquals(objectUnderTest().getKeyConfig(KEY_ID_DEFAULT).isCurrent(), true); - } - - @Test public void shouldReadASpecificKeyConfig() { String keySubsection = "someKeyConfig"; String cipherAlgorithm = "AES/CFB8/NoPadding"; String secretKeyAlgorithm = "DES"; config.setBoolean(CONF_KEY_SECTION, keySubsection, CURRENT_CONFIG_LABEL, true); + config.setString( + CONF_KEY_SECTION, keySubsection, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); config.setString(CONF_KEY_SECTION, keySubsection, CIPHER_ALGO_CONFIG_LABEL, cipherAlgorithm); config.setString(CONF_KEY_SECTION, keySubsection, SECRET_KEY_CONFIG_LABEL, secretKeyAlgorithm); @@ -96,10 +80,16 @@ @Test public void shouldReturnTheExpectedKeyConfigAsCurrent() { - config.setBoolean(CONF_KEY_SECTION, "currentKeyConfig", CURRENT_CONFIG_LABEL, true); - config.setBoolean(CONF_KEY_SECTION, "someOtherKeyConfig", CURRENT_CONFIG_LABEL, false); + String currentKeyConfig = "currentKeyConfig"; + String someOtherKeyConfig = "someOtherKeyConfig"; + config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true); + config.setString( + CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); + config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false); + config.setString( + CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); - assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), "currentKeyConfig"); + assertEquals(objectUnderTest().getCurrentKeyConfig().getKeyId(), currentKeyConfig); } @Test @@ -107,7 +97,11 @@ String currentKeyConfig = "currentKeyConfig"; String someOtherKeyConfig = "someOtherKeyConfig"; config.setBoolean(CONF_KEY_SECTION, currentKeyConfig, CURRENT_CONFIG_LABEL, true); + config.setString( + CONF_KEY_SECTION, currentKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); config.setBoolean(CONF_KEY_SECTION, someOtherKeyConfig, CURRENT_CONFIG_LABEL, false); + config.setString( + CONF_KEY_SECTION, someOtherKeyConfig, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); GitHubOAuthConfig objectUnderTest = objectUnderTest(); @@ -116,6 +110,18 @@ } @Test + public void shouldThrowWhenNoKeyIdIsConfigured() { + IllegalStateException illegalStateException = + assertThrows(IllegalStateException.class, this::objectUnderTest); + + assertEquals( + illegalStateException.getMessage(), + String.format( + "Expected exactly 1 subsection of '%s' to be configured as 'current', %d found", + CONF_KEY_SECTION, 0)); + } + + @Test public void shouldThrowWhenNoKeyConfigIsSetAsCurrent() { config.setBoolean(CONF_KEY_SECTION, "someKeyConfig", CURRENT_CONFIG_LABEL, false); @@ -145,6 +151,21 @@ assertThrows(IllegalStateException.class, this::objectUnderTest); } + @Test + public void shouldThrowWhenKeyIdMissesPasswordDevice() { + String someKeyConfig = "someKeyConfig"; + config.setBoolean(CONF_KEY_SECTION, someKeyConfig, CURRENT_CONFIG_LABEL, true); + + IllegalStateException illegalStateException = + assertThrows(IllegalStateException.class, this::objectUnderTest); + + assertEquals( + String.format( + "Configuration error. Missing %s.%s for key-id '%s'", + CONF_KEY_SECTION, PASSWORD_DEVICE_CONFIG_LABEL, someKeyConfig), + illegalStateException.getMessage()); + } + private GitHubOAuthConfig objectUnderTest() { return new GitHubOAuthConfig(config, canonicalWebUrl); }
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java index 9621fc5..3e31d6b 100644 --- a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java +++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/OAuthTokenCipherTest.java
@@ -17,6 +17,7 @@ import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_SECTION; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL; import static com.googlesource.gerrit.plugins.github.oauth.OAuthTokenCipher.splitKeyIdFromMaterial; @@ -31,32 +32,38 @@ import com.google.inject.util.Providers; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import java.util.Base64; import java.util.List; import org.eclipse.jgit.lib.Config; import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; public class OAuthTokenCipherTest { CanonicalWebUrl canonicalWebUrl; Config config; + @ClassRule public static TemporaryFolder temporaryFolder = new TemporaryFolder(); + private static final String VERSION1_KEY_ID = "version1"; private static final String VERSION2_KEY_ID = "version2"; @Before public void setUp() { - config = new Config(); - - config.setString(CONF_SECTION, null, "clientSecret", "theSecret"); - config.setString(CONF_SECTION, null, "clientId", "theClientId"); - config.setString("auth", null, "httpHeader", "GITHUB_USER"); - config.setString("auth", null, "type", AuthType.HTTP.toString()); + config = createCommonConfig(); config.setBoolean(CONF_KEY_SECTION, VERSION1_KEY_ID, CURRENT_CONFIG_LABEL, true); config.setBoolean(CONF_KEY_SECTION, VERSION2_KEY_ID, CURRENT_CONFIG_LABEL, false); + String testPasswordDevice = "/dev/zero"; + config.setString( + CONF_KEY_SECTION, VERSION1_KEY_ID, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); + config.setString( + CONF_KEY_SECTION, VERSION2_KEY_ID, PASSWORD_DEVICE_CONFIG_LABEL, testPasswordDevice); + canonicalWebUrl = Guice.createInjector( new AbstractModule() { @@ -71,10 +78,32 @@ } @Test - public void shouldEncryptAndDecryptAToken() throws IOException { - String someOauthToken = "someToken"; - OAuthTokenCipher objectUnderTest = objectUnderTest(); + public void shouldEncryptAndDecryptATokenWithPasswordGeneratedAtInit() throws IOException { + // simulate plugin init step by generating a password to a file and configuring it in + // gerrit.config + Path passwordFilePath = + temporaryFolder.newFolder().toPath().resolve(PasswordGenerator.DEFAULT_PASSWORD_FILE); + new PasswordGenerator().generate(passwordFilePath); + config = createCommonConfig(); + config.setBoolean(CONF_KEY_SECTION, KEY_ID_DEFAULT, CURRENT_CONFIG_LABEL, true); + config.setString( + CONF_KEY_SECTION, + KEY_ID_DEFAULT, + PASSWORD_DEVICE_CONFIG_LABEL, + passwordFilePath.toString()); + + verifyTokenEncryptionAndDecryption(objectUnderTest()); + } + + @Test + public void shouldEncryptAndDecryptAToken() throws IOException { + verifyTokenEncryptionAndDecryption(objectUnderTest()); + } + + private void verifyTokenEncryptionAndDecryption(OAuthTokenCipher objectUnderTest) + throws CipherException { + String someOauthToken = "someToken"; String encrypt = objectUnderTest.encrypt(someOauthToken); assertNotEquals(encrypt, someOauthToken); assertEquals(objectUnderTest.decrypt(encrypt), someOauthToken); @@ -160,6 +189,19 @@ } private OAuthTokenCipher objectUnderTest() throws IOException { - return new OAuthTokenCipher(new GitHubOAuthConfig(config, canonicalWebUrl)); + return objectUnderTest(config); + } + + private OAuthTokenCipher objectUnderTest(Config testConfig) throws IOException { + return new OAuthTokenCipher(new GitHubOAuthConfig(testConfig, canonicalWebUrl)); + } + + private static Config createCommonConfig() { + Config config = new Config(); + config.setString(CONF_SECTION, null, "clientSecret", "theSecret"); + config.setString(CONF_SECTION, null, "clientId", "theClientId"); + config.setString("auth", null, "httpHeader", "GITHUB_USER"); + config.setString("auth", null, "type", AuthType.HTTP.toString()); + return config; } }
diff --git a/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java new file mode 100644 index 0000000..0fe37a8 --- /dev/null +++ b/github-oauth/src/test/java/com/googlesource/gerrit/plugins/github/oauth/PasswordGeneratorTest.java
@@ -0,0 +1,92 @@ +// Copyright (C) 2022 The Android Open Source Project +// +// Licensed 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 +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// 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 com.googlesource.gerrit.plugins.github.oauth; + +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +public class PasswordGeneratorTest { + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + private Path passwordPath; + private final PasswordGenerator objectUnderTest = new PasswordGenerator(); + + @Before + public void setup() throws IOException { + passwordPath = + temporaryFolder.newFolder().toPath().resolve(PasswordGenerator.DEFAULT_PASSWORD_FILE); + } + + @Test + public void shouldGenerateKeyFileWithPasswordDefaultLength() throws IOException { + assertTrue(objectUnderTest.generate(passwordPath)); + assertTrue(Files.isRegularFile(passwordPath)); + + byte[] token = Files.readAllBytes(passwordPath); + assertEquals( + String.format( + "Generated password length doesn't equal to expected %d", PASSWORD_LENGTH_DEFAULT), + token.length, + PASSWORD_LENGTH_DEFAULT); + } + + @Test + public void shouldNotGenerateNewDefaultKeyIfOneAlreadyExistAndIsNotEmpty() throws IOException { + assertTrue(objectUnderTest.generate(passwordPath)); + byte[] expected = Files.readAllBytes(passwordPath); + assertFalse(objectUnderTest.generate(passwordPath)); + byte[] token = Files.readAllBytes(passwordPath); + assertArrayEquals("Existing password file was overwritten", expected, token); + } + + @Test + public void shouldGenerateDifferentContentForDifferentSites() throws IOException { + assertTrue(objectUnderTest.generate(passwordPath)); + byte[] siteA = Files.readAllBytes(passwordPath); + + assertTrue(passwordPath.toFile().delete()); + assertTrue(objectUnderTest.generate(passwordPath)); + byte[] siteB = Files.readAllBytes(passwordPath); + assertFalse( + "The same password was generated for two different sites", Arrays.equals(siteA, siteB)); + } + + @Test + public void shouldThrowIllegalStateExceptionWhenDefaultKeyIsDirectory() throws IOException { + // create dir from passwordPath + assertTrue(passwordPath.toFile().mkdir()); + + IllegalStateException illegalStateException = + assertThrows(IllegalStateException.class, () -> objectUnderTest.generate(passwordPath)); + + assertTrue( + illegalStateException + .getMessage() + .endsWith("is directory whilst a regular file was expected.")); + } +}
diff --git a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java index 828613e..894e1a5 100644 --- a/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java +++ b/github-plugin/src/main/java/com/googlesource/gerrit/plugins/github/InitGitHub.java
@@ -13,15 +13,35 @@ // limitations under the License. package com.googlesource.gerrit.plugins.github; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.CONF_KEY_SECTION; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGORITHM_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CIPHER_ALGO_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.CURRENT_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.IS_CURRENT_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.KEY_ID_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_DEVICE_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.PASSWORD_LENGTH_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_ALGORITHM_DEFAULT; +import static com.googlesource.gerrit.plugins.github.oauth.GitHubOAuthConfig.KeyConfig.SECRET_KEY_CONFIG_LABEL; +import static com.googlesource.gerrit.plugins.github.oauth.PasswordGenerator.DEFAULT_PASSWORD_FILE; + import com.google.common.base.Strings; +import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.client.AuthType; import com.google.gerrit.pgm.init.api.ConsoleUI; +import com.google.gerrit.pgm.init.api.InitFlags; import com.google.gerrit.pgm.init.api.InitStep; import com.google.gerrit.pgm.init.api.InitUtil; import com.google.gerrit.pgm.init.api.Section; +import com.google.gerrit.server.config.SitePaths; import com.google.inject.Inject; +import com.googlesource.gerrit.plugins.github.oauth.PasswordGenerator; import java.net.URISyntaxException; +import java.nio.file.Path; import java.util.EnumSet; +import java.util.Optional; +import org.eclipse.jgit.storage.file.FileBasedConfig; public class InitGitHub implements InitStep { private static final String GITHUB_URL = "https://github.com"; @@ -29,15 +49,26 @@ private static final String GITHUB_REGISTER_APPLICATION_PATH = "/settings/applications/new"; private static final String GERRIT_OAUTH_CALLBACK_PATH = "oauth"; + private final Path pluginData; private final ConsoleUI ui; private final Section auth; private final Section httpd; private final Section github; private final Section gerrit; + private final Section.Factory sections; + private final FileBasedConfig cfg; @Inject - InitGitHub(final ConsoleUI ui, final Section.Factory sections) { + InitGitHub( + @PluginName String pluginName, + SitePaths site, + final ConsoleUI ui, + final Section.Factory sections, + InitFlags flags) { + this.pluginData = site.data_dir.resolve(pluginName); this.ui = ui; + this.sections = sections; + this.cfg = flags.cfg; this.github = sections.get("github", null); this.httpd = sections.get("httpd", null); this.auth = sections.get("auth", null); @@ -85,6 +116,87 @@ httpd.unset("filterClass"); httpd.unset("httpHeader"); } + + setupGitHubOAuthTokenCipher(); + } + + private void setupGitHubOAuthTokenCipher() { + ui.header("GitHub OAuth token cipher configuration"); + Optional<String> maybeCurrentKeyId = getCurrentKeyId(); + + boolean configureNewPasswordDevice = false; + if (maybeCurrentKeyId.isPresent()) { + if (!ui.yesno( + false, + "Current GitHub OAuth token cipher is configured under the %s key id. Do you want to configure a new one?", + maybeCurrentKeyId.get())) { + return; + } + + String notUniqueKeyIdPrefix = "K"; + do { + String newKeyId = ui.readString("some-key-id", "%sey identifier", notUniqueKeyIdPrefix); + if (cfg.getSubsections(CONF_KEY_SECTION).contains(newKeyId)) { + notUniqueKeyIdPrefix = + String.format( + "Provided key id '%s' already exists. Please provide a different key.", newKeyId); + continue; + } + + cfg.setBoolean(CONF_KEY_SECTION, maybeCurrentKeyId.get(), CURRENT_CONFIG_LABEL, false); + maybeCurrentKeyId = Optional.of(newKeyId); + configureNewPasswordDevice = true; + break; + } while (true); + } + + String currentKeyId = maybeCurrentKeyId.orElse(KEY_ID_DEFAULT); + ui.message("Configuring GitHub OAuth token cipher under '%s' key id\n", currentKeyId); + Section gitHubKey = sections.get(CONF_KEY_SECTION, currentKeyId); + + gitHubKey.set(CURRENT_CONFIG_LABEL, "true"); + + Path defaultPasswordPath = pluginData.resolve(DEFAULT_PASSWORD_FILE); + String currentPasswordPath = + gitHubKey.string( + "Password file or device", + PASSWORD_DEVICE_CONFIG_LABEL, + configureNewPasswordDevice ? null : defaultPasswordPath.toString()); + if (defaultPasswordPath.toString().equalsIgnoreCase(currentPasswordPath)) { + pluginData.toFile().mkdirs(); + if (new PasswordGenerator().generate(Path.of(currentPasswordPath))) { + ui.message( + "New password (%d bytes long) was generated under '%s' file.\n", + PASSWORD_LENGTH_DEFAULT, currentPasswordPath); + } else { + ui.message( + "The file under '%s' path already exists. Password wasn't regenerated.\n", + currentPasswordPath); + } + } else { + // ask for length only if default password is not used + gitHubKey.set( + PASSWORD_LENGTH_CONFIG_LABEL, + String.valueOf(ui.readInt(PASSWORD_LENGTH_DEFAULT, "Password length in bytes"))); + } + + gitHubKey.string( + "The algorithm to be used to encrypt the provided password", + SECRET_KEY_CONFIG_LABEL, + SECRET_KEY_ALGORITHM_DEFAULT); + + gitHubKey.string( + "The algorithm to be used for encryption/decryption", + CIPHER_ALGO_CONFIG_LABEL, + CIPHER_ALGORITHM_DEFAULT); + } + + private Optional<String> getCurrentKeyId() { + return cfg.getSubsections(CONF_KEY_SECTION).stream() + .filter( + keyId -> + cfg.getBoolean(CONF_KEY_SECTION, keyId, CURRENT_CONFIG_LABEL, IS_CURRENT_DEFAULT)) + .findFirst(); } private void authSetDefault(String key, String defValue) {
diff --git a/github-plugin/src/main/resources/Documentation/config.md b/github-plugin/src/main/resources/Documentation/config.md index ea32890..e2ea0a3 100644 --- a/github-plugin/src/main/resources/Documentation/config.md +++ b/github-plugin/src/main/resources/Documentation/config.md
@@ -7,7 +7,8 @@ library to work properly. GitHub OAuth library rely on Gerrit HTTP authentication defined during the standard -Gerrit init steps. +Gerrit init steps. It also requires GitHub OAuth token cipher configuration +(details `Key configuration` section) but provides convenient defaults for it. See below a sample session of relevant init steps for a default configuration pointing to the Web GitHub instance: @@ -36,6 +37,15 @@ ClientSecret []: f82c3f9b3802666f2adcc4c8cacfb164295b0a99 confirm password : HTTP Authentication Header [GITHUB_USER]: + + *** GitHub OAuth token cipher configuration + *** + + Configuring GitHub OAuth token cipher under 'current' key id + Password file or device [gerrit/data/github-plugin/default.key]: + New password (16 bytes long) was generated under 'gerrit/data/github-plugin/default.key' file. + The algorithm to be used to encrypt the provided password [AES]: + The algorithm to be used for encryption/decryption [AES/ECB/PKCS5Padding]: ``` Configuration @@ -89,15 +99,14 @@ ------------- Since this plugin obtains credentials from Github and persists them in Gerrit, -it also takes care of encrypting them at rest. The Gerrit admin can configure -how this is done by setting the relevant configuration parameters. +it also takes care of encrypting them at rest. Encryption configuration is a +mandatory step performed during the plugin init (that also provides convenient +defaults). The Gerrit admin can introduce its own cipher configuration +(already in init step) by setting the following parameters. github-key.<key-id>.passwordDevice : The device or file where to retrieve the encryption passphrase.\ -Default: /dev/zero - -*NOTE*: such configuration is considered insecure and should *not be used in -production*, always set a non-zero password device for deriving the key. +This is a required parameter for `key-id` configuration. github-key.<key-id>.passwordLength : The length in bytes of the password read from the passwordDevice.\ @@ -111,7 +120,7 @@ Default: AES/ECB/PKCS5Padding github-key.<key-id>.secretKeyAlgorithm -: the algorithm to be used to encrypt the provided password. Available +: The algorithm to be used to encrypt the provided password. Available algorithms are described in the [Cipher section](https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#cipher-algorithm-names) of the Java Cryptography Architecture Standard Algorithm Name Documentation.\ @@ -129,14 +138,12 @@ in the configuration. New credentials will always be encrypted with the `current` `<key-id>`. -Note that unencrypted oauth tokens will be handled gracefully and just passed -through to github by the decryption algorithm. This is done so that oauth tokens -that were persisted _before_ the encryption feature was implemented will still -be considered valid until their natural expiration time. +*Notes:* +Unencrypted oauth tokens will be handled gracefully and just passed through to +github by the decryption algorithm. This is done so that oauth tokens that were +persisted _before_ the encryption feature was implemented will still be +considered valid until their natural expiration time. -If no `github-key.<key-id>` exists in configuration, then a default current key -configuration -(named `current`) will be inferred, using the defaults documented above. - -*NOTE* such configuration is considered insecure and should *not be used in -production*. \ No newline at end of file +Plugin will not start if no `github-key.<key-id>` section, marked as current, +exists in configuration. One needs to either configure it manually or call init +for a default configuration to be created.