Make BiqueryPollJobAction endpoint internal only (#955)

* Make BiqueryPollJobAction endpoint internal only

This endpoint makes use of java object deserialization, which allows a
malicious actor to craft a request that can initiate overly broad actions on
the server.  Since this endpoint is not widely used for operational purposes,
limit its authorization to "internal only" so that no user agents (even with
admin privs) can access it.
This commit is contained in:
Michael Muller 2021-02-05 07:50:51 -05:00 committed by GitHub
parent 5100057dd5
commit 29bf0f3965
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 60 additions and 8 deletions

View file

@ -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.
*
* <p>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();

View file

@ -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.
*
* <p>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;

View file

@ -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);
}
}

View file

@ -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