389-ds-base

Clone
Source Code
GIT

#51216 Issue 51129 - SSL alert: The value of sslVersionMax "TLS1.3" is higher than the supported version
Closed 3 years ago by spichugi. Opened 3 years ago by mreynolds.
mreynolds/389-ds-base issue51129  into  master

file modified
+80 -75 
@@ -50,11 +50,11 @@ 
 

   ******************************************************************************/
 

  

  #define DEFVERSION "TLS1.2"
 

- #define CURRENT_DEFAULT_SSL_VERSION SSL_LIBRARY_VERSION_TLS_1_2
 

  

  extern char *slapd_SSL3ciphers;
 

  extern symbol_t supported_ciphers[];
 

- static SSLVersionRange enabledNSSVersions;
 

+ static SSLVersionRange defaultNSSVersions;
 

+ static SSLVersionRange supportedNSSVersions;
 

  static SSLVersionRange slapdNSSVersions;
 

  

  
@@ -1014,15 +1014,24 @@ 
 

      int create_certdb = 0;
 

      PRUint32 nssFlags = 0;
 

      char *certdir;
 

-     char emin[VERSION_STR_LENGTH], emax[VERSION_STR_LENGTH];
 

-     /* Get the range of the supported SSL version */
 

-     SSL_VersionRangeGetDefault(ssl_variant_stream, &enabledNSSVersions);
 

+     char dmin[VERSION_STR_LENGTH], dmax[VERSION_STR_LENGTH];
 

+     char smin[VERSION_STR_LENGTH], smax[VERSION_STR_LENGTH];
 

  

-     (void)slapi_getSSLVersion_str(enabledNSSVersions.min, emin, sizeof(emin));
 

-     (void)slapi_getSSLVersion_str(enabledNSSVersions.max, emax, sizeof(emax));
 

+     /* Get the range of the supported SSL version */
 

+     SSL_VersionRangeGetSupported(ssl_variant_stream, &supportedNSSVersions);
 

+     (void)slapi_getSSLVersion_str(supportedNSSVersions.min, smin, sizeof(smin));
 

+     (void)slapi_getSSLVersion_str(supportedNSSVersions.max, smax, sizeof(smax));
 

+ 

+     /* Get the enabled default range */
 

+     SSL_VersionRangeGetDefault(ssl_variant_stream, &defaultNSSVersions);
 

+     (void)slapi_getSSLVersion_str(defaultNSSVersions.min, dmin, sizeof(dmin));
 

+     (void)slapi_getSSLVersion_str(defaultNSSVersions.max, dmax, sizeof(dmax));
 

      slapi_log_err(SLAPI_LOG_CONFIG, "Security Initialization",
 

                    "slapd_nss_init - Supported range by NSS: min: %s, max: %s\n",
 

-                   emin, emax);
 

+                   smin, smax);
 

