You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Rohit Yadav <ro...@gmail.com> on 2014/12/04 07:05:39 UTC
Re: git commit: updated refs/heads/master to 3c9e14e
Hi Sheng,
On Thu, Dec 4, 2014 at 9:30 AM, <ya...@apache.org> wrote:
> Repository: cloudstack
> Updated Branches:
> refs/heads/master 1716067dd -> 3c9e14e85
>
>
> CLOUDSTACK-5241: Remove Rot13 usage
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
> Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/3c9e14e8
> Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/3c9e14e8
> Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/3c9e14e8
>
> Branch: refs/heads/master
> Commit: 3c9e14e85a124105e8c1d790a9c46fff7844f7ab
> Parents: 1716067
> Author: Sheng Yang <sh...@citrix.com>
> Authored: Wed Dec 3 19:38:47 2014 -0800
> Committer: Sheng Yang <sh...@citrix.com>
> Committed: Wed Dec 3 19:38:47 2014 -0800
>
> ----------------------------------------------------------------------
> .../network/element/CloudZonesNetworkElement.java | 4 +---
> .../cloud/network/router/CommandSetupHelper.java | 6 ++----
> .../debian/config/opt/cloud/bin/savepassword.sh | 4 +---
> utils/src/com/cloud/utils/PasswordGenerator.java | 18 ------------------
> .../com/cloud/utils/PasswordGeneratorTest.java | 15 ---------------
> 5 files changed, 4 insertions(+), 43 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3c9e14e8/server/src/com/cloud/network/element/CloudZonesNetworkElement.java
> ----------------------------------------------------------------------
> diff --git
> a/server/src/com/cloud/network/element/CloudZonesNetworkElement.java
> b/server/src/com/cloud/network/element/CloudZonesNetworkElement.java
> index 64a8cec..c748991 100644
> --- a/server/src/com/cloud/network/element/CloudZonesNetworkElement.java
> +++ b/server/src/com/cloud/network/element/CloudZonesNetworkElement.java
> @@ -50,7 +50,6 @@ import com.cloud.network.PhysicalNetworkServiceProvider;
> import com.cloud.network.dao.NetworkDao;
> import com.cloud.offering.NetworkOffering;
> import com.cloud.service.dao.ServiceOfferingDao;
> -import com.cloud.utils.PasswordGenerator;
> import com.cloud.utils.component.AdapterBase;
> import com.cloud.vm.NicProfile;
> import com.cloud.vm.ReservationContext;
> @@ -215,8 +214,7 @@ public class CloudZonesNetworkElement extends
> AdapterBase implements NetworkElem
>
> Commands cmds = new Commands(Command.OnError.Continue);
> if (password != null && nic.isDefaultNic()) {
> - final String encodedPassword =
> PasswordGenerator.rot13(password);
> - SavePasswordCommand cmd = new
> SavePasswordCommand(encodedPassword, nic.getIp4Address(),
> uservm.getHostName(), _networkMgr.getExecuteInSeqNtwkElmtCmd());
> + SavePasswordCommand cmd = new
> SavePasswordCommand(password, nic.getIp4Address(), uservm.getHostName(),
> _networkMgr.getExecuteInSeqNtwkElmtCmd());
> cmds.addCommand("password", cmd);
> }
> String serviceOffering =
> _serviceOfferingDao.findByIdIncludingRemoved(uservm.getServiceOfferingId()).getDisplayText();
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3c9e14e8/server/src/com/cloud/network/router/CommandSetupHelper.java
> ----------------------------------------------------------------------
> diff --git a/server/src/com/cloud/network/router/CommandSetupHelper.java
> b/server/src/com/cloud/network/router/CommandSetupHelper.java
> index 727df8a..25bbd35 100644
> --- a/server/src/com/cloud/network/router/CommandSetupHelper.java
> +++ b/server/src/com/cloud/network/router/CommandSetupHelper.java
> @@ -112,7 +112,6 @@ import com.cloud.storage.dao.GuestOSDao;
> import com.cloud.user.Account;
> import com.cloud.uservm.UserVm;
> import com.cloud.utils.Pair;
> -import com.cloud.utils.PasswordGenerator;
> import com.cloud.utils.StringUtils;
> import com.cloud.utils.db.EntityManager;
> import com.cloud.utils.net.NetUtils;
> @@ -616,8 +615,7 @@ public class CommandSetupHelper {
>
> // password should be set only on default network element
> if (password != null && nic.isDefaultNic()) {
> - final String encodedPassword =
> PasswordGenerator.rot13(password);
> - final SavePasswordCommand cmd = new
> SavePasswordCommand(encodedPassword, nic.getIp4Address(),
> profile.getVirtualMachine().getHostName(),
> + final SavePasswordCommand cmd = new
> SavePasswordCommand(password, nic.getIp4Address(),
> profile.getVirtualMachine().getHostName(),
>
Regarding the fix, I see you've simply removed the methods so we're not
passing encoded password in the command at all. Is this a security risk?
Or, should we implement some other encryption or simply pass to VR over SSL?
Regards.
> _networkModel.getExecuteInSeqNtwkElmtCmd());
> cmd.setAccessDetail(NetworkElementCommand.ROUTER_IP,
> _routerControlHelper.getRouterControlIp(router.getId()));
> cmd.setAccessDetail(NetworkElementCommand.ROUTER_GUEST_IP,
> _routerControlHelper.getRouterIpInNetwork(nic.getNetworkId(),
> router.getId()));
> @@ -1026,4 +1024,4 @@ public class CommandSetupHelper {
> }
> return dhcpRange;
> }
> -}
> \ No newline at end of file
> +}
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3c9e14e8/systemvm/patches/debian/config/opt/cloud/bin/savepassword.sh
> ----------------------------------------------------------------------
> diff --git a/systemvm/patches/debian/config/opt/cloud/bin/savepassword.sh
> b/systemvm/patches/debian/config/opt/cloud/bin/savepassword.sh
> index 21fa09d..ab027b6 100755
> --- a/systemvm/patches/debian/config/opt/cloud/bin/savepassword.sh
> +++ b/systemvm/patches/debian/config/opt/cloud/bin/savepassword.sh
> @@ -40,9 +40,7 @@ do
> case $OPTION in
> v) VM_IP="$OPTARG"
> ;;
> - p)
> - ENCODEDPASSWORD="$OPTARG"
> - PASSWORD=$(echo $ENCODEDPASSWORD | tr
> '[a-m][n-z][A-M][N-Z]' '[n-z][a-m][N-Z][A-M]')
> + p) PASSWORD="$OPTARG"
> ;;
> ?) echo "Incorrect usage"
> unlock_exit 1 $lock $locked
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3c9e14e8/utils/src/com/cloud/utils/PasswordGenerator.java
> ----------------------------------------------------------------------
> diff --git a/utils/src/com/cloud/utils/PasswordGenerator.java
> b/utils/src/com/cloud/utils/PasswordGenerator.java
> index 6fa2843..0d79143 100644
> --- a/utils/src/com/cloud/utils/PasswordGenerator.java
> +++ b/utils/src/com/cloud/utils/PasswordGenerator.java
> @@ -87,22 +87,4 @@ public class PasswordGenerator {
> return psk.toString();
>
> }
> -
> - public static String rot13(final String password) {
> - final StringBuilder newPassword = new
> StringBuilder(password.length());
> -
> - for (int i = 0; i < password.length(); i++) {
> - char c = password.charAt(i);
> -
> - if ((c >= 'a' && c <= 'm') || ((c >= 'A' && c <= 'M'))) {
> - c += 13;
> - } else if ((c >= 'n' && c <= 'z') || (c >= 'N' && c <= 'Z')) {
> - c -= 13;
> - }
> -
> - newPassword.append(c);
> - }
> -
> - return newPassword.toString();
> - }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/3c9e14e8/utils/test/com/cloud/utils/PasswordGeneratorTest.java
> ----------------------------------------------------------------------
> diff --git a/utils/test/com/cloud/utils/PasswordGeneratorTest.java
> b/utils/test/com/cloud/utils/PasswordGeneratorTest.java
> index bd87987..413b866 100644
> --- a/utils/test/com/cloud/utils/PasswordGeneratorTest.java
> +++ b/utils/test/com/cloud/utils/PasswordGeneratorTest.java
> @@ -39,19 +39,4 @@ public class PasswordGeneratorTest {
> // and the third is a digit
> Assert.assertTrue(Character.isDigit(password.charAt(2)));
> }
> -
> - @Test
> - public void rot13() {
> - // only letters are handled, numbers are unchanged
> - Assert.assertEquals("1234", PasswordGenerator.rot13("1234"));
> - // letters are moved by +-13 characters
> - Assert.assertEquals("nop", PasswordGenerator.rot13("abc"));
> - // the transformation it is reversable
> - Assert.assertEquals("abc", PasswordGenerator.rot13("nop"));
> - // which means for any string
> - Assert.assertEquals("abcdefghijklmnooprstuvxyzuv1234?",
> PasswordGenerator.rot13(PasswordGenerator.rot13("abcdefghijklmnooprstuvxyzuv1234?")));
> - // same for capital letters
> - Assert.assertEquals("ABC", PasswordGenerator.rot13("NOP"));
> - Assert.assertEquals("NOP", PasswordGenerator.rot13("ABC"));
> - }
> }
>
>