diff --git a/reviewboard/admin/forms.py b/reviewboard/admin/forms.py index 839a123566645715a43b427b04f8e345e68d899b..d7f88407d68a70964a4e4eea0b7d0dbd61cbf93a 100644 --- a/reviewboard/admin/forms.py +++ b/reviewboard/admin/forms.py @@ -44,7 +44,7 @@ from reviewboard.admin.checks import get_can_enable_search, \ get_can_use_amazon_s3, \ get_can_use_couchdb from reviewboard.admin.siteconfig import load_site_config -from reviewboard.scmtools import sshutils +from reviewboard.ssh.client import SSHClient class GeneralSettingsForm(SiteSettingsForm): @@ -537,7 +537,7 @@ class SSHSettingsForm(forms.Form): def create(self, files): if self.cleaned_data['generate_key']: try: - sshutils.generate_user_key() + SSHClient().generate_user_key() except IOError, e: self.errors['generate_key'] = forms.util.ErrorList([ _('Unable to write SSH key file: %s') % e @@ -550,7 +550,7 @@ class SSHSettingsForm(forms.Form): raise elif self.cleaned_data['keyfile']: try: - sshutils.import_user_key(files['keyfile']) + SSHClient().import_user_key(files['keyfile']) except IOError, e: self.errors['keyfile'] = forms.util.ErrorList([ _('Unable to write SSH key file: %s') % e diff --git a/reviewboard/admin/views.py b/reviewboard/admin/views.py index 3e3c9c89e362b87462f4c5bd5402254972afc47e..754d5946bdbdcd1c0304a0dac66bf93bb95374c1 100644 --- a/reviewboard/admin/views.py +++ b/reviewboard/admin/views.py @@ -15,7 +15,8 @@ from reviewboard.admin.cache_stats import get_cache_stats, get_has_cache_stats from reviewboard.admin.forms import SSHSettingsForm from reviewboard.reviews.models import Group, DefaultReviewer from reviewboard.scmtools.models import Repository -from reviewboard.scmtools import sshutils +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.utils import humanize_key @staff_member_required @@ -61,7 +62,8 @@ def site_settings(request, form_class, @staff_member_required def ssh_settings(request, template_name='admin/ssh_settings.html'): - key = sshutils.get_user_key() + client = SSHClient() + key = client.get_user_key() if request.method == 'POST': form = SSHSettingsForm(request.POST, request.FILES) @@ -77,7 +79,7 @@ def ssh_settings(request, template_name='admin/ssh_settings.html'): form = SSHSettingsForm() if key: - fingerprint = sshutils.humanize_key(key) + fingerprint = humanize_key(key) else: fingerprint = None @@ -85,7 +87,7 @@ def ssh_settings(request, template_name='admin/ssh_settings.html'): 'title': _('SSH Settings'), 'key': key, 'fingerprint': fingerprint, - 'public_key': sshutils.get_public_key(key), + 'public_key': client.get_public_key(key), 'form': form, })) diff --git a/reviewboard/cmdline/rbssh.py b/reviewboard/cmdline/rbssh.py index 0b7ca400796eac65be884a402c0faeb8666be437..079a266f9afb1fe0cf6fcc7199d31325fc847d57 100755 --- a/reviewboard/cmdline/rbssh.py +++ b/reviewboard/cmdline/rbssh.py @@ -39,8 +39,8 @@ from optparse import OptionParser import paramiko from reviewboard import get_version_string -from reviewboard.scmtools import sshutils from reviewboard.scmtools.core import SCMTool +from reviewboard.ssh.client import SSHClient DEBUG = os.getenv('DEBUG_RBSSH') @@ -277,13 +277,13 @@ def main(): logging.debug('!!! %s, %s, %s' % (hostname, username, command)) - client = sshutils.get_ssh_client(options.local_site_name) + client = SSHClient(namespace=options.local_site_name) client.set_missing_host_key_policy(paramiko.WarningPolicy()) attempts = 0 password = None - key = sshutils.get_user_key(options.local_site_name) + key = client.get_user_key() while True: try: diff --git a/reviewboard/reviews/forms.py b/reviewboard/reviews/forms.py index 7996c654db276b0a3b67069c9c70e3e1cf20dd96..79eb2bf049b603c467d43e07f1af99abad46b4bd 100644 --- a/reviewboard/reviews/forms.py +++ b/reviewboard/reviews/forms.py @@ -15,6 +15,7 @@ from reviewboard.scmtools.errors import SCMError, ChangeNumberInUseError, \ ChangeSetError from reviewboard.scmtools.models import Repository from reviewboard.site.validation import validate_review_groups, validate_users +from reviewboard.ssh.errors import SSHError class DefaultReviewerForm(forms.ModelForm): @@ -187,7 +188,7 @@ class NewReviewRequestForm(forms.Form): # This scmtool doesn't have changesets self.errors['changenum'] = forms.util.ErrorList(['Changesets are not supported.']) raise ChangeSetError(None) - except SCMError, e: + except (SCMError, SSHError), e: self.errors['changenum'] = forms.util.ErrorList([str(e)]) raise ChangeSetError(None) diff --git a/reviewboard/reviews/views.py b/reviewboard/reviews/views.py index b79e910e87d980114aca87e64afcd33602f63c2b..aaaa1c4225baf2e0273554924146b87ef1307775 100644 --- a/reviewboard/reviews/views.py +++ b/reviewboard/reviews/views.py @@ -51,6 +51,7 @@ from reviewboard.reviews.models import Comment, FileAttachmentComment, \ from reviewboard.scmtools.core import PRE_CREATION from reviewboard.scmtools.errors import SCMError from reviewboard.site.models import LocalSite +from reviewboard.ssh.errors import SSHError from reviewboard.webapi.encoder import status_to_string @@ -266,7 +267,7 @@ def new_review_request(request, parent_diff_file=request.FILES.get('parent_diff_path'), local_site=local_site) return HttpResponseRedirect(review_request.get_absolute_url()) - except (OwnershipError, SCMError, ValueError): + except (OwnershipError, SCMError, SSHError, ValueError): pass else: form = NewReviewRequestForm(request.user, local_site) diff --git a/reviewboard/scmtools/bzr.py b/reviewboard/scmtools/bzr.py index 3ea42582b2f9e58bf38364c27709dfdd05f5e9c4..4d7aee02d1524f8a9dc9432ba6b552b198cdfd20 100644 --- a/reviewboard/scmtools/bzr.py +++ b/reviewboard/scmtools/bzr.py @@ -15,9 +15,9 @@ try: except ImportError: has_bzrlib = False -from reviewboard.scmtools import sshutils from reviewboard.scmtools.core import SCMTool, HEAD, PRE_CREATION from reviewboard.scmtools.errors import RepositoryNotFoundError, SCMError +from reviewboard.ssh import utils as sshutils # Register these URI schemes so we can handle them properly. diff --git a/reviewboard/scmtools/core.py b/reviewboard/scmtools/core.py index f08b071d5e9f383f09b0f8941383de7449dc697d..bbdf88a640c80afdfd284457f8b2fa20099ca22d 100644 --- a/reviewboard/scmtools/core.py +++ b/reviewboard/scmtools/core.py @@ -6,8 +6,11 @@ import urllib2 import urlparse import reviewboard.diffviewer.parser as diffparser -from reviewboard.scmtools import sshutils -from reviewboard.scmtools.errors import FileNotFoundError, SCMError +from reviewboard.scmtools.errors import AuthenticationError, \ + FileNotFoundError, \ + SCMError +from reviewboard.ssh import utils as sshutils +from reviewboard.ssh.errors import SSHAuthenticationError class ChangeSet: @@ -149,7 +152,18 @@ class SCMTool(object): logging.debug( "%s: Attempting ssh connection with host: %s, username: %s" % \ (cls.__name__, hostname, username)) - sshutils.check_host(hostname, username, password, local_site_name) + + try: + sshutils.check_host(hostname, username, password, + local_site_name) + except SSHAuthenticationError, e: + # Represent an SSHAuthenticationError as a standard + # AuthenticationError. + raise AuthenticationError(e.allowed_types, unicode(e), + e.user_key) + except: + # Re-raise anything else + raise @classmethod def get_auth_from_uri(cls, path, username): diff --git a/reviewboard/scmtools/cvs.py b/reviewboard/scmtools/cvs.py index 0a1e84101f65d01d321bc7f233d6768b34d1b497..24c66aeb9f7b7b79a815b08d6954a91c050626d7 100644 --- a/reviewboard/scmtools/cvs.py +++ b/reviewboard/scmtools/cvs.py @@ -5,11 +5,14 @@ import urlparse from djblets.util.filesystem import is_exe_in_path -from reviewboard.scmtools import sshutils from reviewboard.scmtools.core import SCMTool, HEAD, PRE_CREATION -from reviewboard.scmtools.errors import SCMError, FileNotFoundError, \ +from reviewboard.scmtools.errors import AuthenticationError, \ + SCMError, \ + FileNotFoundError, \ RepositoryNotFoundError from reviewboard.diffviewer.parser import DiffParser, DiffParserError +from reviewboard.ssh import utils as sshutils +from reviewboard.ssh.errors import SSHAuthenticationError, SSHError sshutils.register_rbssh('CVS_RSH') @@ -120,15 +123,24 @@ class CVSTool(SCMTool): m = cls.ext_cvsroot_re.match(path) if m: - sshutils.check_host(m.group('hostname'), username, password, - local_site_name) + try: + sshutils.check_host(m.group('hostname'), username, password, + local_site_name) + except SSHAuthenticationError, e: + # Represent an SSHAuthenticationError as a standard + # AuthenticationError. + raise AuthenticationError(e.allowed_types, unicode(e), + e.user_key) + except: + # Re-raise anything else + raise cvsroot, repopath = cls.build_cvsroot(path, username, password) client = CVSClient(cvsroot, repopath, local_site_name) try: client.cat_file('CVSROOT/modules', HEAD) - except (SCMError, FileNotFoundError): + except (SCMError, SSHError, FileNotFoundError): raise RepositoryNotFoundError() @classmethod diff --git a/reviewboard/scmtools/errors.py b/reviewboard/scmtools/errors.py index e6bbe894203cc60603c3c426f72d76c5865d2c4e..e988151122b2a95c20e86bc405b61c6edc6c5d46 100644 --- a/reviewboard/scmtools/errors.py +++ b/reviewboard/scmtools/errors.py @@ -1,7 +1,6 @@ -import socket - from django.utils.translation import ugettext as _ -from djblets.util.humanize import humanize_list + +from reviewboard.ssh.errors import SSHAuthenticationError class SCMError(Exception): @@ -70,7 +69,7 @@ class RepositoryNotFoundError(SCMError): 'specified path.')) -class AuthenticationError(SCMError): +class AuthenticationError(SSHAuthenticationError, SCMError): """An error representing a failed authentication for a repository. This takes a list of authentication types that are allowed. These @@ -79,20 +78,7 @@ class AuthenticationError(SCMError): This may also take the user's SSH key that was tried, if any. """ - def __init__(self, allowed_types=[], msg=None, user_key=None): - if allowed_types: - msg = _('Unable to authenticate against this repository using one ' - 'of the supported authentication types ' - '(%(allowed_types)s).') % { - 'allowed_types': humanize_list(allowed_types), - } - elif not msg: - msg = _('Unable to authenticate against this repository using one ' - 'of the supported authentication types.') - - SCMError.__init__(self, msg) - self.allowed_types = allowed_types - self.user_key = user_key + pass class UnverifiedCertificateError(SCMError): @@ -103,54 +89,3 @@ class UnverifiedCertificateError(SCMError): self.certificate = certificate -class UnsupportedSSHKeyError(SCMError): - """An error representing an unsupported type of SSH key.""" - def __init__(self): - SCMError.__init__(self, - _('This SSH key is not a valid RSA or DSS key.')) - - -class SSHKeyError(SCMError): - """An error involving a host key on an SSH connection.""" - def __init__(self, hostname, key, message): - from reviewboard.scmtools.sshutils import humanize_key - - SCMError.__init__(self, message) - self.hostname = hostname - self.key = humanize_key(key) - self.raw_key = key - - -class BadHostKeyError(SSHKeyError): - """An error representing a bad or malicious key for an SSH connection.""" - def __init__(self, hostname, key, expected_key): - from reviewboard.scmtools.sshutils import humanize_key - - SSHKeyError.__init__( - self, hostname, key, - _("Warning! The host key for server %(hostname)s does not match " - "the expected key.\n" - "It's possible that someone is performing a man-in-the-middle " - "attack. It's also possible that the RSA host key has just " - "been changed. Please contact your system administrator if " - "you're not sure. Do not accept this host key unless you're " - "certain it's safe!") - % { - 'hostname': hostname, - 'ip_address': socket.gethostbyname(hostname), - }) - self.expected_key = humanize_key(expected_key) - self.raw_expected_key = expected_key - - -class UnknownHostKeyError(SSHKeyError): - """An error representing an unknown host key for an SSH connection.""" - def __init__(self, hostname, key): - SSHKeyError.__init__( - self, hostname, key, - _("The authenticity of the host '%(hostname)s (%(ip)s)' " - "couldn't be determined.") % { - 'hostname': hostname, - 'ip': socket.gethostbyname(hostname), - } - ) diff --git a/reviewboard/scmtools/forms.py b/reviewboard/scmtools/forms.py index ed36a8498858bdc5f203c2e1983efe3eb50b9666..116d57ecb2f4189ffd248bb9851e878aedb30534 100644 --- a/reviewboard/scmtools/forms.py +++ b/reviewboard/scmtools/forms.py @@ -12,14 +12,14 @@ from reviewboard.hostingsvcs.errors import AuthorizationError from reviewboard.hostingsvcs.models import HostingServiceAccount from reviewboard.hostingsvcs.service import get_hosting_services, \ get_hosting_service -from reviewboard.scmtools import sshutils from reviewboard.scmtools.errors import AuthenticationError, \ - BadHostKeyError, \ - UnknownHostKeyError, \ UnverifiedCertificateError from reviewboard.scmtools.models import Repository, Tool from reviewboard.site.models import LocalSite from reviewboard.site.validation import validate_review_groups, validate_users +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.errors import BadHostKeyError, \ + UnknownHostKeyError class RepositoryForm(forms.ModelForm): @@ -242,8 +242,9 @@ class RepositoryForm(forms.ModelForm): # Get the current SSH public key that would be used for repositories, # if one has been created. - self.public_key = \ - sshutils.get_public_key(sshutils.get_user_key(self.local_site_name)) + self.ssh_client = SSHClient(namespace=self.local_site_name) + self.public_key = self.ssh_client.get_public_key( + self.ssh_client.get_user_key()) if self.instance: self._populate_hosting_service_fields() @@ -887,10 +888,9 @@ class RepositoryForm(forms.ModelForm): except BadHostKeyError, e: if self.cleaned_data['trust_host']: try: - sshutils.replace_host_key(e.hostname, - e.raw_expected_key, - e.raw_key, - self.local_site_name) + self.ssh_client.replace_host_key(e.hostname, + e.raw_expected_key, + e.raw_key) except IOError, e: raise forms.ValidationError(e) else: @@ -899,8 +899,7 @@ class RepositoryForm(forms.ModelForm): except UnknownHostKeyError, e: if self.cleaned_data['trust_host']: try: - sshutils.add_host_key(e.hostname, e.raw_key, - self.local_site_name) + self.ssh_client.add_host_key(e.hostname, e.raw_key) except IOError, e: raise forms.ValidationError(e) else: diff --git a/reviewboard/scmtools/git.py b/reviewboard/scmtools/git.py index 57385cca80a7e86de498fcee9b91966b4687a6bf..07506e5d3e1b51ebb69296eee83776843197d8ac 100644 --- a/reviewboard/scmtools/git.py +++ b/reviewboard/scmtools/git.py @@ -14,12 +14,12 @@ from django.utils.translation import ugettext_lazy as _ from djblets.util.filesystem import is_exe_in_path from reviewboard.diffviewer.parser import DiffParser, DiffParserError, File -from reviewboard.scmtools import sshutils from reviewboard.scmtools.core import SCMClient, SCMTool, HEAD, PRE_CREATION from reviewboard.scmtools.errors import FileNotFoundError, \ InvalidRevisionFormatError, \ RepositoryNotFoundError, \ SCMError +from reviewboard.ssh import utils as sshutils GIT_DIFF_EMPTY_CHANGESET_SIZE = 3 diff --git a/reviewboard/scmtools/sshutils.py b/reviewboard/scmtools/sshutils.py deleted file mode 100644 index 687ecb30afd02ecce12aec6d956aedc31de5a83e..0000000000000000000000000000000000000000 --- a/reviewboard/scmtools/sshutils.py +++ /dev/null @@ -1,395 +0,0 @@ -import logging -import os -import urlparse - -from django.utils.translation import ugettext_lazy as _ -import paramiko - -from reviewboard.scmtools.errors import AuthenticationError, \ - BadHostKeyError, SCMError, \ - UnknownHostKeyError, \ - UnsupportedSSHKeyError - - -# A list of known SSH URL schemes. -ssh_uri_schemes = ["ssh", "sftp"] - -urlparse.uses_netloc.extend(ssh_uri_schemes) - -_ssh_dir = None - - -class RaiseUnknownHostKeyPolicy(paramiko.MissingHostKeyPolicy): - """A Paramiko policy that raises UnknownHostKeyError for missing keys.""" - def missing_host_key(self, client, hostname, key): - raise UnknownHostKeyError(hostname, key) - - -class MakeSSHDirError(IOError): - def __init__(self, dirname): - IOError.__init__(_("Unable to create directory %(dirname)s, which is " - "needed for the SSH host keys. Create this " - "directory, set the web server's user as the " - "the owner, and make it writable only by that " - "user.") % { - 'dirname': dirname, - }) - - -def humanize_key(key): - """Returns a human-readable key as a series of hex characters.""" - return ':'.join(["%02x" % ord(c) for c in key.get_fingerprint()]) - - -def set_ssh_dir(path): - """Sets the SSH directory to use. - - This is mostly intended for unit tests. - """ - global _ssh_dir - _ssh_dir = path - - -def get_ssh_dir(local_site_name=None, ssh_dir_name=None): - """Returns the path to the SSH directory on the system. - - By default, this will attempt to find either a .ssh or ssh directory. - If ``ssh_dir_name`` is specified, the search will be skipped, and we'll - use that name instead. - """ - global _ssh_dir - path = _ssh_dir - - if not _ssh_dir or ssh_dir_name: - path = os.path.expanduser('~') - - if not ssh_dir_name: - ssh_dir_name = '.ssh' - - for name in ('.ssh', 'ssh'): - if os.path.exists(os.path.join(path, name)): - ssh_dir_name = name - break - - path = os.path.join(path, ssh_dir_name) - - if not ssh_dir_name: - _ssh_dir = path - - if local_site_name: - return os.path.join(path, local_site_name) - else: - return path - - -def get_host_keys_filename(local_site_name=None): - """Returns the path to the known host keys file.""" - return os.path.join(get_ssh_dir(local_site_name), 'known_hosts') - - -def get_user_key(local_site_name=None): - """Returns the keypair of the user running Review Board. - - This will be an instance of :py:mod:`paramiko.PKey`, representing - a DSS or RSA key, as long as one exists. Otherwise, it may return None. - """ - keyfiles = [] - - for cls, filename in ((paramiko.RSAKey, 'id_rsa'), - (paramiko.DSSKey, 'id_dsa')): - # Paramiko looks in ~/.ssh and ~/ssh, depending on the platform, - # so check both. - for sshdir in ('.ssh', 'ssh'): - path = os.path.join(get_ssh_dir(local_site_name, sshdir), - filename) - - if os.path.isfile(path): - keyfiles.append((cls, path)) - - for cls, keyfile in keyfiles: - try: - return cls.from_private_key_file(keyfile) - except paramiko.SSHException, e: - logging.error('SSH: Unknown error accessing local key file %s: %s' - % (keyfile, e)) - except paramiko.PasswordRequiredException, e: - logging.error('SSH: Unable to access password protected key file ' - '%s: %s' % (keyfile, e)) - except IOError, e: - logging.error('SSH: Error reading local key file %s: %s' - % (keyfile, e)) - - return None - - -def get_public_key(key): - """Returns the public key portion of an SSH key. - - This will be formatted for display. - """ - public_key = '' - - if key: - base64 = key.get_base64() - - # TODO: Move this wrapping logic into a common templatetag. - for i in range(0, len(base64), 64): - public_key += base64[i:i + 64] + '\n' - - return public_key - - -def is_key_authorized(key): - """Returns whether or not a public key is currently authorized.""" - authorized = False - public_key = key.get_base64() - - try: - filename = os.path.join(get_ssh_dir(), 'authorized_keys') - fp = open(filename, 'r') - - for line in fp.xreadlines(): - try: - authorized_key = line.split()[1] - except ValueError: - continue - except IndexError: - continue - - if authorized_key == public_key: - authorized = True - break - - fp.close() - except IOError: - pass - - return authorized - - -def ensure_ssh_dir(local_site_name=None): - """Ensures the existance of the .ssh directory. - - If the directory doesn't exist, it will be created. - The full path to the directory will be returned. - - Callers are expected to handle any exceptions. This may raise - IOError for any problems in creating the directory. - """ - sshdir = get_ssh_dir(local_site_name) - - if local_site_name: - # The parent will be the .ssh dir. - parent = os.path.dirname(sshdir) - - if not os.path.exists(parent): - try: - os.mkdir(parent, 0700) - except OSError: - raise MakeSSHDirError(parent) - - if not os.path.exists(sshdir): - try: - os.mkdir(sshdir, 0700) - except OSError: - raise MakeSSHDirError(sshdir) - - return sshdir - - -def generate_user_key(local_site_name=None): - """Generates a new RSA keypair for the user running Review Board. - - This will store the new key in :file:`$HOME/.ssh/id_rsa` and return the - resulting key as an instance of :py:mod:`paramiko.RSAKey`. - - If a key already exists in the id_rsa file, it's returned instead. - - Callers are expected to handle any exceptions. This may raise - IOError for any problems in writing the key file, or - paramiko.SSHException for any other problems. - """ - sshdir = ensure_ssh_dir(local_site_name) - filename = os.path.join(sshdir, 'id_rsa') - - if os.path.isfile(filename): - return get_user_key(local_site_name) - - key = paramiko.RSAKey.generate(2048) - key.write_private_key_file(filename) - return key - - -def import_user_key(keyfile, local_site_name=None): - """Imports an uploaded key file into Review Board. - - ``keyfile`` is expected to be an ``UploadedFile`` or a paramiko - ``KeyFile``. If this is a valid key file, it will be saved in - :file:`$HOME/.ssh/`` and the resulting key as an instance of - :py:mod:`paramiko.RSAKey` will be returned. - - If a key of this name already exists, it will be overwritten. - - Callers are expected to handle any exceptions. This may raise - IOError for any problems in writing the key file, or - paramiko.SSHException for any other problems. - - This will raise UnsupportedSSHKeyError if the uploaded key is not - a supported type. - """ - sshdir = ensure_ssh_dir(local_site_name) - - # Try to find out what key this is. - for cls, filename in ((paramiko.RSAKey, 'id_rsa'), - (paramiko.DSSKey, 'id_dsa')): - try: - key = None - - if not isinstance(keyfile, paramiko.PKey): - keyfile.seek(0) - key = cls.from_private_key(keyfile) - elif isinstance(keyfile, cls): - key = keyfile - except paramiko.SSHException: - # We don't have more detailed info than this, but most - # likely, it's not a valid key. Skip to the next. - continue - - if key: - key.write_private_key_file(os.path.join(sshdir, filename)) - return key - - raise UnsupportedSSHKeyError() - - -def is_ssh_uri(url): - """Returns whether or not a URL represents an SSH connection.""" - return urlparse.urlparse(url)[0] in ssh_uri_schemes - - -def get_ssh_client(local_site_name=None): - """Returns a new paramiko.SSHClient with all known host keys added.""" - client = paramiko.SSHClient() - filename = get_host_keys_filename(local_site_name) - - if os.path.exists(filename): - client.load_host_keys(filename) - - return client - - -def add_host_key(hostname, key, local_site_name=None): - """Adds a host key to the known hosts file.""" - ensure_ssh_dir(local_site_name) - filename = get_host_keys_filename(local_site_name) - - try: - fp = open(filename, 'a') - fp.write('%s %s %s\n' % (hostname, key.get_name(), key.get_base64())) - fp.close() - except IOError, e: - raise IOError( - _('Unable to write host keys file %(filename)s: %(error)s') % { - 'filename': filename, - 'error': e, - }) - - -def replace_host_key(hostname, old_key, new_key, local_site_name=None): - """Replaces a host key in the known hosts file with another. - - This is used for replacing host keys that have changed. - """ - filename = get_host_keys_filename(local_site_name) - - if not os.path.exists(filename): - add_host_key(hostname, new_key, local_site_name) - return - - try: - fp = open(filename, 'r') - lines = fp.readlines() - fp.close() - - old_key_base64 = old_key.get_base64() - except IOError, e: - raise IOError( - _('Unable to read host keys file %(filename)s: %(error)s') % { - 'filename': filename, - 'error': e, - }) - - try: - fp = open(filename, 'w') - - for line in lines: - parts = line.strip().split(" ") - - if parts[-1] == old_key_base64: - parts[-1] = new_key.get_base64() - - fp.write(' '.join(parts) + '\n') - - fp.close() - except IOError, e: - raise IOError( - _('Unable to write host keys file %(filename)s: %(error)s') % { - 'filename': filename, - 'error': e, - }) - - -def check_host(hostname, username=None, password=None, local_site_name=None): - """ - Checks if we can connect to a host with a known key. - - This will raise an exception if we cannot connect to the host. The - exception will be one of BadHostKeyError, UnknownHostKeyError, or - SCMError. - """ - from django.conf import settings - - client = get_ssh_client(local_site_name) - client.set_missing_host_key_policy(RaiseUnknownHostKeyPolicy()) - - kwargs = {} - - # We normally want to notify on unknown host keys, but not when running - # unit tests. - if getattr(settings, 'RUNNING_TEST', False): - client.set_missing_host_key_policy(paramiko.WarningPolicy()) - kwargs['allow_agent'] = False - - try: - client.connect(hostname, username=username, password=password, - pkey=get_user_key(local_site_name), **kwargs) - except paramiko.BadHostKeyException, e: - raise BadHostKeyError(e.hostname, e.key, e.expected_key) - except paramiko.AuthenticationException, e: - # Some AuthenticationException instances have allowed_types set, - # and some don't. - allowed_types = getattr(e, 'allowed_types', []) - - if 'publickey' in allowed_types: - key = get_user_key(local_site_name) - else: - key = None - - raise AuthenticationError(allowed_types=allowed_types, user_key=key) - except paramiko.SSHException, e: - if str(e) == 'No authentication methods available': - raise AuthenticationError - else: - raise SCMError(unicode(e)) - - -def register_rbssh(envvar): - """Registers rbssh in an environment variable. - - This is a convenience method for making sure that rbssh is set properly - in the environment for different tools. In some cases, we need to - specifically place it in the system environment using ``os.putenv``, - while in others (Mercurial, Bazaar), we need to place it in ``os.environ``. - """ - os.putenv(envvar, 'rbssh') - os.environ[envvar] = 'rbssh' diff --git a/reviewboard/scmtools/svn.py b/reviewboard/scmtools/svn.py index 8909a6d65108e8eb6e6742eaabf307cbdd9083a6..0805da3e934d01b0d231b5de923b6d73a77251d2 100644 --- a/reviewboard/scmtools/svn.py +++ b/reviewboard/scmtools/svn.py @@ -12,7 +12,6 @@ except ImportError: from django.utils.translation import ugettext as _ from reviewboard.diffviewer.parser import DiffParser -from reviewboard.scmtools import sshutils from reviewboard.scmtools.certs import Certificate from reviewboard.scmtools.core import SCMTool, HEAD, PRE_CREATION, UNKNOWN from reviewboard.scmtools.errors import AuthenticationError, \ @@ -20,6 +19,7 @@ from reviewboard.scmtools.errors import AuthenticationError, \ RepositoryNotFoundError, \ SCMError, \ UnverifiedCertificateError +from reviewboard.ssh import utils as sshutils # Register these URI schemes so we can handle them properly. diff --git a/reviewboard/scmtools/tests.py b/reviewboard/scmtools/tests.py index f76e34df2fa9b31546ee1c1f57b606f5a82be82e..4d8b3ca797ad4452f17833cd582454bb9b095168 100644 --- a/reviewboard/scmtools/tests.py +++ b/reviewboard/scmtools/tests.py @@ -1,9 +1,6 @@ import errno import imp import os -import nose -import paramiko -import shutil import socket import tempfile try: @@ -14,6 +11,7 @@ except ImportError: from django.contrib.auth.models import AnonymousUser, User from django.test import TestCase as DjangoTestCase from djblets.util.filesystem import is_exe_in_path +import nose try: imp.find_module("P4") from P4 import P4Error @@ -24,7 +22,6 @@ from reviewboard.diffviewer.diffutils import patch from reviewboard.diffviewer.parser import DiffParserError from reviewboard.hostingsvcs.models import HostingServiceAccount from reviewboard.reviews.models import Group -from reviewboard.scmtools import sshutils from reviewboard.scmtools.core import HEAD, PRE_CREATION, ChangeSet, Revision from reviewboard.scmtools.errors import SCMError, FileNotFoundError, \ RepositoryNotFoundError, \ @@ -34,38 +31,31 @@ from reviewboard.scmtools.git import ShortSHA1Error from reviewboard.scmtools.models import Repository, Tool from reviewboard.scmtools.perforce import STunnelProxy, STUNNEL_SERVER from reviewboard.site.models import LocalSite +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.tests import SSHTestCase -class SCMTestCase(DjangoTestCase): +class SCMTestCase(SSHTestCase): + ssh_client = None _can_test_ssh = None def setUp(self): - self.old_home = os.getenv('HOME') - self.tempdir = None + super(SCMTestCase, self).setUp() self.tool = None - os.environ['RBSSH_ALLOW_AGENT'] = '0' - - def tearDown(self): - self._set_home(self.old_home) - - if self.tempdir: - shutil.rmtree(self.tempdir) - def _set_home(self, homedir): - os.environ['HOME'] = homedir - - def _check_can_test_ssh(self): + def _check_can_test_ssh(self, local_site_name=None): if SCMTestCase._can_test_ssh is None: - key = sshutils.get_user_key() - - SCMTestCase._can_test_ssh = (key is not None and - sshutils.is_key_authorized(key)) + SCMTestCase.ssh_client = SSHClient() + key = self.ssh_client.get_user_key() + SCMTestCase._can_test_ssh = \ + key is not None and self.ssh_client.is_key_authorized(key) if not SCMTestCase._can_test_ssh: raise nose.SkipTest( "Cannot perform SSH access tests. The local user's SSH " "public key must be in the %s file and SSH must be enabled." - % os.path.join(sshutils.get_ssh_dir(), 'authorized_keys')) + % os.path.join(self.ssh_client.get_ssh_dir(), + 'authorized_keys')) def _test_ssh(self, repo_path, filename=None): self._check_can_test_ssh() @@ -94,7 +84,7 @@ class SCMTestCase(DjangoTestCase): self._check_can_test_ssh() # Get the user's .ssh key, for use in the tests - user_key = sshutils.get_user_key() + user_key = self.ssh_client.get_user_key() self.assertNotEqual(user_key, None) # Switch to a new SSH directory. @@ -102,17 +92,17 @@ class SCMTestCase(DjangoTestCase): sshdir = os.path.join(self.tempdir, '.ssh') self._set_home(self.tempdir) - self.assertEqual(sshdir, sshutils.get_ssh_dir()) + self.assertEqual(sshdir, self.ssh_client.get_ssh_dir()) self.assertFalse(os.path.exists(os.path.join(sshdir, 'id_rsa'))) self.assertFalse(os.path.exists(os.path.join(sshdir, 'id_dsa'))) - self.assertEqual(sshutils.get_user_key(), None) + self.assertEqual(self.ssh_client.get_user_key(), None) tool_class = self.repository.tool # Make sure we aren't using the old SSH key. We want auth errors. repo = Repository(name='SSH Test', path=repo_path, tool=tool_class) tool = repo.get_scmtool() - self.assertRaises(sshutils.AuthenticationError, + self.assertRaises(AuthenticationError, lambda: tool.check_repository(repo_path)) if filename: @@ -127,10 +117,11 @@ class SCMTestCase(DjangoTestCase): local_site=local_site) tool = repo.get_scmtool() - self.assertEqual(sshutils.get_ssh_dir(local_site_name), + ssh_client = SSHClient(namespace=local_site_name) + self.assertEqual(ssh_client.get_ssh_dir(), os.path.join(sshdir, local_site_name)) - sshutils.import_user_key(user_key, local_site_name) - self.assertEqual(sshutils.get_user_key(local_site_name), user_key) + ssh_client.import_user_key(user_key) + self.assertEqual(ssh_client.get_user_key(), user_key) # Make sure we can verify the repository and access files. tool.check_repository(repo_path, local_site_name=local_site_name) @@ -155,99 +146,6 @@ class CoreTests(DjangoTestCase): self.assert_(len(cs.files) == 0) -class SSHUtilsTests(SCMTestCase): - """Unit tests for sshutils.""" - def setUp(self): - super(SSHUtilsTests, self).setUp() - - self.tempdir = tempfile.mkdtemp(prefix='rb-tests-home-') - - def test_get_ssh_dir_with_dot_ssh(self): - """Testing sshutils.get_ssh_dir with ~/.ssh""" - self._set_home(self.tempdir) - sshdir = os.path.join(self.tempdir, '.ssh') - self.assertEqual(sshutils.get_ssh_dir(), sshdir) - - def test_get_ssh_dir_with_ssh(self): - """Testing sshutils.get_ssh_dir with ~/ssh""" - self._set_home(self.tempdir) - sshdir = os.path.join(self.tempdir, 'ssh') - os.mkdir(sshdir, 0700) - self.assertEqual(sshutils.get_ssh_dir(), sshdir) - - def test_get_ssh_dir_with_dot_ssh_and_localsite(self): - """Testing sshutils.get_ssh_dir with ~/.ssh and localsite""" - self._set_home(self.tempdir) - sshdir = os.path.join(self.tempdir, '.ssh', 'site-1') - self.assertEqual(sshutils.get_ssh_dir(local_site_name='site-1'), sshdir) - - def test_get_ssh_dir_with_ssh_and_localsite(self): - """Testing sshutils.get_ssh_dir with ~/ssh and localsite""" - self._set_home(self.tempdir) - sshdir = os.path.join(self.tempdir, 'ssh') - os.mkdir(sshdir, 0700) - sshdir = os.path.join(sshdir, 'site-1') - self.assertEqual(sshutils.get_ssh_dir(local_site_name='site-1'), sshdir) - - def test_generate_user_key(self, local_site_name=None): - """Testing sshutils.generate_user_key""" - self._set_home(self.tempdir) - key = sshutils.generate_user_key(local_site_name) - key_file = os.path.join(sshutils.get_ssh_dir(local_site_name), 'id_rsa') - self.assertTrue(os.path.exists(key_file)) - self.assertEqual(sshutils.get_user_key(local_site_name), key) - - def test_generate_user_key_with_localsite(self): - """Testing sshutils.generate_user_key with localsite""" - self.test_generate_user_key('site-1') - - def test_add_host_key(self, local_site_name=None): - """Testing sshutils.add_host_key""" - self._set_home(self.tempdir) - key = paramiko.RSAKey.generate(2048) - sshutils.add_host_key('example.com', key, local_site_name) - - known_hosts_file = sshutils.get_host_keys_filename(local_site_name) - self.assertTrue(os.path.exists(known_hosts_file)) - - f = open(known_hosts_file, 'r') - lines = f.readlines() - f.close() - - self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].split(), - ['example.com', key.get_name(), key.get_base64()]) - - def test_add_host_key_with_localsite(self): - """Testing sshutils.add_host_key with localsite""" - self.test_add_host_key('site-1') - - def test_replace_host_key(self, local_site_name=None): - """Testing sshutils.replace_host_key""" - self._set_home(self.tempdir) - key = paramiko.RSAKey.generate(2048) - sshutils.add_host_key('example.com', key, local_site_name) - - new_key = paramiko.RSAKey.generate(2048) - sshutils.replace_host_key('example.com', key, new_key, local_site_name) - - known_hosts_file = sshutils.get_host_keys_filename(local_site_name) - self.assertTrue(os.path.exists(known_hosts_file)) - - f = open(known_hosts_file, 'r') - lines = f.readlines() - f.close() - - self.assertEqual(len(lines), 1) - self.assertEqual(lines[0].split(), - ['example.com', new_key.get_name(), - new_key.get_base64()]) - - def test_replace_host_key_with_localsite(self): - """Testing sshutils.replace_host_key with localsite""" - self.test_replace_host_key('site-1') - - class BZRTests(SCMTestCase): """Unit tests for bzr.""" fixtures = ['test_scmtools.json'] diff --git a/reviewboard/settings.py b/reviewboard/settings.py index 2502d947d06faa95e63399a650eb8e750d040d33..ce62a490993f2ded76395e954e46e340089b3d6e 100644 --- a/reviewboard/settings.py +++ b/reviewboard/settings.py @@ -118,6 +118,7 @@ RB_BUILTIN_APPS = [ 'reviewboard.reviews', 'reviewboard.scmtools', 'reviewboard.site', + 'reviewboard.ssh', 'reviewboard.webapi', ] RB_EXTRA_APPS = [] diff --git a/reviewboard/ssh/client.py b/reviewboard/ssh/client.py new file mode 100644 index 0000000000000000000000000000000000000000..d4bf08a0547126519117f3c39d526da5ac3fc928 --- /dev/null +++ b/reviewboard/ssh/client.py @@ -0,0 +1,282 @@ +import logging +import os + +from django.utils.translation import ugettext_lazy as _ +import paramiko + +from reviewboard.ssh.errors import MakeSSHDirError, UnsupportedSSHKeyError + + +class SSHClient(paramiko.SSHClient): + _ssh_dir = None + + def __init__(self, namespace=None): + super(SSHClient, self).__init__() + + self.namespace = namespace + + filename = self.get_host_keys_filename() + + if os.path.exists(filename): + self.load_host_keys(filename) + + def get_host_keys_filename(self): + """Returns the path to the known host keys file.""" + return os.path.join(self.get_ssh_dir(), 'known_hosts') + + def get_ssh_dir(self, ssh_dir_name=None): + """Returns the path to the SSH directory on the system. + + By default, this will attempt to find either a .ssh or ssh directory. + If ``ssh_dir_name`` is specified, the search will be skipped, and we'll + use that name instead. + """ + path = SSHClient._ssh_dir + + if not SSHClient._ssh_dir or ssh_dir_name: + path = os.path.expanduser('~') + + if not ssh_dir_name: + ssh_dir_name = '.ssh' + + for name in ('.ssh', 'ssh'): + if os.path.exists(os.path.join(path, name)): + ssh_dir_name = name + break + + path = os.path.join(path, ssh_dir_name) + + if not ssh_dir_name: + SSHClient._ssh_dir = path + + if self.namespace: + return os.path.join(path, self.namespace) + else: + return path + + def get_user_key(self): + """Returns the keypair of the user running Review Board. + + This will be an instance of :py:mod:`paramiko.PKey`, representing + a DSS or RSA key, as long as one exists. Otherwise, it may return None. + """ + keyfiles = [] + + for cls, filename in ((paramiko.RSAKey, 'id_rsa'), + (paramiko.DSSKey, 'id_dsa')): + # Paramiko looks in ~/.ssh and ~/ssh, depending on the platform, + # so check both. + for sshdir in ('.ssh', 'ssh'): + path = os.path.join(self.get_ssh_dir(sshdir), filename) + + if os.path.isfile(path): + keyfiles.append((cls, path)) + + for cls, keyfile in keyfiles: + try: + return cls.from_private_key_file(keyfile) + except paramiko.SSHException, e: + logging.error('SSH: Unknown error accessing local key file ' + '%s: %s' + % (keyfile, e)) + except paramiko.PasswordRequiredException, e: + logging.error('SSH: Unable to access password protected ' + 'key file %s: %s' % (keyfile, e)) + except IOError, e: + logging.error('SSH: Error reading local key file %s: %s' + % (keyfile, e)) + + return None + + def get_public_key(self, key): + """Returns the public key portion of an SSH key. + + This will be formatted for display. + """ + public_key = '' + + if key: + base64 = key.get_base64() + + # TODO: Move this wrapping logic into a common templatetag. + for i in range(0, len(base64), 64): + public_key += base64[i:i + 64] + '\n' + + return public_key + + def is_key_authorized(self, key): + """Returns whether or not a public key is currently authorized.""" + authorized = False + public_key = key.get_base64() + + try: + filename = os.path.join(self.get_ssh_dir(), 'authorized_keys') + fp = open(filename, 'r') + + for line in fp.xreadlines(): + try: + authorized_key = line.split()[1] + except (ValueError, IndexError): + continue + + if authorized_key == public_key: + authorized = True + break + + fp.close() + except IOError: + pass + + return authorized + + def ensure_ssh_dir(self): + """Ensures the existance of the .ssh directory. + + If the directory doesn't exist, it will be created. + The full path to the directory will be returned. + + Callers are expected to handle any exceptions. This may raise + IOError for any problems in creating the directory. + """ + sshdir = self.get_ssh_dir() + + if self.namespace: + # The parent will be the .ssh dir. + parent = os.path.dirname(sshdir) + + if not os.path.exists(parent): + try: + os.mkdir(parent, 0700) + except OSError: + raise MakeSSHDirError(parent) + + if not os.path.exists(sshdir): + try: + os.mkdir(sshdir, 0700) + except OSError: + raise MakeSSHDirError(sshdir) + + return sshdir + + def generate_user_key(self): + """Generates a new RSA keypair for the user running Review Board. + + This will store the new key in :file:`$HOME/.ssh/id_rsa` and return the + resulting key as an instance of :py:mod:`paramiko.RSAKey`. + + If a key already exists in the id_rsa file, it's returned instead. + + Callers are expected to handle any exceptions. This may raise + IOError for any problems in writing the key file, or + paramiko.SSHException for any other problems. + """ + sshdir = self.ensure_ssh_dir() + filename = os.path.join(sshdir, 'id_rsa') + + if os.path.isfile(filename): + return self.get_user_key() + + key = paramiko.RSAKey.generate(2048) + key.write_private_key_file(filename) + return key + + def import_user_key(self, keyfile): + """Imports an uploaded key file into Review Board. + + ``keyfile`` is expected to be an ``UploadedFile`` or a paramiko + ``KeyFile``. If this is a valid key file, it will be saved in + :file:`$HOME/.ssh/`` and the resulting key as an instance of + :py:mod:`paramiko.RSAKey` will be returned. + + If a key of this name already exists, it will be overwritten. + + Callers are expected to handle any exceptions. This may raise + IOError for any problems in writing the key file, or + paramiko.SSHException for any other problems. + + This will raise UnsupportedSSHKeyError if the uploaded key is not + a supported type. + """ + sshdir = self.ensure_ssh_dir() + + # Try to find out what key this is. + for cls, filename in ((paramiko.RSAKey, 'id_rsa'), + (paramiko.DSSKey, 'id_dsa')): + try: + key = None + + if not isinstance(keyfile, paramiko.PKey): + keyfile.seek(0) + key = cls.from_private_key(keyfile) + elif isinstance(keyfile, cls): + key = keyfile + except paramiko.SSHException: + # We don't have more detailed info than this, but most + # likely, it's not a valid key. Skip to the next. + continue + + if key: + key.write_private_key_file(os.path.join(sshdir, filename)) + return key + + raise UnsupportedSSHKeyError() + + def add_host_key(self, hostname, key): + """Adds a host key to the known hosts file.""" + self.ensure_ssh_dir() + filename = self.get_host_keys_filename() + + try: + fp = open(filename, 'a') + fp.write('%s %s %s\n' % (hostname, key.get_name(), + key.get_base64())) + fp.close() + except IOError, e: + raise IOError( + _('Unable to write host keys file %(filename)s: %(error)s') % { + 'filename': filename, + 'error': e, + }) + + def replace_host_key(self, hostname, old_key, new_key): + """Replaces a host key in the known hosts file with another. + + This is used for replacing host keys that have changed. + """ + filename = self.get_host_keys_filename() + + if not os.path.exists(filename): + self.add_host_key(hostname, new_key) + return + + try: + fp = open(filename, 'r') + lines = fp.readlines() + fp.close() + + old_key_base64 = old_key.get_base64() + except IOError, e: + raise IOError( + _('Unable to read host keys file %(filename)s: %(error)s') % { + 'filename': filename, + 'error': e, + }) + + try: + fp = open(filename, 'w') + + for line in lines: + parts = line.strip().split(" ") + + if parts[-1] == old_key_base64: + parts[-1] = new_key.get_base64() + + fp.write(' '.join(parts) + '\n') + + fp.close() + except IOError, e: + raise IOError( + _('Unable to write host keys file %(filename)s: %(error)s') % { + 'filename': filename, + 'error': e, + }) diff --git a/reviewboard/ssh/errors.py b/reviewboard/ssh/errors.py new file mode 100644 index 0000000000000000000000000000000000000000..2409789954c535d22f3891585625f47b56e8208e --- /dev/null +++ b/reviewboard/ssh/errors.py @@ -0,0 +1,97 @@ +import socket + +from django.utils.translation import ugettext as _ +from djblets.util.humanize import humanize_list + + +class SSHError(Exception): + """An SSH-related error.""" + pass + + +class MakeSSHDirError(IOError, SSHError): + def __init__(self, dirname): + IOError.__init__(_("Unable to create directory %(dirname)s, which is " + "needed for the SSH host keys. Create this " + "directory, set the web server's user as the " + "the owner, and make it writable only by that " + "user.") % { + 'dirname': dirname, + }) + + +class SSHAuthenticationError(SSHError): + """An error representing a failed authentication over SSH. + + This takes a list of SSH authentication types that are allowed. + Primarily, we respond to "password" and "publickey". + + This may also take the user's SSH key that was tried, if any. + """ + def __init__(self, allowed_types=[], msg=None, user_key=None): + if allowed_types: + msg = _('Unable to authenticate against this repository using one ' + 'of the supported authentication types ' + '(%(allowed_types)s).') % { + 'allowed_types': humanize_list(allowed_types), + } + elif not msg: + msg = _('Unable to authenticate against this repository using one ' + 'of the supported authentication types.') + + SSHError.__init__(self, msg) + self.allowed_types = allowed_types + self.user_key = user_key + + +class UnsupportedSSHKeyError(SSHError): + """An error representing an unsupported type of SSH key.""" + def __init__(self): + SSHError.__init__(self, + _('This SSH key is not a valid RSA or DSS key.')) + + +class SSHKeyError(SSHError): + """An error involving a host key on an SSH connection.""" + def __init__(self, hostname, key, message): + from reviewboard.ssh.utils import humanize_key + + SSHError.__init__(self, message) + self.hostname = hostname + self.key = humanize_key(key) + self.raw_key = key + + +class BadHostKeyError(SSHKeyError): + """An error representing a bad or malicious key for an SSH connection.""" + def __init__(self, hostname, key, expected_key): + from reviewboard.ssh.utils import humanize_key + + SSHKeyError.__init__( + self, hostname, key, + _("Warning! The host key for server %(hostname)s does not match " + "the expected key.\n" + "It's possible that someone is performing a man-in-the-middle " + "attack. It's also possible that the RSA host key has just " + "been changed. Please contact your system administrator if " + "you're not sure. Do not accept this host key unless you're " + "certain it's safe!") + % { + 'hostname': hostname, + 'ip_address': socket.gethostbyname(hostname), + }) + self.expected_key = humanize_key(expected_key) + self.raw_expected_key = expected_key + + +class UnknownHostKeyError(SSHKeyError): + """An error representing an unknown host key for an SSH connection.""" + def __init__(self, hostname, key): + SSHKeyError.__init__( + self, hostname, key, + _("The authenticity of the host '%(hostname)s (%(ip)s)' " + "couldn't be determined.") % { + 'hostname': hostname, + 'ip': socket.gethostbyname(hostname), + } + ) diff --git a/reviewboard/ssh/policy.py b/reviewboard/ssh/policy.py new file mode 100644 index 0000000000000000000000000000000000000000..b636831af9ca899bff062972d8ba6f7e938800f8 --- /dev/null +++ b/reviewboard/ssh/policy.py @@ -0,0 +1,9 @@ +import paramiko + +from reviewboard.ssh.errors import UnknownHostKeyError + + +class RaiseUnknownHostKeyPolicy(paramiko.MissingHostKeyPolicy): + """A Paramiko policy that raises UnknownHostKeyError for missing keys.""" + def missing_host_key(self, client, hostname, key): + raise UnknownHostKeyError(hostname, key) diff --git a/reviewboard/ssh/tests.py b/reviewboard/ssh/tests.py new file mode 100644 index 0000000000000000000000000000000000000000..0c02e56eaaa9076146831485c436c7f846421466 --- /dev/null +++ b/reviewboard/ssh/tests.py @@ -0,0 +1,131 @@ +import os +import shutil +import tempfile + +from django.test import TestCase as DjangoTestCase +import paramiko + +from reviewboard.ssh.client import SSHClient + + +class SSHTestCase(DjangoTestCase): + def setUp(self): + self.old_home = os.getenv('HOME') + self.tempdir = None + os.environ['RBSSH_ALLOW_AGENT'] = '0' + + def tearDown(self): + self._set_home(self.old_home) + + if self.tempdir: + shutil.rmtree(self.tempdir) + + def _set_home(self, homedir): + os.environ['HOME'] = homedir + + +class SSHClientTests(SSHTestCase): + """Unit tests for SSHClient.""" + def setUp(self): + super(SSHClientTests, self).setUp() + + self.tempdir = tempfile.mkdtemp(prefix='rb-tests-home-') + + def test_get_ssh_dir_with_dot_ssh(self): + """Testing SSHClient.get_ssh_dir with ~/.ssh""" + self._set_home(self.tempdir) + sshdir = os.path.join(self.tempdir, '.ssh') + + client = SSHClient() + self.assertEqual(client.get_ssh_dir(), sshdir) + + def test_get_ssh_dir_with_ssh(self): + """Testing SSHClient.get_ssh_dir with ~/ssh""" + self._set_home(self.tempdir) + sshdir = os.path.join(self.tempdir, 'ssh') + os.mkdir(sshdir, 0700) + + client = SSHClient() + self.assertEqual(client.get_ssh_dir(), sshdir) + + def test_get_ssh_dir_with_dot_ssh_and_localsite(self): + """Testing SSHClient.get_ssh_dir with ~/.ssh and localsite""" + self._set_home(self.tempdir) + sshdir = os.path.join(self.tempdir, '.ssh', 'site-1') + + client = SSHClient(namespace='site-1') + self.assertEqual(client.get_ssh_dir(), sshdir) + + def test_get_ssh_dir_with_ssh_and_localsite(self): + """Testing SSHClient.get_ssh_dir with ~/ssh and localsite""" + self._set_home(self.tempdir) + sshdir = os.path.join(self.tempdir, 'ssh') + os.mkdir(sshdir, 0700) + sshdir = os.path.join(sshdir, 'site-1') + + client = SSHClient(namespace='site-1') + self.assertEqual(client.get_ssh_dir(), sshdir) + + def test_generate_user_key(self, namespace=None): + """Testing SSHClient.generate_user_key""" + self._set_home(self.tempdir) + + client = SSHClient(namespace=namespace) + key = client.generate_user_key() + key_file = os.path.join(client.get_ssh_dir(), 'id_rsa') + self.assertTrue(os.path.exists(key_file)) + self.assertEqual(client.get_user_key(), key) + + def test_generate_user_key_with_localsite(self): + """Testing SSHClient.generate_user_key with localsite""" + self.test_generate_user_key('site-1') + + def test_add_host_key(self, namespace=None): + """Testing SSHClient.add_host_key""" + self._set_home(self.tempdir) + client = SSHClient(namespace=namespace) + + key = paramiko.RSAKey.generate(2048) + client.add_host_key('example.com', key) + + known_hosts_file = client.get_host_keys_filename() + self.assertTrue(os.path.exists(known_hosts_file)) + + f = open(known_hosts_file, 'r') + lines = f.readlines() + f.close() + + self.assertEqual(len(lines), 1) + self.assertEqual(lines[0].split(), + ['example.com', key.get_name(), key.get_base64()]) + + def test_add_host_key_with_localsite(self): + """Testing SSHClient.add_host_key with localsite""" + self.test_add_host_key('site-1') + + def test_replace_host_key(self, namespace=None): + """Testing SSHClient.replace_host_key""" + self._set_home(self.tempdir) + client = SSHClient(namespace=namespace) + + key = paramiko.RSAKey.generate(2048) + client.add_host_key('example.com', key) + + new_key = paramiko.RSAKey.generate(2048) + client.replace_host_key('example.com', key, new_key) + + known_hosts_file = client.get_host_keys_filename() + self.assertTrue(os.path.exists(known_hosts_file)) + + f = open(known_hosts_file, 'r') + lines = f.readlines() + f.close() + + self.assertEqual(len(lines), 1) + self.assertEqual(lines[0].split(), + ['example.com', new_key.get_name(), + new_key.get_base64()]) + + def test_replace_host_key_with_localsite(self): + """Testing SSHClient.replace_host_key with localsite""" + self.test_replace_host_key('site-1') diff --git a/reviewboard/ssh/utils.py b/reviewboard/ssh/utils.py new file mode 100644 index 0000000000000000000000000000000000000000..19b12786ce19a901e17b2793fffee6c453c664a5 --- /dev/null +++ b/reviewboard/ssh/utils.py @@ -0,0 +1,81 @@ +import os +import urlparse + +import paramiko + +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.errors import BadHostKeyError, SSHAuthenticationError, \ + SSHError +from reviewboard.ssh.policy import RaiseUnknownHostKeyPolicy + + +# A list of known SSH URL schemes. +ssh_uri_schemes = ["ssh", "sftp"] + +urlparse.uses_netloc.extend(ssh_uri_schemes) + + +def humanize_key(key): + """Returns a human-readable key as a series of hex characters.""" + return ':'.join(["%02x" % ord(c) for c in key.get_fingerprint()]) + + +def is_ssh_uri(url): + """Returns whether or not a URL represents an SSH connection.""" + return urlparse.urlparse(url)[0] in ssh_uri_schemes + + +def check_host(hostname, username=None, password=None, namespace=None): + """ + Checks if we can connect to a host with a known key. + + This will raise an exception if we cannot connect to the host. The + exception will be one of BadHostKeyError, UnknownHostKeyError, or + SCMError. + """ + from django.conf import settings + + client = SSHClient(namespace=namespace) + client.set_missing_host_key_policy(RaiseUnknownHostKeyPolicy()) + + kwargs = {} + + # We normally want to notify on unknown host keys, but not when running + # unit tests. + if getattr(settings, 'RUNNING_TEST', False): + client.set_missing_host_key_policy(paramiko.WarningPolicy()) + kwargs['allow_agent'] = False + + try: + client.connect(hostname, username=username, password=password, + pkey=client.get_user_key(), **kwargs) + except paramiko.BadHostKeyException, e: + raise BadHostKeyError(e.hostname, e.key, e.expected_key) + except paramiko.AuthenticationException, e: + # Some AuthenticationException instances have allowed_types set, + # and some don't. + allowed_types = getattr(e, 'allowed_types', []) + + if 'publickey' in allowed_types: + key = client.get_user_key() + else: + key = None + + raise SSHAuthenticationError(allowed_types=allowed_types, user_key=key) + except paramiko.SSHException, e: + if str(e) == 'No authentication methods available': + raise SSHAuthenticationError + else: + raise SSHError(unicode(e)) + + +def register_rbssh(envvar): + """Registers rbssh in an environment variable. + + This is a convenience method for making sure that rbssh is set properly + in the environment for different tools. In some cases, we need to + specifically place it in the system environment using ``os.putenv``, + while in others (Mercurial, Bazaar), we need to place it in ``os.environ``. + """ + os.putenv(envvar, 'rbssh') + os.environ[envvar] = 'rbssh' diff --git a/reviewboard/webapi/resources.py b/reviewboard/webapi/resources.py index 7e45142c6118c3b61b3bcaa723a79481cb0f77ac..e79b427f87d0adae7b809fd12685986451ad230e 100644 --- a/reviewboard/webapi/resources.py +++ b/reviewboard/webapi/resources.py @@ -51,20 +51,21 @@ from reviewboard.reviews.models import BaseComment, Comment, DiffSet, \ ReviewRequest, ReviewRequestDraft, \ Review, ScreenshotComment, Screenshot, \ FileAttachmentComment -from reviewboard.scmtools import sshutils from reviewboard.scmtools.errors import AuthenticationError, \ - BadHostKeyError, \ ChangeNumberInUseError, \ EmptyChangeSetError, \ FileNotFoundError, \ InvalidChangeNumberError, \ SCMError, \ RepositoryNotFoundError, \ - UnknownHostKeyError, \ UnverifiedCertificateError from reviewboard.scmtools.models import Tool from reviewboard.site.models import LocalSite from reviewboard.site.urlresolvers import local_site_reverse +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.errors import SSHError, \ + BadHostKeyError, \ + UnknownHostKeyError from reviewboard.webapi.decorators import webapi_check_login_required, \ webapi_check_local_site from reviewboard.webapi.encoder import status_to_string, string_to_status @@ -2740,10 +2741,10 @@ class RepositoryResource(WebAPIResource): except BadHostKeyError, e: if trust_host: try: - sshutils.replace_host_key(e.hostname, - e.raw_expected_key, - e.raw_key, - local_site_name) + client = SSHClient(namespace=local_site_name) + client.replace_host_key(e.hostname, + e.raw_expected_key, + e.raw_key) except IOError, e: return SERVER_CONFIG_ERROR, { 'reason': str(e), @@ -2757,8 +2758,8 @@ class RepositoryResource(WebAPIResource): except UnknownHostKeyError, e: if trust_host: try: - sshutils.add_host_key(e.hostname, e.raw_key, - local_site_name) + client = SSHClient(namespace=local_site_name) + client.add_host_key(e.hostname, e.raw_key) except IOError, e: return SERVER_CONFIG_ERROR, { 'reason': str(e), @@ -2796,8 +2797,16 @@ class RepositoryResource(WebAPIResource): return REPO_AUTHENTICATION_ERROR, { 'reason': str(e), } + except SSHError, e: + logging.error('Got unexpected SSHError when checking ' + 'repository: %s' + % e, exc_info=1) + return REPO_INFO_ERROR, { + 'error': str(e), + } except SCMError, e: - logging.error('Got unexpected SCMError when checking repository: %s' + logging.error('Got unexpected SCMError when checking ' + 'repository: %s' % e, exc_info=1) return REPO_INFO_ERROR, { 'error': str(e), @@ -6006,6 +6015,10 @@ class ReviewRequestResource(WebAPIResource): return INVALID_CHANGE_NUMBER except EmptyChangeSetError: return EMPTY_CHANGESET + except SSHError, e: + logging.error("Got unexpected SSHError when creating repository: %s" + % e, exc_info=1) + return REPO_INFO_ERROR except SCMError, e: logging.error("Got unexpected SCMError when creating repository: %s" % e, exc_info=1) diff --git a/reviewboard/webapi/tests.py b/reviewboard/webapi/tests.py index e012d9011064dab751fcb1ea397c4a9aa247c4d4..4c16e9f10d52bfc30758f23497fecfc34da14a39 100644 --- a/reviewboard/webapi/tests.py +++ b/reviewboard/webapi/tests.py @@ -23,15 +23,15 @@ from reviewboard.reviews.models import FileAttachmentComment, Group, \ ReviewRequest, ReviewRequestDraft, \ Review, Comment, Screenshot, \ ScreenshotComment -from reviewboard.scmtools import sshutils from reviewboard.scmtools.errors import AuthenticationError, \ - BadHostKeyError, \ - UnknownHostKeyError, \ UnverifiedCertificateError from reviewboard.scmtools.models import Repository, Tool from reviewboard.scmtools.svn import SVNTool from reviewboard.site.urlresolvers import local_site_reverse from reviewboard.site.models import LocalSite +from reviewboard.ssh.client import SSHClient +from reviewboard.ssh.errors import BadHostKeyError, \ + UnknownHostKeyError from reviewboard.webapi.errors import BAD_HOST_KEY, \ DIFF_TOO_BIG, \ INVALID_REPOSITORY, \ @@ -594,16 +594,16 @@ class RepositoryResourceTests(BaseWebAPITestCase): # so we can restore them. self._old_check_repository = SVNTool.check_repository self._old_accept_certificate = SVNTool.accept_certificate - self._old_add_host_key = sshutils.add_host_key - self._old_replace_host_key = sshutils.replace_host_key + self._old_add_host_key = SSHClient.add_host_key + self._old_replace_host_key = SSHClient.replace_host_key def tearDown(self): super(RepositoryResourceTests, self).tearDown() SVNTool.check_repository = self._old_check_repository SVNTool.accept_certificate = self._old_accept_certificate - sshutils.add_host_key = self._old_add_host_key - sshutils.replace_host_key = self._old_replace_host_key + SSHClient.add_host_key = self._old_add_host_key + SSHClient.replace_host_key = self._old_replace_host_key def test_get_repositories(self): """Testing the GET repositories/ API""" @@ -664,7 +664,7 @@ class RepositoryResourceTests(BaseWebAPITestCase): expected_key = key2 saw = {'replace_host_key': False} - def _replace_host_key(_hostname, _expected_key, _key, local_site_name): + def _replace_host_key(cls, _hostname, _expected_key, _key): self.assertEqual(hostname, _hostname) self.assertEqual(expected_key, _expected_key) self.assertEqual(key, _key) @@ -676,7 +676,7 @@ class RepositoryResourceTests(BaseWebAPITestCase): raise BadHostKeyError(hostname, key, expected_key) SVNTool.check_repository = _check_repository - sshutils.replace_host_key = _replace_host_key + SSHClient.replace_host_key = _replace_host_key self._login_user(admin=True) self._post_repository(False, data={ @@ -711,7 +711,7 @@ class RepositoryResourceTests(BaseWebAPITestCase): key = key1 saw = {'add_host_key': False} - def _add_host_key(_hostname, _key, local_site_name): + def _add_host_key(cls, _hostname, _key): self.assertEqual(hostname, _hostname) self.assertEqual(key, _key) saw['add_host_key'] = True @@ -722,7 +722,7 @@ class RepositoryResourceTests(BaseWebAPITestCase): raise UnknownHostKeyError(hostname, key) SVNTool.check_repository = _check_repository - sshutils.add_host_key = _add_host_key + SSHClient.add_host_key = _add_host_key self._login_user(admin=True) self._post_repository(False, data={