You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by remibergsma <gi...@git.apache.org> on 2015/08/14 10:17:50 UTC

[GitHub] cloudstack pull request: Fix site-to-site VPN feature

GitHub user remibergsma opened a pull request:

    https://github.com/apache/cloudstack/pull/693

    Fix site-to-site VPN feature

    This is work done together with @Jayapal on fixing the site2site VPN. The first part was done in PR #690 by @Jayapal. On top of that, some other fixes were needed and those are added in this PR. It made sense to make a new PR which includes all fixes so we can actually test it. 
    
    The original PR #690 is already merged into this one, so can be closed. Since the commit ids are kept the same, merging this will close both.
    
    I closely compared the 4.4/4.5 implementation with the new 4.6 one. I did not only make it work, but also added some security improvements (some of which were also in 4.4/4.5). I noticed the pre shared key was being logged, so removed that as well.
    
    This is how I tested and verified it:
    https://github.com/schubergphilis/MCT-shared/tree/master/helper_scripts/cloudstack/vpn_tests
    When I have some time available, I'll write a Marvin test for it that we can include in the repo.
    
    It now works(tm) with one manual step due to CLOUDSTACK-8685: 
    We need a default gateway before site-to-site VPN will actually work. It will connect, but not forward packets. The reason for this, is due to the iptables setup. VM1 has router1 as gateway, but router1 does not know the route to VM2 so it will give up. With a default gateway, the packets are about to be forwarded to the default gateway but when they reach eth1 the public nic, iptables kicks in, does some magic and forwards it through the ipsec tunnel. So, you need a default gw set to upstream.
    
    Workaround for now is setting the route manually:
    ``route add default gw 1.2.3.4``  or  ``ip route add default via 1.2.3.4``
    
    In other words, we need to fix CLOUDSTACK-8685 soon, too.
    
    @Jayapal @snuf could you please review this?

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/remibergsma/cloudstack s2svpn-fixes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cloudstack/pull/693.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 #693
    
----
commit da9e757b8e48c54a4ecbd3bdb027b573ac5a3314
Author: Jayapal <ja...@apache.org>
Date:   2015-08-13T08:37:12Z

    CLOUDSTACK-8710: Fixed applying iptables rules for s2s vpn

commit 382458317ead1ded0149b0fa43d41cd527d22e50
Author: Remi Bergsma <gi...@remi.nl>
Date:   2015-08-13T19:35:44Z

    CLOUDSTACK-8730: fix s2s iptables rules and ipsec config
    
    For site2site VPN to work, we need a default gateway to be set.
    See CLOUDSTACK-8685

commit 9b97719c5c7839215fa4ff4392995af28055f803
Author: Remi Bergsma <gi...@remi.nl>
Date:   2015-08-14T07:05:59Z

    tighten security of site-to-site VPN
    
    It was like this in 4.4 and 4.5

commit 4f8ab51f7f2a1d8b78e754981d92c3904408fb30
Author: Remi Bergsma <gi...@remi.nl>
Date:   2015-08-14T07:07:25Z

    do not log sensitive site-to-site VPN PSK
    
    Logging before:
    2015-08-12 16:30:07,126 Searching for 192.168.23.6  and replacing with 192.168.23.6 192.168.23.5: PSK "preSharedKey"
    
    Logging after:
    2015-08-12 16:30:07,126 Searching for 192.168.23.6  and replacing with 192.168.23.6 192.168.23.5: PSK "****"

commit 7ddec661ca6d17fbf9ecbc0ee60e34d68f97a94d
Author: Remi Bergsma <gi...@remi.nl>
Date:   2015-08-14T07:10:28Z

    Merge pull request #690 from jayapalu/vpn
    
    CLOUDSTACK-8710: Fixed applying iptables rules for s2s vpn
    @remibergsma @wilderrodrigues
    Moved applying iptables rules apply after vpn configuration so that vpn specific rules also get applied
    
    * pr/690:
      CLOUDSTACK-8710: Fixed applying iptables rules for s2s vpn
    
    This closes #690
    
    Signed-off-by: Remi Bergsma <gi...@remi.nl>

