snoopdave commented on code in PR #189:
URL: https://github.com/apache/roller/pull/189#discussion_r4041933638
##########
app/src/main/java/org/apache/roller/weblogger/ui/core/filters/BootstrapSecurityFilter.java:
##########
@@ -0,0 +1,29 @@
+package org.apache.roller.weblogger.ui.core.filters;
+
+import java.io.IOException;
+import javax.servlet.*;
+import javax.servlet.http.*;
+import org.apache.roller.weblogger.business.WebloggerFactory;
+import org.apache.roller.weblogger.ui.core.security.BootstrapSecurity;
+
+/** Prevents anonymous access to installer and first-user actions. */
+public class BootstrapSecurityFilter implements Filter {
+ public void doFilter(ServletRequest req, ServletResponse res, FilterChain
chain) throws IOException, ServletException {
+ HttpServletRequest r = (HttpServletRequest) req;
+ HttpServletResponse p = (HttpServletResponse) res;
+ String uri = r.getRequestURI();
+ boolean tokenPage = uri != null &&
(uri.endsWith("/bootstrap-token.rol")
+ || uri.endsWith("/bootstrap-token!redeem.rol"));
+ boolean installer = uri != null && (tokenPage ||
uri.contains("/roller-ui/install/")
+ || uri.endsWith("/roller-ui/register.rol")
+ || uri.endsWith("/roller-ui/register!save.rol")
+ || uri.endsWith("/roller-ui/setup.rol"));
+ if (installer && !BootstrapSecurity.isCompleted()) {
Review Comment:
Once `isCompleted()` is true this whole block is skipped, so
`bootstrap-token.rol` falls through to the action and renders a live-looking
token form.
Verified on a fully installed instance (`bootstrap.completed=true` in
`roller_properties`): `install.rol` correctly 302s to `/`, but `GET
/roller-ui/bootstrap-token.rol` returns **200 with the token `<input>`
present**.
It is inert — `digest` is null once setup completes, so any submission gets
"The setup token is invalid or has expired" — but it reads against the PR's
"permanently close bootstrap access" goal, and an operator who finds that page
has no way to tell it is dead. Returning 404, or redirecting to `/` when
`isCompleted()`, would match the stated behaviour.
##########
app/src/main/java/org/apache/roller/weblogger/ui/core/RollerContext.java:
##########
@@ -186,6 +191,21 @@ public void contextInitialized(ServletContextEvent sce) {
// trigger initialization process
weblogger = WebloggerFactory.getWeblogger();
weblogger.initialize();
+ try {
+ org.apache.roller.weblogger.pojos.RuntimeConfigProperty
marker =
+
weblogger.getPropertiesManager().getProperty(BootstrapSecurity.COMPLETION_PROPERTY);
+ if (marker != null &&
"true".equalsIgnoreCase(marker.getValue())) {
+ BootstrapSecurity.complete();
+ } else if (weblogger.getUserManager().getUserCount() > 0) {
Review Comment:
This block and the first-admin path in `Register` are the only places that
write `bootstrap.completed` and call `complete()` — and this one only runs
inside `contextInitialized`.
That leaves a gap on the **upgrade-only** path, which is the common case for
an existing site. When the operator redeems the token and completes
`install!upgrade.rol` → `install!bootstrap.rol`, the business tier comes up but
this block never re-runs, so `completed` stays `false` and no marker is written
until the next restart. No first-user registration happens either, since the
users already exist.
Observed in that window:
- anonymous `GET /roller-ui/install/install.rol` → 302 to
`bootstrap-token.rol`, a page that can no longer be satisfied because `digest`
was consumed at redemption — a dead end until Tomcat restarts;
- `bootstrap.completed` absent from `roller_properties`, appearing only
after the next restart via this code;
- the redeeming session keeps its 60-minute grant over the installer and
`register` actions. That may well be intended, but combined with the above it
means "setup finished" and "bootstrap closed" are two different moments.
Calling `complete()` and saving the marker at the end of the successful
install/upgrade action would close the gate when setup actually finishes,
rather than at the next restart.
##########
app/src/main/java/org/apache/roller/weblogger/ui/struts2/core/BootstrapToken.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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. For additional information regarding
+ * copyright in this work, please see the NOTICE file in the top level
+ * directory of this distribution.
+ */
+
+package org.apache.roller.weblogger.ui.struts2.core;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.roller.weblogger.ui.core.security.BootstrapSecurity;
+import org.apache.roller.weblogger.ui.struts2.util.UIAction;
+import org.apache.struts2.interceptor.ServletRequestAware;
+import org.apache.struts2.interceptor.ServletResponseAware;
+
+public class BootstrapToken extends UIAction implements ServletRequestAware,
ServletResponseAware {
+
+ private static final Log LOG = LogFactory.getLog(BootstrapToken.class);
+
+ private HttpServletRequest request;
+ private HttpServletResponse response;
+ private String token;
+
+ public BootstrapToken() {
+ this.pageTitle = "installer.bootstrap.pageTitle";
+ }
+
+ public String execute() {
+ setResponseHeaders();
+ LOG.info("Roller is waiting for an administrator to submit the
one-time setup token; "
+ + "database setup will not continue until the token is
accepted. Setup page: "
+ + request.getRequestURL());
+ return INPUT;
+ }
+
+ public String redeem() {
+ setResponseHeaders();
+ if (!"POST".equalsIgnoreCase(request.getMethod())) {
+ addActionError(getText("installer.bootstrap.postRequired"));
+ return INPUT;
+ }
+ if (BootstrapSecurity.redeem(request, token == null ? null :
token.trim())) {
+ LOG.info("One-time setup token accepted; continuing database
setup.");
+ return SUCCESS;
+ }
+ addActionError(getText("installer.bootstrap.invalidToken"));
Review Comment:
A rejected token produces no distinct log line. The only bootstrap logging
is the `LOG.info` in `execute()` saying Roller is waiting for a token — which
is emitted on every render of the page, including the re-render after a
failure. So repeated failed redemptions are effectively invisible in
`roller.log`.
A `LOG.warn` here including `request.getRemoteAddr()` would make probing
visible. The token is 256 bits so brute force isn't a realistic threat, but a
rejected setup-token submission is exactly the sort of event an operator would
want logged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
