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: Closed
    • 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.

        Attachments

          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.
          Hide
          linus Linus Nordberg added a comment -
          radsecproxy-1.6.2
          Show
          linus Linus Nordberg added a comment - radsecproxy-1.6.2

            People

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

              Dates

              • Created:
                Updated:
                Resolved: