diff --git a/core/src/main/java/google/registry/export/BigqueryPollJobAction.java b/core/src/main/java/google/registry/export/BigqueryPollJobAction.java index a5a4aebbf..2877f24b7 100644 --- a/core/src/main/java/google/registry/export/BigqueryPollJobAction.java +++ b/core/src/main/java/google/registry/export/BigqueryPollJobAction.java @@ -45,13 +45,16 @@ import org.joda.time.Duration; /** * An action which polls the state of a bigquery job. If it is completed then it will log its * completion state; otherwise it will return a failure code so that the task will be retried. + * + *
Note that this is AUTH_INTERNAL_ONLY: we don't allow "admin" for this to mitigate a + * vulnerability, see b/177308043. */ @Action( service = Action.Service.BACKEND, path = BigqueryPollJobAction.PATH, method = {Action.Method.GET, Action.Method.POST}, automaticallyPrintOk = true, - auth = Auth.AUTH_INTERNAL_OR_ADMIN) + auth = Auth.AUTH_INTERNAL_ONLY) public class BigqueryPollJobAction implements Runnable { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); diff --git a/core/src/main/java/google/registry/request/auth/Auth.java b/core/src/main/java/google/registry/request/auth/Auth.java index d76dec6c3..30cf2a062 100644 --- a/core/src/main/java/google/registry/request/auth/Auth.java +++ b/core/src/main/java/google/registry/request/auth/Auth.java @@ -65,13 +65,18 @@ public enum Auth { AUTH_PUBLIC_OR_INTERNAL( ImmutableList.of(AuthMethod.INTERNAL, AuthMethod.API), AuthLevel.APP, UserPolicy.PUBLIC), - /** - * Allows only admins or App Engine task-queue access. - */ + /** Allows only admins or App Engine task-queue access. */ AUTH_INTERNAL_OR_ADMIN( - ImmutableList.of(AuthMethod.INTERNAL, AuthMethod.API), - AuthLevel.APP, - UserPolicy.ADMIN); + ImmutableList.of(AuthMethod.INTERNAL, AuthMethod.API), AuthLevel.APP, UserPolicy.ADMIN), + + /** + * Allows only App Engine task-queue access. + * + *
In general, prefer AUTH_INTERNAL_OR_ADMIN. This level of access should be reserved for + * endpoints that have some sensitivity (it was introduced to mitigate a remote-shell + * vulnerability). + */ + AUTH_INTERNAL_ONLY(ImmutableList.of(AuthMethod.INTERNAL), AuthLevel.APP, UserPolicy.IGNORED); private final AuthSettings authSettings; diff --git a/core/src/test/java/google/registry/request/RequestHandlerTest.java b/core/src/test/java/google/registry/request/RequestHandlerTest.java index 6169b0b2e..6fc29cb10 100644 --- a/core/src/test/java/google/registry/request/RequestHandlerTest.java +++ b/core/src/test/java/google/registry/request/RequestHandlerTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.request.Action.Method.GET; import static google.registry.request.Action.Method.POST; +import static google.registry.request.auth.Auth.AUTH_INTERNAL_ONLY; import static google.registry.request.auth.Auth.AUTH_INTERNAL_OR_ADMIN; import static google.registry.request.auth.Auth.AUTH_PUBLIC; import static org.mockito.ArgumentMatchers.any; @@ -140,6 +141,17 @@ public final class RequestHandlerTest { } } + @Action( + service = Action.Service.DEFAULT, + path = "/auth/internal", + auth = AUTH_INTERNAL_ONLY, + method = GET) + public class AuthInternalAction extends AuthBase { + AuthInternalAction(AuthResult authResult) { + super(authResult); + } + } + public class Component { private RequestModule requestModule = null; @@ -179,6 +191,10 @@ public final class RequestHandlerTest { public AuthAdminUserAction authAdminUserAction() { return new AuthAdminUserAction(component.getRequestModule().provideAuthResult()); } + + public AuthInternalAction authInternalAction() { + return new AuthInternalAction(component.getRequestModule().provideAuthResult()); + } } /** Fake Builder for the fake component above to satisfy RequestHandler expectations. */ @@ -462,4 +478,32 @@ public final class RequestHandlerTest { assertThat(providedAuthResult.userAuthInfo().get().oauthTokenInfo()).isEmpty(); assertMetric("/auth/adminUser", GET, AuthLevel.USER, true); } + + @Test + void testInternalAuthNeeded_failure() throws Exception { + when(req.getMethod()).thenReturn("GET"); + when(req.getRequestURI()).thenReturn("/auth/internal"); + when(requestAuthenticator.authorize(AUTH_INTERNAL_ONLY.authSettings(), req)) + .thenReturn(Optional.empty()); + + handler.handleRequest(req, rsp); + + verify(rsp).sendError(403, "Not authorized"); + assertThat(providedAuthResult).isNull(); + } + + @Test + void testInternalAuthNeeded_success() throws Exception { + when(req.getMethod()).thenReturn("GET"); + when(req.getRequestURI()).thenReturn("/auth/internal"); + when(requestAuthenticator.authorize(AUTH_INTERNAL_ONLY.authSettings(), req)) + .thenReturn(Optional.of(AuthResult.create(AuthLevel.APP))); + + handler.handleRequest(req, rsp); + + assertThat(providedAuthResult).isNotNull(); + assertThat(providedAuthResult.authLevel()).isEqualTo(AuthLevel.APP); + assertThat(providedAuthResult.userAuthInfo()).isEmpty(); + assertMetric("/auth/internal", GET, AuthLevel.APP, true); + } } diff --git a/core/src/test/resources/google/registry/module/backend/backend_routing.txt b/core/src/test/resources/google/registry/module/backend/backend_routing.txt index 6ee85fba4..f52e3d54a 100644 --- a/core/src/test/resources/google/registry/module/backend/backend_routing.txt +++ b/core/src/test/resources/google/registry/module/backend/backend_routing.txt @@ -24,7 +24,7 @@ PATH CLASS METHOD /_dr/task/icannReportingUpload IcannReportingUploadAction POST n INTERNAL,API APP ADMIN /_dr/task/nordnUpload NordnUploadAction POST y INTERNAL,API APP ADMIN /_dr/task/nordnVerify NordnVerifyAction POST y INTERNAL,API APP ADMIN -/_dr/task/pollBigqueryJob BigqueryPollJobAction GET,POST y INTERNAL,API APP ADMIN +/_dr/task/pollBigqueryJob BigqueryPollJobAction GET,POST y INTERNAL APP IGNORED /_dr/task/publishDnsUpdates PublishDnsUpdatesAction POST y INTERNAL,API APP ADMIN /_dr/task/publishInvoices PublishInvoicesAction POST n INTERNAL,API APP ADMIN /_dr/task/publishSpec11 PublishSpec11ReportAction POST n INTERNAL,API APP ADMIN