----


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cloudstack/pull/693


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by snuf <gi...@git.apache.org>.
Github user snuf commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131097876
  
    LGTM


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by jayapal <gi...@git.apache.org>.
Github user jayapal commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131026551
  
    Guys wrong tag.
    
    Myuser name is jayapal and actual commiter was jayapalu
    
    On Fri, Aug 14, 2015 at 1:47 PM, Remi Bergsma <no...@github.com>
    wrote:
    
    > This is work done together with @Jayapal <https://github.com/Jayapal> on
    > fixing the site2site VPN. The first part was done in PR #690
    > <https://github.com/apache/cloudstack/pull/690> by @Jayapal
    > <https://github.com/Jayapal>. On top of that, some other fixes were
    > needed and those are added in this PR. It made sense to make a new PR which
    > includes all fixes so we can actually test it.
    >
    > The original PR #690 <https://github.com/apache/cloudstack/pull/690> is
    > already merged into this one, so can be closed. Since the commit ids are
    > kept the same, merging this will close both.
    >
    > I closely compared the 4.4/4.5 implementation with the new 4.6 one. I did
    > not only make it work, but also added some security improvements (some of
    > which were also in 4.4/4.5). I noticed the pre shared key was being logged,
    > so removed that as well.
    >
    > This is how I tested and verified it:
    >
    > https://github.com/schubergphilis/MCT-shared/tree/master/helper_scripts/cloudstack/vpn_tests
    > When I have some time available, I'll write a Marvin test for it that we
    > can include in the repo.
    >
    > It now works(tm) with one manual step due to CLOUDSTACK-8685:
    > We need a default gateway before site-to-site VPN will actually work. It
    > will connect, but not forward packets. The reason for this, is due to the
    > iptables setup. VM1 has router1 as gateway, but router1 does not know the
    > route to VM2 so it will give up. With a default gateway, the packets are
    > about to be forwarded to the default gateway but when they reach eth1 the
    > public nic, iptables kicks in, does some magic and forwards it through the
    > ipsec tunnel. So, you need a default gw set to upstream.
    >
    > Workaround for now is setting the route manually:
    > route add default gw 1.2.3.4 or ip route add default via 1.2.3.4
    >
    > In other words, we need to fix CLOUDSTACK-8685 soon, too.
    >
    > @Jayapal <https://github.com/Jayapal> @snuf <https://github.com/snuf>
    > could you please review this?
    > ------------------------------
    > You can view, comment on, or merge this pull request online at:
    >
    >   https://github.com/apache/cloudstack/pull/693
    > Commit Summary
    >
    >    - CLOUDSTACK-8710: Fixed applying iptables rules for s2s vpn
    >    - CLOUDSTACK-8730: fix s2s iptables rules and ipsec config
    >    - tighten security of site-to-site VPN
    >    - do not log sensitive site-to-site VPN PSK
    >    - Merge pull request #690 from jayapalu/vpn
    >
    > File Changes
    >
    >    - *M* systemvm/patches/debian/config/opt/cloud/bin/configure.py
    >    <https://github.com/apache/cloudstack/pull/693/files#diff-0> (18)
    >    - *M* systemvm/patches/debian/config/opt/cloud/bin/cs/CsFile.py
    >    <https://github.com/apache/cloudstack/pull/693/files#diff-1> (5)
    >
    > Patch Links:
    >
    >    - https://github.com/apache/cloudstack/pull/693.patch
    >    - https://github.com/apache/cloudstack/pull/693.diff
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/cloudstack/pull/693>.
    >



---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by jayapalu <gi...@git.apache.org>.
Github user jayapalu commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131078443
  
    The iptables rules and vpn config changes are looking good.
    LGTM


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on a diff in the pull request:

    https://github.com/apache/cloudstack/pull/693#discussion_r37057824
  
    --- Diff: systemvm/patches/debian/config/opt/cloud/bin/cs/CsFile.py ---
    @@ -114,7 +114,10 @@ def greplace(self, search, replace):
     
         def search(self, search, replace):
             found = False
    -        logging.debug("Searching for %s and replacing with %s" % (search, replace))
    +        replace_filtered = replace
    +        if re.search("PSK \"", replace):
    +            replace_filtered = re.sub(r'".*"', '"****"', replace)
    +        logging.debug("Searching for %s and replacing with %s" % (search, replace_filtered))
    --- End diff --
    
    "Searching for %s and replacing the value following it with %s" would be closer to the truth


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by remibergsma <gi...@git.apache.org>.
Github user remibergsma commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131027193
  
    @jayapal Sorry for bothering you, just updated the tag. Thanks for letting me know!


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by DaanHoogland <gi...@git.apache.org>.
Github user DaanHoogland commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131019303
  
    no furhter comments, LGTM


---
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] cloudstack pull request: Fix site-to-site VPN feature

Posted by jayapal <gi...@git.apache.org>.
Github user jayapal commented on the pull request:

    https://github.com/apache/cloudstack/pull/693#issuecomment-131106594
  
    No problem
    
    Best,
    Jayapal
    Mobile, xcuse typoerror.
    On 14 Aug 2015 18:29, "Funs" <no...@github.com> wrote:
    
    > LGTM
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/cloudstack/pull/693#issuecomment-131097876>.
    >



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