opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-3.0' into stable-3.1

Browse Source 
* stable-3.0:
  Restore existing Base64 transcoder for private key
  Remove unused annotation @ChangeUpdateExecutor

Change-Id: I911b54160e966ebc2a18e9762c494e2759aa7396
This commit is contained in:
Luca Milanesio
2020-05-01 14:22:58 +01:00
parent 584ce3bf29 b633dfc671
commit 46c0d503e9
5 changed files with 36 additions and 93 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
java/com/google/gerrit/server/config/ChangeUpdateExecutor.java
View File

@@ -1,29 +0,0 @@
// Copyright (C) 2012 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.google.gerrit.server.config;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.inject.BindingAnnotation;
import java.lang.annotation.Retention;
/**
* Marker on the global {@link ListeningExecutorService} used by asynchronous {@link BatchUpdate}s.
*/
@Retention(RUNTIME)
@BindingAnnotation
public @interface ChangeUpdateExecutor {}

30
java/com/google/gerrit/server/config/SysExecutorModule.java
View File

@@ -14,19 +14,13 @@
package com.google.gerrit.server.config;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.server.FanOutExecutor;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.logging.LoggingContextAwareExecutorService;
import com.google.inject.AbstractModule;
import com.google.inject.Provides;
import com.google.inject.Singleton;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.lib.Config;
/**
@@ -73,28 +67,4 @@ public class SysExecutorModule extends AbstractModule {
}
return queues.createQueue(poolSize, "FanOut");
}
@Provides
@Singleton
@ChangeUpdateExecutor
public ListeningExecutorService createChangeUpdateExecutor(@GerritServerConfig Config config) {
int poolSize = config.getInt("receive", null, "changeUpdateThreads", 1);
if (poolSize <= 1) {
return MoreExecutors.newDirectExecutorService();
}
return MoreExecutors.listeningDecorator(
new LoggingContextAwareExecutorService(
MoreExecutors.getExitingExecutorService(
new ThreadPoolExecutor(
1,
poolSize,
10,
TimeUnit.MINUTES,
new ArrayBlockingQueue<>(poolSize),
new ThreadFactoryBuilder()
.setNameFormat("ChangeUpdate-%d")
.setDaemon(true)
.build(),
new ThreadPoolExecutor.CallerRunsPolicy()))));
}
}

21
java/com/google/gerrit/server/mail/SignedToken.java
View File

@@ -22,6 +22,7 @@ import java.util.Arrays;
import javax.crypto.Mac;
import javax.crypto.ShortBufferException;
import javax.crypto.spec.SecretKeySpec;
import org.apache.commons.codec.binary.Base64;
/**
* Utility function to compute and verify XSRF tokens.
@@ -46,7 +47,7 @@ public class SignedToken {
public static String generateRandomKey() {
final byte[] r = new byte[26];
new SecureRandom().nextBytes(r);
return encodeBase64(r);
return encodeBase64PrivateKey(r);
}
private final int maxAge;
@@ -63,7 +64,7 @@ public class SignedToken {
*/
public SignedToken(final int age, final String keyBase64) throws XsrfException {
maxAge = age > 5 ? age / 5 : age;
key = new SecretKeySpec(decodeBase64(keyBase64), MAC_ALG);
key = new SecretKeySpec(decodeBase64PrivateKey(keyBase64), MAC_ALG);
rng = new SecureRandom();
tokenLength = 2 * INT_SZ + newMac().getMacLength();
}
@@ -169,6 +170,14 @@ public class SignedToken {
return (int) (System.currentTimeMillis() / 5000L);
}
private static byte[] decodeBase64PrivateKey(final String privateKeyBase64String) {
return Base64.decodeBase64(toBytes(privateKeyBase64String));
}
private static String encodeBase64PrivateKey(final byte[] buf) {
return toString(Base64.encodeBase64(buf));
}
private static byte[] decodeBase64(final String s) {
return BaseEncoding.base64Url().decode(s);
}
@@ -208,4 +217,12 @@ public class SignedToken {
}
return r;
}
private static String toString(final byte[] b) {
final StringBuilder r = new StringBuilder(b.length);
for (int i = 0; i < b.length; i++) {
r.append((char) b[i]);
}
return r.toString();
}
}

5
java/com/google/gerrit/testing/InMemoryModule.java
View File

