koji

Clone

#263 Error message for missing certificates
Merged 7 years ago by mikem. Opened 7 years ago by tkopecek.
tkopecek/koji issue250  into  master

file modified
+13 -2 
@@ -4991,9 +4991,9 @@ 
 

                                       '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 @@ 
 

      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):

file modified
+4 -2 
@@ -2172,8 +2172,10 @@ 
 

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

file modified
+14 -5 
@@ -62,12 +62,10 @@ 
 

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

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

      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:

file modified
+13 -3 
@@ -729,9 +729,9 @@ 
 

                  '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 @@ 
 

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

  

      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:

no initial comment

Ah, the cert defaults in kojid are wrong. We need to do the same thing there that we do in koji.read_config().

1 new commit added

  • backward-compatible default value for kojid certs
7 years ago

This is not exactly same behaviour as before. Now there is no way to set serverca/cert to None. It will be always replaced by default value and fallback to PKI will not happen. Is it something we want?
Edited 7 years ago by tkopecek

Question in code.

This is in the ssl_login method. This login method has always required a valid and explicit serverca setting.

Granted, we might be able to lower that restriction now and let the system fall back to pki

1 new commit added

  • Don't require cert/serverca for kojid
7 years ago

This looks fine, but the original issue also asked about kojira and koji-gc. Would you like to update those here or make a separate PR?

I've merged it here.

rebased
7 years ago

This allows cert=None to proceed, which cannot work

rebased
7 years ago

Reverted 'cert is None'.

Commit 1593e03 fixes this pull-request

Pull-Request has been merged by mikem@redhat.com
7 years ago
Metadata
None
No Tags
Changes Summary 4
+13 -2
file changed
builder/kojid
+4 -2
file changed
koji/__init__.py
+14 -5
file changed
util/koji-gc
+13 -3
file changed
util/kojira
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.