You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by bhaisaab <gi...@git.apache.org> on 2015/03/10 11:12:46 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8272: Python based file-lock f...
GitHub user bhaisaab opened a pull request:
https://github.com/apache/cloudstack/pull/106
CLOUDSTACK-8272: Python based file-lock free password server implementat...
Major changes:
- VRs are single CPU, so Threading based implementation favoured than Forking based
- Implements a Python based password server that does not use file based locks
- Saving password mechanism is provided by using secure token only to VR (localhost)
- Old serve_password implementation is removed
- Runs with Python 2.6+ with no external dependencies
- Locks used within threads for extra safety
The aim for this is to improve password server by relying on a threading based approach to minimize RAM usage and at the same time avoid using file based lock. For 4.5.1 and 4.6.0
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/cloudstack concurrent-password-server
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/106.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 #106
----
commit 3951ab00e25458c9c804b190ae7c7758f57985a3
Author: Rohit Yadav <ro...@shapeblue.com>
Date: 2015-03-10T10:05:31Z
CLOUDSTACK-8272: Python based file-lock free password server implementation
- VRs are single CPU, so Threading based implementation favoured than Forking based
- Implements a Python based password server that does not use file based locks
- Saving password mechanism is provided by using secure token only to VR (localhost)
- Old serve_password implementation is removed
- Runs with Python 2.6+ with no external dependencies
- Locks used within threads for extra safety
Signed-off-by: Rohit Yadav <ro...@shapeblue.com>
----
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab closed the pull request at:
https://github.com/apache/cloudstack/pull/106
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78440246
Tested against KVM works for me, closing. Merged on 4.5 and master.
Opened two ticket to track futher work on it to implement a SSL based server and IPv6 support.
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78037787
In a general discussion, should the stored passwords be made expirable; i.e. after a certain interval if the passwords are not served they expire, or if within certain interval (say half an hour) if any password is served should be expired?
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78034264
Thanks @vincentbernat @brutasse for your reviews, fixing them now.
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78217077
@vincentbernat thanks for the snippets!
@resmo in case the passwdsrvrtoken went missing (file does not exist), the script may break, the || true is just so that it return 0 (exit 0). I guess your snippet is correct as well (check and cat file), using that approach.
@pdion891 looking at the code at least on the VR side there is no password expiry period imposed by the server. I can sort of understand this, because someone could shutdown a server, do a reset password and if the VM won't get started we should store the password in the VR. But let's discuss the security aspects on what/how we should implement a self expiring cache. I propose:
1. if we have served the password at least once, let's expire it in 15-30 minutes
2. else, expire all the passwords in next 1-24 hours whether they have been served to user VMs (for resetting purposes) or not.
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] cloudstack pull request: CLOUDSTACK-8272: Python based file-lock f...
Posted by brutasse <gi...@git.apache.org>.
Github user brutasse commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/106#discussion_r26112397
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/passwd_server_ip.py ---
@@ -0,0 +1,204 @@
+#!/usr/bin/env python
+# 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.
+#
+# Client usage examples;
+# Getting password:
+# wget -q -t 3 -T 20 -O - --header "DomU_Request: send_my_password" <routerIP>:8080
+# Send ack:
+# wget -t 3 -T 20 -O - --header "DomU_Request: saved_password" localhost:8080
+# Save password only from within router:
+# /opt/cloud/bin/savepassword.sh -v <IP> -p <password>
+# curl --header "DomU_Request: save_password" http://localhost:8080/ -F ip=<IP> -F password=<passwd>
+
+import binascii
+import cgi
+import os
+import sys
+import syslog
+import threading
+import time
+import urlparse
+
+from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
+from SocketServer import ThreadingMixIn, ForkingMixIn
+
+# Global password map
+passMap = {}
+secureToken = None
+
+# Global passMap lock
+lock = threading.Lock()
+
+def getTokenFile():
+ return "/tmp/passwdsrvrtoken"
+
+def getPasswordFile():
+ return "/var/cache/cloud/passwords"
+
+def initToken():
+ global secureToken
+ f = open(getTokenFile(), "w")
+ secureToken = binascii.hexlify(os.urandom(16))
+ f.write(secureToken)
+ f.close()
+
+def checkToken(token):
+ global secureToken
+ return token == secureToken
+
+def loadPasswordFile():
+ global passMap
+ if os.path.exists(getPasswordFile()):
+ f = open(getPasswordFile(), "r")
+ data = f.read()
+ f.close()
+ for line in data.split("\n"):
+ splitLine = line.split("=")
+ if len(splitLine) == 2:
+ passMap[splitLine[0]] = splitLine[1]
+
+def savePasswordFile():
+ global passMap, lock
+ lock.acquire()
+ try:
+ f = open(getPasswordFile(), "w")
+ for ip in passMap.keys():
+ f.write("%s=%s\n" % (ip, passMap[ip]))
+ f.close()
+ except IOError, e:
+ syslog.syslog("serve_password: Unable to save to password file %s" % e)
+ finally:
+ lock.release()
+
+def getPassword(ip):
+ global passMap
+ if ip in passMap:
+ return passMap[ip]
+ return None
+
+def setPassword(ip, password):
+ global passMap, lock
+ if ip == None or ip == "" or password == None or password == "":
+ return
+ lock.acquire()
+ try:
+ passMap[ip] = password
+ finally:
+ lock.release()
+
+def removePassword(ip):
+ global passMap, lock
+ lock.acquire()
+ try:
+ if ip in passMap:
+ del passMap[ip]
+ finally:
+ lock.release()
+
+
+class ThreadedHTTPServer(ThreadingMixIn, HTTPServer):
+ pass
+
+
+class PasswordRequestHandler(BaseHTTPRequestHandler):
+ server_version = "CloudStack Password Server"
+ sys_version = "4.x"
+ def do_GET(self):
+ self.send_response(200)
+ self.send_header("Content-type", "text/plain")
+ self.send_header("Server", "CloudStack Password Server")
+ self.end_headers()
+ requestType = self.headers.get('DomU_Request')
+ clientAddress = self.client_address[0]
+ if requestType == "send_my_password":
+ password = getPassword(clientAddress)
+ if not password:
+ syslog.syslog("serve_password: requested password not found for %s" % clientAddress)
+ else:
+ self.wfile.write(password)
+ syslog.syslog("serve_password: password sent to %s" % clientAddress)
+ elif requestType == "saved_password":
+ removePassword(clientAddress)
+ savePasswordFile()
+ self.wfile.write("saved_password")
+ syslog.syslog("serve_password: saved_password ack received from %s" % clientAddress)
+ else:
+ self.send_response(400)
+ self.wfile.write("bad_request")
+ syslog.syslog("serve_password: bad_request from IP %s" % clientAddress)
+ return
+
+ def do_POST(self):
+ form = cgi.FieldStorage(
+ fp=self.rfile,
+ headers=self.headers,
+ environ={'REQUEST_METHOD':'POST',
+ 'CONTENT_TYPE':self.headers['Content-Type'],
+ })
+ self.send_response(200)
+ self.end_headers()
+ clientAddress = self.client_address[0]
+ if not (clientAddress == "localhost" or clientAddress == "127.0.0.1"):
+ syslog.syslog("serve_password: non-localhost IP trying to save password: %s" % clientAddress)
+ self.send_response(403)
+ return
+ if "ip" not in form.keys() or "password" not in form.keys() or "token" not in form.keys() or self.headers.get('DomU_Request') != "save_password":
--- End diff --
With dictionaries you can directly check for membership with `if "ip" not in form`.
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by pdion891 <gi...@git.apache.org>.
Github user pdion891 commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78049653
@bhaisaab, can you confirm that if the password is not grap after a certain period it expired (ex: 15mins), and also if the password is grap once, it is not available anymore and have to be reset again. Looks like it is the current approach. Does the current script use inside VMs will remain functional?
Thanks
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by brutasse <gi...@git.apache.org>.
Github user brutasse commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/106#discussion_r26112312
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/passwd_server_ip.py ---
@@ -0,0 +1,204 @@
+#!/usr/bin/env python
+# 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.
+#
+# Client usage examples;
+# Getting password:
+# wget -q -t 3 -T 20 -O - --header "DomU_Request: send_my_password" <routerIP>:8080
+# Send ack:
+# wget -t 3 -T 20 -O - --header "DomU_Request: saved_password" localhost:8080
+# Save password only from within router:
+# /opt/cloud/bin/savepassword.sh -v <IP> -p <password>
+# curl --header "DomU_Request: save_password" http://localhost:8080/ -F ip=<IP> -F password=<passwd>
+
+import binascii
+import cgi
+import os
+import sys
+import syslog
+import threading
+import time
+import urlparse
+
+from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
+from SocketServer import ThreadingMixIn, ForkingMixIn
+
+# Global password map
+passMap = {}
+secureToken = None
+
+# Global passMap lock
+lock = threading.Lock()
+
+def getTokenFile():
+ return "/tmp/passwdsrvrtoken"
+
+def getPasswordFile():
+ return "/var/cache/cloud/passwords"
+
+def initToken():
+ global secureToken
+ f = open(getTokenFile(), "w")
+ secureToken = binascii.hexlify(os.urandom(16))
+ f.write(secureToken)
+ f.close()
+
+def checkToken(token):
+ global secureToken
+ return token == secureToken
+
+def loadPasswordFile():
+ global passMap
+ if os.path.exists(getPasswordFile()):
+ f = open(getPasswordFile(), "r")
+ data = f.read()
+ f.close()
+ for line in data.split("\n"):
+ splitLine = line.split("=")
+ if len(splitLine) == 2:
+ passMap[splitLine[0]] = splitLine[1]
+
+def savePasswordFile():
+ global passMap, lock
+ lock.acquire()
+ try:
+ f = open(getPasswordFile(), "w")
+ for ip in passMap.keys():
+ f.write("%s=%s\n" % (ip, passMap[ip]))
+ f.close()
+ except IOError, e:
+ syslog.syslog("serve_password: Unable to save to password file %s" % e)
+ finally:
+ lock.release()
+
+def getPassword(ip):
+ global passMap
+ if ip in passMap:
+ return passMap[ip]
+ return None
+
+def setPassword(ip, password):
+ global passMap, lock
+ if ip == None or ip == "" or password == None or password == "":
+ return
+ lock.acquire()
+ try:
+ passMap[ip] = password
+ finally:
+ lock.release()
+
+def removePassword(ip):
+ global passMap, lock
+ lock.acquire()
+ try:
+ if ip in passMap:
+ del passMap[ip]
+ finally:
+ lock.release()
+
+
+class ThreadedHTTPServer(ThreadingMixIn, HTTPServer):
+ pass
+
+
+class PasswordRequestHandler(BaseHTTPRequestHandler):
+ server_version = "CloudStack Password Server"
+ sys_version = "4.x"
+ def do_GET(self):
+ self.send_response(200)
+ self.send_header("Content-type", "text/plain")
+ self.send_header("Server", "CloudStack Password Server")
+ self.end_headers()
+ requestType = self.headers.get('DomU_Request')
+ clientAddress = self.client_address[0]
+ if requestType == "send_my_password":
+ password = getPassword(clientAddress)
+ if not password:
+ syslog.syslog("serve_password: requested password not found for %s" % clientAddress)
+ else:
+ self.wfile.write(password)
+ syslog.syslog("serve_password: password sent to %s" % clientAddress)
+ elif requestType == "saved_password":
+ removePassword(clientAddress)
+ savePasswordFile()
+ self.wfile.write("saved_password")
+ syslog.syslog("serve_password: saved_password ack received from %s" % clientAddress)
+ else:
+ self.send_response(400)
+ self.wfile.write("bad_request")
+ syslog.syslog("serve_password: bad_request from IP %s" % clientAddress)
+ return
+
+ def do_POST(self):
+ form = cgi.FieldStorage(
+ fp=self.rfile,
+ headers=self.headers,
+ environ={'REQUEST_METHOD':'POST',
+ 'CONTENT_TYPE':self.headers['Content-Type'],
+ })
+ self.send_response(200)
+ self.end_headers()
+ clientAddress = self.client_address[0]
+ if not (clientAddress == "localhost" or clientAddress == "127.0.0.1"):
--- End diff --
`if clientAddress not in ['localhost', '127.0.0.1']`
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by vincentbernat <gi...@git.apache.org>.
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/106#issuecomment-78031911
Some remarks:
- Use `with lock:` instead of `lock.acquire()` then `try/finally` (compatible with Python 2.6)
- Use `with file(...) as f:` (idem)
- `global passMap` and `global lock` are useless since you don't do direct assignments
- `getPassword()` could be `passMap.get(..., None)`
---
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: CLOUDSTACK-8272: Python based file-lock f...
Posted by brutasse <gi...@git.apache.org>.
Github user brutasse commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/106#discussion_r26112215
--- Diff: systemvm/patches/debian/config/opt/cloud/bin/passwd_server_ip.py ---
@@ -0,0 +1,204 @@
+#!/usr/bin/env python
+# 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.
+#
+# Client usage examples;
+# Getting password:
+# wget -q -t 3 -T 20 -O - --header "DomU_Request: send_my_password" <routerIP>:8080
+# Send ack:
+# wget -t 3 -T 20 -O - --header "DomU_Request: saved_password" localhost:8080
+# Save password only from within router:
+# /opt/cloud/bin/savepassword.sh -v <IP> -p <password>
+# curl --header "DomU_Request: save_password" http://localhost:8080/ -F ip=<IP> -F password=<passwd>
+
+import binascii
+import cgi
+import os
+import sys
+import syslog
+import threading
+import time
+import urlparse
+
+from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
+from SocketServer import ThreadingMixIn, ForkingMixIn
+
+# Global password map
+passMap = {}
+secureToken = None
+
+# Global passMap lock
+lock = threading.Lock()
+
+def getTokenFile():
+ return "/tmp/passwdsrvrtoken"
+
+def getPasswordFile():
+ return "/var/cache/cloud/passwords"
+
+def initToken():
+ global secureToken
+ f = open(getTokenFile(), "w")
+ secureToken = binascii.hexlify(os.urandom(16))
+ f.write(secureToken)
+ f.close()
+
+def checkToken(token):
+ global secureToken
+ return token == secureToken
+
+def loadPasswordFile():
+ global passMap
+ if os.path.exists(getPasswordFile()):
+ f = open(getPasswordFile(), "r")
+ data = f.read()
+ f.close()
+ for line in data.split("\n"):
+ splitLine = line.split("=")
+ if len(splitLine) == 2:
+ passMap[splitLine[0]] = splitLine[1]
+
+def savePasswordFile():
+ global passMap, lock
+ lock.acquire()
+ try:
+ f = open(getPasswordFile(), "w")
+ for ip in passMap.keys():
+ f.write("%s=%s\n" % (ip, passMap[ip]))
+ f.close()
+ except IOError, e:
+ syslog.syslog("serve_password: Unable to save to password file %s" % e)
+ finally:
+ lock.release()
+
+def getPassword(ip):
+ global passMap
+ if ip in passMap:
+ return passMap[ip]
+ return None
+
+def setPassword(ip, password):
+ global passMap, lock
+ if ip == None or ip == "" or password == None or password == "":
--- End diff --
You could simply test for `if not ip or not 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.
---