From a8053ca42fde4ff61cda4a9c4ee9aadf3d62a2bb Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jan 13 2017 09:06:05 +0000 Subject: [PATCH 1/3] Error message for missing certificates --- diff --git a/koji/__init__.py b/koji/__init__.py index 144c2c5..5f29018 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2174,6 +2174,10 @@ class ClientSession(object): raise AuthError('No certification provided') if serverca is None: raise AuthError('No server CA provided') + if not os.access(cert, os.R_OK): + raise AuthError("Certificate %s doesn't exist or is not accessible" % cert) + if not os.access(serverca, os.R_OK): + raise AuthError("Server CA %s doesn't exist or is not accessible" % serverca) # FIXME: ca is not useful here and therefore ignored, can be removed # when API is changed From 8f231cb9fdfa128430f9ebd0005976397af0012f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jan 13 2017 09:06:05 +0000 Subject: [PATCH 2/3] backward-compatible default value for kojid/kojira/koji-gc certs --- diff --git a/builder/kojid b/builder/kojid index 446ab2e..37a1709 100755 --- a/builder/kojid +++ b/builder/kojid @@ -4991,9 +4991,9 @@ def get_options(): 'resolver-status.properties *.lastUpdated', 'failed_buildroot_lifetime' : 3600 * 4, 'rpmbuild_timeout' : 3600 * 24, - 'cert': '/etc/kojid/client.crt', + 'cert': None, 'ca': '', # FIXME: Unused, remove in next major release - 'serverca': '/etc/kojid/serverca.crt'} + 'serverca': None} if config.has_section('kojid'): for name, value in config.items('kojid'): if name in ['sleeptime', 'maxjobs', 'minspace', 'retry_interval', @@ -5050,6 +5050,17 @@ def get_options(): if options.debug_mock: logger.warning("The debug-mock option is obsolete") + # special handling for cert defaults + cert_defaults = { + 'cert': '/etc/kojid/client.crt', + 'serverca': '/etc/kojid/serverca.crt', + } + for name in cert_defaults: + if getattr(options, name, None) is None: + fn = cert_defaults[name] + if os.path.exists(fn): + setattr(options, name, fn) + return options def quit(msg=None, code=1): diff --git a/util/koji-gc b/util/koji-gc index 2f0ae02..fd4aa26 100755 --- a/util/koji-gc +++ b/util/koji-gc @@ -62,12 +62,10 @@ def get_options(): help=_("do not authenticate")) parser.add_option("--network-hack", action="store_true", default=False, help=optparse.SUPPRESS_HELP) # no longer used - parser.add_option("--cert", default='/etc/koji-gc/client.crt', - help=_("Client SSL certificate file for authentication")) + parser.add_option("--cert", help=_("Client SSL certificate file for authentication")) parser.add_option("--ca", default='', help=_("ignored")) # FIXME: remove in next major release - parser.add_option("--serverca", default='/etc/koji-gc/serverca.crt', - help=_("CA cert file that issued the hub certificate")) + parser.add_option("--serverca", help=_("CA cert file that issued the hub certificate")) parser.add_option("-n", "--test", action="store_true", default=False, help=_("test mode")) parser.add_option("-d", "--debug", action="store_true", default=False, @@ -213,6 +211,17 @@ def get_options(): except ValueError: parser.error(_("Invalid time interval: %s") % value) + # special handling for cert defaults + cert_defaults = { + 'cert': '/etc/koji-gc/client.crt', + 'serverca': '/etc/koji-gc/serverca.crt', + } + for name in cert_defaults: + if getattr(options, name, None) is None: + fn = cert_defaults[name] + if os.path.exists(fn): + setattr(options, name, fn) + return options, args def check_tag(name): @@ -350,7 +359,7 @@ def activate_session(session): if options.noauth: #skip authentication pass - elif os.path.isfile(options.cert): + elif options.cert is not None and os.path.isfile(options.cert): # authenticate using SSL client cert session.ssl_login(options.cert, None, options.serverca, proxyuser=options.runas) elif options.user: diff --git a/util/kojira b/util/kojira index 905fec4..d7f1575 100755 --- a/util/kojira +++ b/util/kojira @@ -729,9 +729,9 @@ def get_options(): 'deleted_repo_lifetime': 7*24*3600, #XXX should really be called expired_repo_lifetime 'sleeptime' : 15, - 'cert': '/etc/kojira/client.crt', + 'cert': None, 'ca': '', # FIXME: unused, remove in next major release - 'serverca': '/etc/kojira/serverca.crt' + 'serverca': None, } if config.has_section(section): int_opts = ('deleted_repo_lifetime', 'max_repo_tasks', 'repo_tasks_limit', @@ -755,6 +755,16 @@ def get_options(): setattr(options, name, value) if options.logfile in ('','None','none'): options.logfile = None + # special handling for cert defaults + cert_defaults = { + 'cert': '/etc/kojira/client.crt', + 'serverca': '/etc/kojira/serverca.crt', + } + for name in cert_defaults: + if getattr(options, name, None) is None: + fn = cert_defaults[name] + if os.path.exists(fn): + setattr(options, name, fn) return options def quit(msg=None, code=1): @@ -797,7 +807,7 @@ if __name__ == "__main__": session_opts = koji.grab_session_options(options) session = koji.ClientSession(options.server,session_opts) - if os.path.isfile(options.cert): + if options.cert is not None and os.path.isfile(options.cert): # authenticate using SSL client certificates session.ssl_login(options.cert, None, options.serverca) elif options.user: From 8cafb7a42e4263fcb16b8c6ee6706c0d670fc4cd Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jan 13 2017 09:06:29 +0000 Subject: [PATCH 3/3] Don't require cert/serverca for kojid Now is demanded from user to supply kojid-specific configuration or previously expected files are used as defaults (/etc/kojid/serverca.crt). So, user has no option to not use these and use just system-wide certificates. If these values are not specified, python-requests would fall back to /etc/pki supplied files. If there is also ssl_verify=True (which is default), everything should work correctly. --- diff --git a/koji/__init__.py b/koji/__init__.py index 5f29018..ed57965 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -2172,11 +2172,9 @@ class ClientSession(object): serverca = serverca or self.opts.get('serverca') if cert is None: raise AuthError('No certification provided') - if serverca is None: - raise AuthError('No server CA provided') if not os.access(cert, os.R_OK): raise AuthError("Certificate %s doesn't exist or is not accessible" % cert) - if not os.access(serverca, os.R_OK): + if serverca is not None and not os.access(serverca, os.R_OK): raise AuthError("Server CA %s doesn't exist or is not accessible" % serverca) # FIXME: ca is not useful here and therefore ignored, can be removed # when API is changed