@@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkState;
import static com.google.inject.Scopes.SINGLETON;
import com.google.common.base.Strings;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
@@ -48,7 +47,6 @@ import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.CanonicalWebUrlModule;
import com.google.gerrit.server.config.CanonicalWebUrlProvider;
import com.google.gerrit.server.config.ChangeUpdateExecutor;
import com.google.gerrit.server.config.DefaultUrlFormatter;
import com.google.gerrit.server.config.GerritGlobalModule;
import com.google.gerrit.server.config.GerritInstanceNameModule;
@@ -186,9 +184,6 @@ public class InMemoryModule extends FactoryModule {
bind(GitRepositoryManager.class).to(InMemoryRepositoryManager.class);
bind(InMemoryRepositoryManager.class).in(SINGLETON);
bind(TrackingFooters.class).toProvider(TrackingFootersProvider.class).in(SINGLETON);
bind(ListeningExecutorService.class)
.annotatedWith(ChangeUpdateExecutor.class)
.toInstance(MoreExecutors.newDirectExecutorService());
bind(SecureStore.class).to(DefaultSecureStore.class);
install(new InMemorySchemaModule());

44
javatests/com/google/gerrit/server/mail/SignedTokenTest.java
View File

@@ -18,20 +18,14 @@ import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import java.util.Random;
import java.util.regex.Pattern;
import org.junit.Before;
import org.junit.Test;
public class SignedTokenTest {
private static final String REGISTER_EMAIL_PRIVATE_KEY =
"R2Vycml0JTIwcmVnaXN0ZXJFbWFpbFByaXZhdGVLZXk=";
private static final String URL_SAFE_REGISTER_EMAIL_PRIVATE_KEY =
REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R2", "_-");
private static final String URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_PLUS =
REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R", "+");
private static final String URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_SLASH =
REGISTER_EMAIL_PRIVATE_KEY.replaceFirst("R", "/");
private static final Pattern URL_UNSAFE_CHARS = Pattern.compile("(\\+|/)");
private static final String REGISTER_EMAIL_PRIVATE_KEY = "TGMv3/bTC42jUKQndTQrXyHhHYMP0t69i/4=";
private static final int maxAge = 5;
private static final String TEXT = "This is a text";
private static final String FORGED_TEXT = "This is a forged text";
@@ -44,29 +38,23 @@ public class SignedTokenTest {
signedToken = new SignedToken(maxAge, REGISTER_EMAIL_PRIVATE_KEY);
}
/**
* Test new token: the key is a normal BASE64 string without index of '62'(+ or _) or '63'(/ or -)
*/
/** Test new token: the key is a normal BASE64 string that can be used for URL safely */
@Test
public void newTokenKeyDoesNotContainUnsafeChar() throws Exception {
new SignedToken(maxAge, REGISTER_EMAIL_PRIVATE_KEY);
}
/** Test new token: the key is an URL safe BASE64 string with indexes of '62'(_) and '63'(-) */
@Test
public void newTokenWithUrlSafeBase64() throws Exception {
new SignedToken(maxAge, URL_SAFE_REGISTER_EMAIL_PRIVATE_KEY);
assertThat(signedToken.newToken(TEXT)).doesNotContainMatch(URL_UNSAFE_CHARS);
}
/** Test new token: the key is an URL unsafe BASE64 string with index of '62'(+) */
@Test
public void newTokenWithUrlUnsafeBase64Plus() throws Exception {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> new SignedToken(maxAge, URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_PLUS));
String token = "+" + signedToken.newToken(TEXT);
CheckTokenException thrown =
assertThrows(CheckTokenException.class, () -> signedToken.checkToken(token, TEXT));
assertThat(thrown).hasMessageThat().contains("decoding failed");
assertThat(thrown)
.hasCauseThat()
.hasMessageThat()
.isEqualTo(
"com.google.common.io.BaseEncoding$DecodingException: Unrecognized character: +");
@@ -75,12 +63,14 @@ public class SignedTokenTest {
/** Test new token: the key is an URL unsafe BASE64 string with '63'(/) */
@Test
public void newTokenWithUrlUnsafeBase64Slash() throws Exception {
IllegalArgumentException thrown =
assertThrows(
IllegalArgumentException.class,
() -> new SignedToken(maxAge, URL_UNSAFE_REGISTER_EMAIL_PRIVATE_KEY_WITH_SLASH));
String token = "/" + signedToken.newToken(TEXT);
CheckTokenException thrown =
assertThrows(CheckTokenException.class, () -> signedToken.checkToken(token, TEXT));
assertThat(thrown).hasMessageThat().contains("decoding failed");
assertThat(thrown)
.hasCauseThat()
.hasMessageThat()
.isEqualTo(
"com.google.common.io.BaseEncoding$DecodingException: Unrecognized character: /");