alts: AuthorizationUtil.clientAuthorizationCheck compares the peer service account case-sensitively
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 2/5
- Tiempo estimado
- 1-3 horas
- Aptitud para principiantes
- 85/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Activo
- Stack tecnológico
- java
- Área
- authorization
Línea de trabajo
La comparación se encuentra en clientAuthorizationCheck en alts/src/main/java/io/grpc/alts/AuthorizationUtil.java, que actualmente usa Collection.contains, una comprobación de coincidencia exacta. Lee ese método y el AuthorizationUtilTest existente, y luego añade el caso de prueba de regresión sin distinción de mayúsculas y minúsculas del issue. Está terminado cuando la nueva prueba pasa, las pruebas existentes siguen pasando con ./gradlew :grpc-alts:test y el Javadoc indica que la coincidencia no distingue mayúsculas y minúsculas.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
alts: AuthorizationUtil.clientAuthorizationCheck compares the peer service account case-sensitively
**正文(Markdown 原文)**
`````markdown
### What version of gRPC-Java are you using?
`master` at [`15d64a4`](https://github.com/grpc/grpc-java/commit/15d64a4678450991aa5bf7af23066d36b3391351) (`1.86.0-SNAPSHOT`); `AuthorizationUtil` is unchanged on current `master`.
### What is your environment?
Linux x86_64, OpenJDK 21.0.2, Gradle 8.14.5 (wrapper).
### What did you expect to see?
`AuthorizationUtil.clientAuthorizationCheck(call, expectedServiceAccounts)` accepts a client whose ALTS peer service account differs from an allowed entry only in letter case (for example peer `alice`, allowed `ALICE`), as grpc-go's equivalent `alts.ClientAuthorizationCheck` does.
### What did you see instead?
It returns `PERMISSION_DENIED` ("Client alice is not authorized").
**Location** (`grpc/grpc-java` @ `15d64a4`):
- [`alts/src/main/java/io/grpc/alts/AuthorizationUtil.java:35-47`](https://github.com/grpc/grpc-java/blob/15d64a4678450991aa5bf7af23066d36b3391351/alts/src/main/java/io/grpc/alts/AuthorizationUtil.java#L35-L47)
```java
public static Status clientAuthorizationCheck(
ServerCall<?, ?> call, Collection<String> expectedServiceAccounts) {
...
if (expectedServiceAccounts.contains(altsContext.getPeerServiceAccount())) {
return Status.OK;
}
return Status.PERMISSION_DENIED.withDescription(
"Client " + altsContext.getPeerServiceAccount() + " is not authorized");
}
Problem
Collection.contains() uses String.equals(), so the check is exact-match. Service account identifiers are e-mail style names that people often write in mixed case in config. grpc-go changed its helper for exactly this reason ("expectedServiceAccounts are commonly mixed-case, while the PeerServiceAccount is lower-case"). With the Java helper, an allow-list entry that differs only in case rejects a legitimate client, and the two languages give different answers for the same allow-list. This only concerns letter case; any other difference in the name is still rejected.
Steps to reproduce the bug
Reproduction
Regression test added to AuthorizationUtilTest (reuses its FakeServerCall, which sets the ALTS peer service account):
diff --git a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java
--- a/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java
+++ b/alts/src/test/java/io/grpc/alts/AuthorizationUtilTest.java
@@ -55,6 +55,14 @@ public final class AuthorizationUtilTest {
assertThat(status.getDescription()).endsWith("not authorized");
}
+ @Test
+ public void altsAuthorizationCheck_ignoresCase() throws Exception {
+ Status status =
+ AuthorizationUtil.clientAuthorizationCheck(
+ new FakeServerCall("alice"), Lists.newArrayList("ALICE", "Bob"));
+ assertThat(status.getCode()).isEqualTo(Status.Code.OK);
+ }
+
private static class FakeServerCall extends ServerCall<String, String> {
final Attributes attrs;
./gradlew :grpc-alts:test --tests io.grpc.alts.AuthorizationUtilTest -PskipCodegen=true -PskipAndroid=true
AuthorizationUtilTest > altsAuthorizationCheck_ignoresCase FAILED
value of: getCode()
expected: OK
but was : PERMISSION_DENIED
at app//io.grpc.alts.AuthorizationUtilTest.altsAuthorizationCheck_ignoresCase(AuthorizationUtilTest.java:63)
152 tests completed, 1 failed
Environment:
- Commit:
15d64a4678450991aa5bf7af23066d36b3391351 - Toolchain: OpenJDK 21.0.2, Gradle 8.14.5
Already fixed in grpc/grpc-go
PR: https://github.com/grpc/grpc-go/pull/3792 ("credentials/alts: ClientAuthorizationCheck to case-fold compare of peer SA", merged 2020-09-01T23:23:56Z, merge commit 48bf772d0c2). It switched the peer service account comparison to strings.EqualFold.
How grpc-go fixed it (relevant hunks, verbatim):
--- a/credentials/alts/utils.go
+++ b/credentials/alts/utils.go
@@ -152,12 +152,13 @@ func AuthInfoFromPeer(p *peer.Peer) (AuthInfo, error) {
func ClientAuthorizationCheck(ctx context.Context, expectedServiceAccounts []string) error {
authInfo, err := AuthInfoFromContext(ctx)
if err != nil {
- return status.Newf(codes.PermissionDenied, "The context is not an ALTS-compatible context: %v", err).Err()
+ return status.Errorf(codes.PermissionDenied, "The context is not an ALTS-compatible context: %v", err)
}
+ peer := authInfo.PeerServiceAccount()
for _, sa := range expectedServiceAccounts {
- if authInfo.PeerServiceAccount() == sa {
+ if strings.EqualFold(peer, sa) {
return nil
}
}
- return status.Newf(codes.PermissionDenied, "Client %v is not authorized", authInfo.PeerServiceAccount()).Err()
+ return status.Errorf(codes.PermissionDenied, "Client %v is not authorized", peer)
}
Suggested fix
Compare the peer service account to each expected entry with equalsIgnoreCase, and document it:
diff --git a/alts/src/main/java/io/grpc/alts/AuthorizationUtil.java b/alts/src/main/java/io/grpc/alts/AuthorizationUtil.java
--- a/alts/src/main/java/io/grpc/alts/AuthorizationUtil.java
+++ b/alts/src/main/java/io/grpc/alts/AuthorizationUtil.java
@@ -29,8 +29,8 @@ public final class AuthorizationUtil {
/**
* Given a server call, performs client authorization check, i.e., checks if the client service
- * account matches one of the expected service accounts. It returns OK if client is authorized and
- * an error otherwise.
+ * account matches one of the expected service accounts, ignoring case. It returns OK if client
+ * is authorized and an error otherwise.
*/
public static Status clientAuthorizationCheck(
ServerCall<?, ?> call, Collection<String> expectedServiceAccounts) {
@@ -39,10 +39,13 @@ public final class AuthorizationUtil {
if (altsContext == null) {
return Status.PERMISSION_DENIED.withDescription("Peer ALTS AuthContext not found");
}
- if (expectedServiceAccounts.contains(altsContext.getPeerServiceAccount())) {
- return Status.OK;
+ String peerServiceAccount = altsContext.getPeerServiceAccount();
+ for (String serviceAccount : expectedServiceAccounts) {
+ if (peerServiceAccount.equalsIgnoreCase(serviceAccount)) {
+ return Status.OK;
+ }
}
return Status.PERMISSION_DENIED.withDescription(
- "Client " + altsContext.getPeerServiceAccount() + " is not authorized");
+ "Client " + peerServiceAccount + " is not authorized");
}
}
Verified on 15d64a4 (OpenJDK 21.0.2, Gradle 8.14.5): the test above fails without this patch and passes with it; no other test results change (:grpc-alts:test).
I can submit a PR for this.
Checked: searches for clientAuthorizationCheck case, ALTS service account case-insensitive and AuthorizationUtil found only #11042 and #7840 (ALTS context API changes), not this comparison.
- Lenguaje dominante
- Java
- Estrellas
- 12.1k
- Forks
- 4k
- Merge medio
- 1 d 13 h
- PR fusionados (30 d)
- 42
Preparar el entorno
- Sin Dockerfile ni archivo de Docker Compose
- Sin plantilla de pull request
- Leer la guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de grpc/grpc-java
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
Los mantenedores suelen responder en 1 día
-
xds: EDS resource is not NACKed when locality weights at one priority sum to more than uint32 maxAbierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
Los mantenedores suelen responder en 1 día
-
okhttp: OkHttpServerTransport logs a WARNING with EOFException for every TCP connection that closes before sending the HTTP/2 preface (e.g. load balancer health checks)Posiblemente ocupada Un pull request vinculado a esta issue está abierto o ya se fusionó. Abierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
xds: RBAC `Authenticated` principal without `principal_name` is rejected ("Unknown StringMatcher match pattern: MATCHPATTERN_NOT_SET")Posiblemente ocupada Un pull request vinculado a esta issue está abierto o ya se fusionó. Abiertobug
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
grpc/grpc-java#13150 · 1 comentario ·
Los mantenedores suelen responder en 1 día
Todos los issues de grpc/grpc-java
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 66/100
a2aproject/a2a-java#1275 ·
Los mantenedores suelen responder en 1 día
-
module/ftp Type/Bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
ballerina-platform/ballerina-library#9476 ·
Los mantenedores suelen responder en 1 día
-
enhancement
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
anwilli5/coin-collection-android-US#515 ·
Los mantenedores suelen responder en 1 día
-
status: waiting-for-triage type: bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
spring-projects/spring-integration#11654 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 Menos de una hora Aptitud para principiantes 70/100
Vakore/ZappierGames#248 ·