[389-devel] Please review (take 4): [389 Project] #47945: Add SSL/TLS version info to the access log

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



#47945: Add SSL/TLS version info to the access log
-------------------------------------+-------------------------------
        Reporter:  nhosoi            |          Owner:  nhosoi
            Type:  defect            |         Status:  accepted
        Priority:  major             |      Milestone:  1.3.3 backlog
       Component:  Directory Server  |        Version:  1.3.0
      Resolution:                    |       Keywords:
      Blocked By:                    |       Blocking:
          Review:  review?           |  Ticket origin:  Community
Red Hat Bugzilla:                    |
-------------------------------------+-------------------------------

Comment (by rmeggins):

 one small coding problem
 {{{
         (void) slapi_getSSLVersion_str(enabledNSSVersions.max, emax,
 sizeof(emin));
 }}}
 should be sizeof(emax)

 Also, in restrict_SSLVersionRange() - I think you can just format all of
 the strings once at the top of the function.  e.g. something like this:
 {{{
     char mymin[VERSION_STR_LENGTH], mymax[VERSION_STR_LENGTH];
     char emin[VERSION_STR_LENGTH], emax[VERSION_STR_LENGTH];
     char recommendedmin[VERSION_STR_LENGTH];

     (void) slapi_getSSLVersion_str(slapdNSSVersions.min, mymin,
 sizeof(mymin));
     (void) slapi_getSSLVersion_str(slapdNSSVersions.max, mymax,
 sizeof(mymax));
     (void) slapi_getSSLVersion_str(enabledNSSVersions.min, emin,
 sizeof(emin));
     (void) slapi_getSSLVersion_str(enabledNSSVersions.max, emax,
 sizeof(emax));
     (void) slapi_getSSLVersion_str(SSL_LIBRARY_VERSION_TLS_1_1,
 recommendedmin, sizeof(recommendedmin));
 }}}
 It would save a lot of code below.

 Finally, we don't have to hardcode the "1" in "TLS1".  Take a look at the
 definitions:
 {{{
     #define SSL_LIBRARY_VERSION_TLS_1_0             0x0301
     #define SSL_LIBRARY_VERSION_TLS_1_1             0x0302
 }}}
 The major version ("1") is just (value >> 8) - 2.  So for "TLS" we could
 format the string version like this:
 {{{
 char *
 slapi_getSSLVersion_str(PRUint16 vnum, char *buf, size_t bufsize)
 {
 ...
     if (vnum >= SSL_LIBRARY_VERSION_3_0) { /* e.g. TLSv1.x, TLSv2.x, etc.
 */
         if (vnum & 0xff) { /* TLS */
             if (buf && bufsize) {
                 PR_snprintf(buf, bufsize, "TLS%d.%d", (vnum >> 8) - 2,
 (vnum & 0xff) - 1);
             } else {
                 vstr = slapi_ch_smprintf("TLS%d.%d", (vnum >> 8) - 2,
 (vnum & 0xff) - 1);
             }
 ...
 }}}
 That should give us a few years of future-proofing if TLS 2.x comes out


--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel

[Index of Archives]     [Fedora Directory Announce]     [Fedora Users]     [Older Fedora Users Mail]     [Fedora Advisory Board]     [Fedora Security]     [Fedora Devel Java]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Mentors]     [Fedora Package Review]     [Fedora Art]     [Fedora Music]     [Fedora Packaging]     [CentOS]     [Fedora SELinux]     [Big List of Linux Books]     [KDE Users]     [Fedora Art]     [Fedora Docs]

  Powered by Linux