+     slapi_log_err(SLAPI_LOG_CONFIG, "Security Initialization",
 

+                   "slapd_nss_init - Enabled default range by NSS: min: %s, max: %s\n",
 

+                   dmin, dmax);
 

  

      /* set in slapd_bootstrap_config,
 

         thus certdir is available even if config_available is false
 
@@ -1344,21 +1353,21 @@ 
 

  set_NSS_version(char *val, PRUint16 *rval, int ismin)
 

  {
 

      char *vp;
 

-     char emin[VERSION_STR_LENGTH], emax[VERSION_STR_LENGTH];
 

+     char dmin[VERSION_STR_LENGTH], dmax[VERSION_STR_LENGTH];
 

  

      if (NULL == rval) {
 

          return 1;
 

      }
 

-     (void)slapi_getSSLVersion_str(enabledNSSVersions.min, emin, sizeof(emin));
 

-     (void)slapi_getSSLVersion_str(enabledNSSVersions.max, emax, sizeof(emax));
 

+     (void)slapi_getSSLVersion_str(defaultNSSVersions.min, dmin, sizeof(dmin));
 

+     (void)slapi_getSSLVersion_str(defaultNSSVersions.max, dmax, sizeof(dmax));
 

  

      if (!strncasecmp(val, SSLSTR, SSLLEN)) { /* ssl# NOT SUPPORTED */
 

          if (ismin) {
 

-             slapd_SSL_warn("SSL3 is no longer supported.  Using NSS default min value: %s\n", emin);
 

-             (*rval) = enabledNSSVersions.min;
 

+             slapd_SSL_warn("SSL3 is no longer supported.  Using NSS default min value: %s", dmin);
 

+             (*rval) = defaultNSSVersions.min;
 

          } else {
 

-             slapd_SSL_warn("SSL3 is no longer supported.  Using NSS default max value: %s\n", emax);
 

-             (*rval) = enabledNSSVersions.max;
 

+             slapd_SSL_warn("SSL3 is no longer supported.  Using NSS default max value: %s", dmax);
 

+             (*rval) = defaultNSSVersions.max;
 

          }
 

      } else if (!strncasecmp(val, TLSSTR, TLSLEN)) { /* tls# */
 

          float tlsv;
 
@@ -1366,122 +1375,122 @@ 
 

          sscanf(vp, "%4f", &tlsv);
 

          if (tlsv < 1.1f) { /* TLS1.0 */
 

              if (ismin) {
 

-                 if (enabledNSSVersions.min > CURRENT_DEFAULT_SSL_VERSION) {
 

+                 if (supportedNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_0) {
 

                      slapd_SSL_warn("The value of sslVersionMin "
 

                                     "\"%s\" is lower than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emin);
 

-                     (*rval) = enabledNSSVersions.min;
 

+                                    val, dmin);
 

+                     (*rval) = defaultNSSVersions.min;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
 

                  }
 

              } else {
 

-                 if (enabledNSSVersions.max < CURRENT_DEFAULT_SSL_VERSION) {
 

+                 if (supportedNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_0) {
 

                      /* never happens */
 

                      slapd_SSL_warn("The value of sslVersionMax "
 

                                     "\"%s\" is higher than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emax);
 

-                     (*rval) = enabledNSSVersions.max;
 

+                                    val, dmax);
 

+                     (*rval) = defaultNSSVersions.max;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_0;
 

                  }
 

              }
 

          } else if (tlsv < 1.2f) { /* TLS1.1 */
 

              if (ismin) {
 

-                 if (enabledNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_1) {
 

+                 if (supportedNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_1) {
 

                      slapd_SSL_warn("The value of sslVersionMin "
 

                                     "\"%s\" is lower than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emin);
 

-                     (*rval) = enabledNSSVersions.min;
 

+                                    val, dmin);
 

+                     (*rval) = defaultNSSVersions.min;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_1;
 

                  }
 

              } else {
 

-                 if (enabledNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_1) {
 

+                 if (supportedNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_1) {
 

                      /* never happens */
 

                      slapd_SSL_warn("The value of sslVersionMax "
 

                                     "\"%s\" is higher than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emax);
 

-                     (*rval) = enabledNSSVersions.max;
 

+                                    val, dmax);
 

+                     (*rval) = defaultNSSVersions.max;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_1;
 

                  }
 

              }
 

          } else if (tlsv < 1.3f) { /* TLS1.2 */
 

              if (ismin) {
 

-                 if (enabledNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_2) {
 

+                 if (supportedNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_2) {
 

                      slapd_SSL_warn("The value of sslVersionMin "
 

                                     "\"%s\" is lower than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emin);
 

-                     (*rval) = enabledNSSVersions.min;
 

+                                    val, dmin);
 

+                     (*rval) = defaultNSSVersions.min;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_2;
 

                  }
 

              } else {
 

-                 if (enabledNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_2) {
 

+                 if (supportedNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_2) {
 

                      /* never happens */
 

                      slapd_SSL_warn("The value of sslVersionMax "
 

                                     "\"%s\" is higher than the supported version; "
 

                                     "the default value \"%s\" is used.",
 

-                                    val, emax);
 

-                     (*rval) = enabledNSSVersions.max;
 

+                                    val, dmax);
 

+                     (*rval) = defaultNSSVersions.max;
 

                  } else {
 

                      (*rval) = SSL_LIBRARY_VERSION_TLS_1_2;
 

                  }
 

              }
 

          } else if (tlsv < 1.4f) { /* TLS1.3 */
 

-                     if (ismin) {
 

-                         if (enabledNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_3) {
 

-                             slapd_SSL_warn("The value of sslVersionMin "
 

-                                            "\"%s\" is lower than the supported version; "
 

-                                            "the default value \"%s\" is used.",
 

-                                            val, emin);
 

-                             (*rval) = enabledNSSVersions.min;
 

-                         } else {
 

-                             (*rval) = SSL_LIBRARY_VERSION_TLS_1_3;
 

-                         }
 

-                     } else {
 

-                         if (enabledNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_3) {
 

-                             /* never happens */
 

-                             slapd_SSL_warn("The value of sslVersionMax "
 

-                                            "\"%s\" is higher than the supported version; "
 

-                                            "the default value \"%s\" is used.",
 

-                                            val, emax);
 

-                             (*rval) = enabledNSSVersions.max;
 

-                         } else {
 

-                             (*rval) = SSL_LIBRARY_VERSION_TLS_1_3;
 

-                         }
 

-                     }
 

+             if (ismin) {
 

+                 if (supportedNSSVersions.min > SSL_LIBRARY_VERSION_TLS_1_3) {
 

+                     slapd_SSL_warn("The value of sslVersionMin "
 

+                                    "\"%s\" is lower than the supported version; "
 

+                                    "the default value \"%s\" is used.",
 

+                                    val, dmin);
 

+                     (*rval) = defaultNSSVersions.min;
 

+                 } else {
 

+                     (*rval) = SSL_LIBRARY_VERSION_TLS_1_3;
 

+                 }
 

+             } else {
 

+                 if (supportedNSSVersions.max < SSL_LIBRARY_VERSION_TLS_1_3) {
 

+                     /* never happens */
 

+                     slapd_SSL_warn("The value of sslVersionMax "
 

+                                    "\"%s\" is higher than the supported version; "
 

+                                    "the default value \"%s\" is used.",
 

+                                    val, dmax);
 

+                     (*rval) = defaultNSSVersions.max;
 

+                 } else {
 

+                     (*rval) = SSL_LIBRARY_VERSION_TLS_1_3;
 

+                 }
 

+             }
 

          } else { /* Specified TLS is newer than supported */
 

              if (ismin) {
 

                  slapd_SSL_warn("The value of sslVersionMin "
 

                                 "\"%s\" is out of the range of the supported version; "
 

                                 "the default value \"%s\" is used.",
 

-                                val, emin);
 

-                 (*rval) = enabledNSSVersions.min;
 

+                                val, dmin);
 

+                 (*rval) = defaultNSSVersions.min;
 

              } else {
 

                  slapd_SSL_warn("The value of sslVersionMax "
 

                                 "\"%s\" is out of the range of the supported version; "
 

                                 "the default value \"%s\" is used.",
 

-                                val, emax);
 

-                 (*rval) = enabledNSSVersions.max;
 

+                                val, dmax);
 

+                 (*rval) = defaultNSSVersions.max;
 

              }
 

          }
 

      } else {
 

          if (ismin) {
 

              slapd_SSL_warn("The value of sslVersionMin "
 

                             "\"%s\" is invalid; the default value \"%s\" is used.",
 

-                            val, emin);
 

-             (*rval) = enabledNSSVersions.min;
 

+                            val, dmin);
 

+             (*rval) = defaultNSSVersions.min;
 

          } else {
 

              slapd_SSL_warn("The value of sslVersionMax "
 

                             "\"%s\" is invalid; the default value \"%s\" is used.",
 

-                            val, emax);
 

-             (*rval) = enabledNSSVersions.max;
 

+                            val, dmax);
 

+             (*rval) = defaultNSSVersions.max;
 

          }
 

      }
 

      return 0;
 
@@ -1511,10 +1520,9 @@ 
 

      char *tmpDir;
 

      Slapi_Entry *e = NULL;
 

      PRBool fipsMode = PR_FALSE;
 

-     PRUint16 NSSVersionMin = enabledNSSVersions.min;
 

-     PRUint16 NSSVersionMax = enabledNSSVersions.max;
 

+     PRUint16 NSSVersionMin = defaultNSSVersions.min;
 

+     PRUint16 NSSVersionMax = defaultNSSVersions.max;
 

      char mymin[VERSION_STR_LENGTH], mymax[VERSION_STR_LENGTH];
 

-     char newmax[VERSION_STR_LENGTH];
 

      int allowweakcipher = CIPHER_SET_DEFAULTWEAKCIPHER;
 

      int_fast16_t renegotiation = (int_fast16_t)SSL_RENEGOTIATE_REQUIRES_XTN;
 

  
@@ -1875,12 +1883,9 @@ 
 

          if (NSSVersionMin > NSSVersionMax) {
 

              (void)slapi_getSSLVersion_str(NSSVersionMin, mymin, sizeof(mymin));
 

              (void)slapi_getSSLVersion_str(NSSVersionMax, mymax, sizeof(mymax));
 

-             slapd_SSL_warn("The min value of NSS version range \"%s\" is greater than the max value \"%s\".",
 

+             slapd_SSL_warn("The min value of NSS version range \"%s\" is greater than the max value \"%s\".  Adjusting the max to match the miniumum.",
 

                             mymin, mymax);
 

-             (void)slapi_getSSLVersion_str(enabledNSSVersions.max, newmax, sizeof(newmax));
 

-             slapd_SSL_warn("Reset the max \"%s\" to supported max \"%s\".",
 

-                            mymax, newmax);
 

-             NSSVersionMax = enabledNSSVersions.max;
 

+             NSSVersionMax = NSSVersionMin;
 

          }
 

      }
 

  
@@ -1896,7 +1901,7 @@ 
 

      if (sslStatus != SECSuccess) {
 

          errorCode = PR_GetError();
 

          slapd_SSL_error("Security Initialization - "
 

-                 "slapd_ssl_init2 - Failed to set SSL range: min: %s, max: %s - error %d (%s)\n",
 

+                 "slapd_ssl_init2 - Failed to set SSL range: min: %s, max: %s - error %d (%s)",
 

                  mymin, mymax, errorCode, slapd_pr_strerror(errorCode));
 

      }
 

      /*
 
@@ -1926,13 +1931,13 @@ 
 

              (void)slapi_getSSLVersion_str(slapdNSSVersions.min, mymin, sizeof(mymin));
 

              (void)slapi_getSSLVersion_str(slapdNSSVersions.max, mymax, sizeof(mymax));
 

              slapd_SSL_error("Security Initialization - "
 

-                     "slapd_ssl_init2 - Failed to set SSL range: min: %s, max: %s - error %d (%s)\n",
 

+                     "slapd_ssl_init2 - Failed to set SSL range: min: %s, max: %s - error %d (%s)",
 

                      mymin, mymax, errorCode, slapd_pr_strerror(errorCode));
 

          }
 

      } else {
 

          errorCode = PR_GetError();
 

          slapd_SSL_error("Security Initialization - ",
 

-                 "slapd_ssl_init2 - Failed to get SSL range from socket - error %d (%s)\n",
 

+                 "slapd_ssl_init2 - Failed to get SSL range from socket - error %d (%s)",
 

                  errorCode, slapd_pr_strerror(errorCode));
 

      }
 

  
@@ -2265,7 +2270,7 @@ 
 

          }
 

      } else {
 

          if (token == NULL) {
 

-             slapd_SSL_warn("slapd_SSL_client_auth - certificate token was not found\n");
 

+             slapd_SSL_warn("slapd_SSL_client_auth - certificate token was not found");
 

          }
 

          rc = -1;
 

      }

file modified
+1 -1 
@@ -207,7 +207,7 @@ 
 

          return {
 

              'base': quoted_vals[0],
 

              'filter': quoted_vals[1],
 

-             'timestamp': re.findall('\[(.*)\]', lines[0])[0],
 

+             'timestamp': re.findall('[(.*)]', lines[0])[0],
 

              'scope': lines[0].split(' scope=', 1)[1].split(' ',1)[0]
 

          }

Bug Description:

If you try and set the sslVersionMax higher than the default range, but within the supported range, you would still get an error and the server would reset the max to "default" max value.

Fix Description:

Keep track of both the supported and default SSL ranges, and correctly use each range for value validation. If the value is outside the supported range, then use default value, etc, but do not check the requested range against the default range. We only use the default range if there is no specified min or max in the config, or if a invalid min or max value is set in the config.

Also, refactored the range variable names to be more accurate:

  • enabledNSSVersions --> defaultNSSVersions
  • emin, emax --> dmin, dmax

Also fixed a warning in lib389 about deprecated regex expression.

relates: https://pagure.io/389-ds-base/issue/51129

Famous last words.

Just confirming that in this case, we default to the supportedNSSVersion.min instead of our DEFAULT_SSL_VERSION?

Otherwise reading it a few times, I have no issues with this :)

rebased onto 41bf43f2225adbc70730ce3d387711567de8bcc5
3 years ago

Just confirming that in this case, we default to the supportedNSSVersion.min instead of our DEFAULT_SSL_VERSION?

Ahh, thanks for bringing this up. This code is somewhat invalid when dealing with TLS1.0. We no longer need CURRENT_DEFAULT_SSL_VERSION, but we are still using DEFVERSION to force a default minimum of TLS1.2 if the user did not specify a TLS minimum.

Please review one last time...

Ahhh yep. Okay, no problems from me :) ack.

rebased onto 78e3fb4a748465498786e27fcd02b7231acc4241
3 years ago

rebased onto 2c8e339
3 years ago

Pull-Request has been merged by mreynolds
3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4269

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 2
+80 -75
file changed
ldap/servers/slapd/ssl.c
+1 -1
file changed
src/lib389/lib389/dirsrv_log.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.