You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2014/09/08 18:52:39 UTC
[GitHub] incubator-brooklyn pull request: Brooklyn 51
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/154
Brooklyn 51
If no security properties are set and `--noConsoleSecurity` is not set then `BrooklynLauncher` configures the server to accept unsecured connections from `127.0.0.1` and secured external connections that are authenticated with the user `brooklyn` and a randomly generated password that is logged once.
The new security filter checks the remote address of the request. I'm assuming that this can't be spoofed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn brooklyn-51-remote-access
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/154.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #154
----
commit ded72870f156f0459ef9be1768c224ad92fcb3d2
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2014-09-08T15:58:04Z
Adds AbstractSecurityProvider
Simplifies ExplicitUsersSecurityProvider and LdapSecurityProvider
commit b7a5ea7a59b9dd7626693d695a221fb6bce72b1f
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2014-09-08T16:41:09Z
BROOKLYN-51: Brooklyn allows passworded remote access when no security configured
Adds BrooklynUserWithRandomPasswordSecurityProvider. Password is logged
once at info.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#issuecomment-54942422
Looks really good; a few minor comments for you to look through. Then I'm happy for this to be merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#discussion_r17288731
--- Diff: usage/rest-server/src/main/java/brooklyn/rest/security/provider/BrooklynUserWithRandomPasswordSecurityProvider.java ---
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+package brooklyn.rest.security.provider;
+
+import javax.servlet.http.HttpSession;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import brooklyn.management.ManagementContext;
+import brooklyn.rest.security.BrooklynPropertiesSecurityFilter;
+import brooklyn.util.text.Identifiers;
+
+public class BrooklynUserWithRandomPasswordSecurityProvider extends AbstractSecurityProvider implements SecurityProvider {
+
+ public static final Logger LOG = LoggerFactory.getLogger(BrooklynUserWithRandomPasswordSecurityProvider.class);
+ private static final String USER = "brooklyn";
+ private final String password;
+
+ public BrooklynUserWithRandomPasswordSecurityProvider() {
+ this.password = Identifiers.makeRandomId(10);
+ LOG.info("Allowing access to web console from localhost or with {}:{}", USER, password);
--- End diff --
I wonder if this should be a `sysout`, as well as a log. If a `sysout`, it should probably be done by the caller so passing in the user+password. No particularly strong feelings.
Wording would perhaps be improved if said `"or with auto-generated username and password {}:{}"`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#discussion_r17289211
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -595,16 +597,18 @@ private void handleSubsystemStartupError(boolean ignoreSuchErrors, String system
}
protected void startWebApps() {
- if (BrooklynWebConfig.hasNoSecurityOptions(brooklynProperties)) {
- if (bindAddress==null) {
- LOG.info("Starting brooklyn web-console on loopback interface because no security config is set");
- bindAddress = Networking.LOOPBACK;
- }
- if (skipSecurityFilter==null) {
- LOG.debug("Starting brooklyn web-console without security because we are loopback and no security is set");
- skipSecurityFilter = true;
- }
+ // No security options in properties and no command line options overriding.
+ if (Boolean.TRUE.equals(skipSecurityFilter) && bindAddress == null) {
+ LOG.info("Starting Brooklyn web-console on loopback because security is explicitly disabled and no bind address was given");
+ bindAddress = Networking.LOOPBACK;
+ } else if (BrooklynWebConfig.hasNoSecurityOptions(brooklynProperties) && bindAddress == null) {
+ LOG.info("Starting Brooklyn web-console with passwordless access on localhost and protected access from other interfaces");
+ bindAddress = Networking.ANY_NIC;
+ brooklynProperties.put(
+ BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME,
+ BrooklynUserWithRandomPasswordSecurityProvider.class.getName());
--- End diff --
I think this is right, but just wanted to raise question... with `ExplicitUsersSecurityProvider.authenticate` it will look up the users+passwords from brooklyn properties, so a properties-reload will affect subsequent authenticate attempts (but not existing sessions). However, if we've set it to use `BrooklynUserWithRandomPasswordSecurityProvider` then the properties-reload to add explicit users + passwords will have no affect. I think that's fine as we've logged to tell the person starting brooklyn, so they can restart if desired.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#issuecomment-55002917
@aledsage Thanks for your comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/154
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#discussion_r17294137
--- Diff: usage/launcher/src/main/java/brooklyn/launcher/BrooklynLauncher.java ---
@@ -595,16 +597,18 @@ private void handleSubsystemStartupError(boolean ignoreSuchErrors, String system
}
protected void startWebApps() {
- if (BrooklynWebConfig.hasNoSecurityOptions(brooklynProperties)) {
- if (bindAddress==null) {
- LOG.info("Starting brooklyn web-console on loopback interface because no security config is set");
- bindAddress = Networking.LOOPBACK;
- }
- if (skipSecurityFilter==null) {
- LOG.debug("Starting brooklyn web-console without security because we are loopback and no security is set");
- skipSecurityFilter = true;
- }
+ // No security options in properties and no command line options overriding.
+ if (Boolean.TRUE.equals(skipSecurityFilter) && bindAddress == null) {
+ LOG.info("Starting Brooklyn web-console on loopback because security is explicitly disabled and no bind address was given");
+ bindAddress = Networking.LOOPBACK;
+ } else if (BrooklynWebConfig.hasNoSecurityOptions(brooklynProperties) && bindAddress == null) {
+ LOG.info("Starting Brooklyn web-console with passwordless access on localhost and protected access from other interfaces");
+ bindAddress = Networking.ANY_NIC;
+ brooklynProperties.put(
+ BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME,
+ BrooklynUserWithRandomPasswordSecurityProvider.class.getName());
--- End diff --
Yes. Reloading properties after modifying `brooklyn.webconsole.security.provider` will not cause the new provider to be used because `DelegatingSecurityProvider` caches its delegate. It would be fairly straightforward to incorporate this - `DelegatingSecurityProvider` can just check that the classname of its delegate matches the value from the management context's property, and reload if there is a difference.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#discussion_r17293749
--- Diff: usage/rest-server/src/main/java/brooklyn/rest/security/provider/BrooklynUserWithRandomPasswordSecurityProvider.java ---
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+package brooklyn.rest.security.provider;
+
+import javax.servlet.http.HttpSession;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import brooklyn.management.ManagementContext;
+import brooklyn.rest.security.BrooklynPropertiesSecurityFilter;
+import brooklyn.util.text.Identifiers;
+
+public class BrooklynUserWithRandomPasswordSecurityProvider extends AbstractSecurityProvider implements SecurityProvider {
+
+ public static final Logger LOG = LoggerFactory.getLogger(BrooklynUserWithRandomPasswordSecurityProvider.class);
+ private static final String USER = "brooklyn";
+ private final String password;
+
+ public BrooklynUserWithRandomPasswordSecurityProvider() {
+ this.password = Identifiers.makeRandomId(10);
+ LOG.info("Allowing access to web console from localhost or with {}:{}", USER, password);
--- End diff --
I would have used `sysout` from `BrooklynLauncher` but to do that I think I would have had to have made `password` static. The security provider's instantiation is deferred until the first request to the server and the launcher has no way to access the instance used (by `DelegatingSecurityProvider`). I thought that it was better to keep `password` different between instances of the class (should there be any).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: BROOKLYN-51
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/154#discussion_r17289245
--- Diff: usage/rest-server/src/main/java/brooklyn/rest/security/provider/BrooklynUserWithRandomPasswordSecurityProvider.java ---
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+package brooklyn.rest.security.provider;
+
+import javax.servlet.http.HttpSession;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import brooklyn.management.ManagementContext;
+import brooklyn.rest.security.BrooklynPropertiesSecurityFilter;
+import brooklyn.util.text.Identifiers;
+
+public class BrooklynUserWithRandomPasswordSecurityProvider extends AbstractSecurityProvider implements SecurityProvider {
+
+ public static final Logger LOG = LoggerFactory.getLogger(BrooklynUserWithRandomPasswordSecurityProvider.class);
+ private static final String USER = "brooklyn";
+ private final String password;
+
+ public BrooklynUserWithRandomPasswordSecurityProvider() {
+ this.password = Identifiers.makeRandomId(10);
+ LOG.info("Allowing access to web console from localhost or with {}:{}", USER, password);
--- End diff --
Should probably `log.warn` - this isn't a desired production configuration. Thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---