Uploaded image for project: 'radsecproxy'
  1. radsecproxy
  2. RADSECPROXY-43

Radsecproxy is mixing up pre- and post-TLS-handshake client verification

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: radsecproxy-1.6.2
    • Component/s: code
    • Labels:
      None

      Description

      Config:

          client wontmatch {
              host 0.0.0.0
              type tls
              tls demoCA
              certificatenamecheck off
              matchCertificateAttribute CN:/^nonexistent$/
          }
          client demo {
              host 0.0.0.0
              type tls
              tls someotherCA
              certificatenamecheck off
              matchCertificateAttribute CN:/^cli.*$/
          }

      Given the above configuration, a client presenting a demoCA cert with
      CN=cli1 will be accepted. This is incorrect.

      The reason is that the CA specified in the first matching client block
      is the one being used for cert chain verification and once that's
      done, verification based on certificate _contents_ is being performed
      without regarding any other CA constraints in other client blocks.

      The propsed fix for this is to stop considering client blocks with a
      CA config (the 'tls' option) not identical to the first matching
      client block.

      This may make Radsecproxy reject clients that are currently
      (erroneously) being accepted. The proposed solution for scenarios
      which can no longer be realised by a single Radsecproxy instance is,
      for now, to run multiple instances on separate addresses or ports.

        Activity

        Hide
        linus Linus Nordberg added a comment -
        This bug was found by Ralf Paffrath.

        A fix has been committed (db965c9) and pushed to master.

        The fix was suggested by Ralf. Thanks!
        Show
        linus Linus Nordberg added a comment - This bug was found by Ralf Paffrath. A fix has been committed (db965c9) and pushed to master. The fix was suggested by Ralf. Thanks!
        Hide
        linus Linus Nordberg added a comment -
        For a better understanding of how radsecproxy does client
        verification, here's a description.

        tlsservernew() starts by looking for a matching client block based on
        client source IP address. First matching client block is used for
        creating a TLS context, creating an SSL object, setting the fd and
        SSL_accept. Creating a TLS context also sets up CAfile and CApath used
        for anchoring the certificate chain as well as the list of CA's to
        present to the client. Note that the CA store information for both of
        these is taken from the configuration of the matching client block.

        (Unrelated to this issue, tlsaddcacrl() also sets 'tlsexpiry' in
        struct ssl, enables CRL verification and sets 'vpm'
        (X509_STORE_set1_param()).)

        If verifytlscert() succeeds, we continue with the cert returned.

        In the example above, the cert recieved is signed by a demoCA
        cert. It's passed to verifyconfcert() for verification which fails due
        to the matchCertificateAttribute configuration.

        A new client block is found ("demo") and a new call to
        verifyconfcert() is done and this time it succeeds. This is wrong. The
        problem is that verifyconfcert() doesn't check the signature chain --
        that's already been done by verifytlscert().
        Show
        linus Linus Nordberg added a comment - For a better understanding of how radsecproxy does client verification, here's a description. tlsservernew() starts by looking for a matching client block based on client source IP address. First matching client block is used for creating a TLS context, creating an SSL object, setting the fd and SSL_accept. Creating a TLS context also sets up CAfile and CApath used for anchoring the certificate chain as well as the list of CA's to present to the client. Note that the CA store information for both of these is taken from the configuration of the matching client block. (Unrelated to this issue, tlsaddcacrl() also sets 'tlsexpiry' in struct ssl, enables CRL verification and sets 'vpm' (X509_STORE_set1_param()).) If verifytlscert() succeeds, we continue with the cert returned. In the example above, the cert recieved is signed by a demoCA cert. It's passed to verifyconfcert() for verification which fails due to the matchCertificateAttribute configuration. A new client block is found ("demo") and a new call to verifyconfcert() is done and this time it succeeds. This is wrong. The problem is that verifyconfcert() doesn't check the signature chain -- that's already been done by verifytlscert().
        Hide
        linus Linus Nordberg added a comment -
        This will have to do for 1.6.1.
        Remaining issues are addressed in RADSECPROXY-44.
        Show
        linus Linus Nordberg added a comment - This will have to do for 1.6.1. Remaining issues are addressed in RADSECPROXY-44 .
        Hide
        linus Linus Nordberg added a comment -
        Raphael Geisser reported that DTLS suffers from the same problem and that it isn't fixed.
        Show
        linus Linus Nordberg added a comment - Raphael Geisser reported that DTLS suffers from the same problem and that it isn't fixed.
        Hide
        linus Linus Nordberg added a comment -
        This bug has been given CVE id CVE-2012-4523.
        ChangeLog and web page have been updated.
        Show
        linus Linus Nordberg added a comment - This bug has been given CVE id CVE-2012-4523. ChangeLog and web page have been updated.
        Hide
        linus Linus Nordberg added a comment -
        Fixed in 1.6.2.
        Show
        linus Linus Nordberg added a comment - Fixed in 1.6.2.
        Hide
        linus Linus Nordberg added a comment -
        The DTLS bug was given CVE id CVE-2012-4566.
        ChangeLog has been updated accordingly.
        Show
        linus Linus Nordberg added a comment - The DTLS bug was given CVE id CVE-2012-4566. ChangeLog has been updated accordingly.

          People

          • Assignee:
            linus Linus Nordberg
            Reporter:
            linus Linus Nordberg
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: