Hi Sheng, On Thu, Dec 4, 2014 at 9:30 AM, <[email protected]> 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 <[email protected]> > Authored: Wed Dec 3 19:38:47 2014 -0800 > Committer: Sheng Yang <[email protected]> > 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")); > - } > } > >
