Uploaded image for project: 'EJBCA'
  1. EJBCA
  2. ECA-7107

Refactor CRL decision logic and write test cases

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      The code for determining whether the a CRL should be generated or not can be found in PublishingCrlSessionBean.java. More specifically in the two methods:

      createCRLNewTransactionConditioned
      createDeltaCRLnewTransactionConditioned

      The scope of this ticket is:

      • Move the CRL decision logic to a separate class, e.g. a new class called CrlGenerator to simplify testing
      • Write unit tests for CrlGenerator
      • Preferably use guard clauses instead of 1 billion nested ifs and simplify the logic if possible
      • Add proper Javadoc to new methods in CrlGenerator which corresponds to what is in the EJBCA documentation
      • Add minimal boilerplate code to PublishingCrlSessionBean, which instantiates a new CrlGenerator and starts it with proper parameters.

      Example code:

          /**
           * Determine if a new CRL should be created for the specified CA, based on the status and
           * settings of the CA, the periodic interval of the CRL Updater service and the current time.
           * <p>
           * CRLs should only be created for an active X.509 CA if any of the following conditions hold:
           * <ul>
           *   <li><b>If CRL Overlap > 0</b>
           *     <br/> - Current Time + Periodic Interval ≥ CRL Next Update - CRL Overlap</li>
           *   <li><b>If CRL Issue Interval = 0</b>
           *     <br/> - Current Time + Periodic Interval ≥ CRL Next Update
           *   </li>
           *   <li><b>If CRL Issue Interval > 0</b>
           *     <br/> - Current Time + Periodic Interval ≥ CRL Creation Date + CRL Issue Interval
           *   </li>
           * </ul>
           * @param caInfo information about the CA to check
           * @param periodicInterval the periodic interval of the CRL Updater Service given in milliseconds
           * @return true if a new CRL should be created for specified CA, false otherwise
           */
          @SuppressWarnings("unused")
          private boolean shouldCreateCrl(final CAInfo caInfo, final long periodicInterval) {
              if (caInfo.getCAType() != CAInfo.CATYPE_X509) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should not create CRL for CA " + caInfo.getName() + " of unsupported type " + caInfo.getCAType());
                  }
                  return false;
              }
              if (caInfo.getStatus() != CAConstants.CA_ACTIVE) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should not create CRL for non-active CA " + caInfo.getName() + " with status " + caInfo.getStatus());
                  }
                  return false;
              }
              final Certificate caCertificate = getCaCertificate(caInfo);
              if (caCertificate == null) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should not create CRL for CA " + caInfo.getName() + " with missing CA certificate.");
                  }
                  return false;
              }
              final Date now = new Date();
              if (log.isDebugEnabled()) {
                  log.debug("Running CRL check for " + caInfo.getName() + " with the following parameters: " + System.lineSeparator()
                          + "  CRL Issue Interval: " + caInfo.getCRLIssueInterval() + System.lineSeparator() 
                          + "  CRL Overlap Time: " + caInfo.getCRLOverlapTime() + System.lineSeparator() 
                          + "  Periodic Interval: " + periodicInterval + System.lineSeparator()
                          + "  Current Time: " + now.getTime());
              }
              final CRLInfo lastCrlInfo = crlSession.getLastCRLInfo(CertTools.getSubjectDN(caCertificate), /* Delta CRL? */ false);
              if (log.isDebugEnabled()) {
                  if (lastCrlInfo == null) {
                      log.debug("No CRL information for " + caInfo.getName() + " found.");
                  } else {
                      log.debug("Read CRL information for " + caInfo.getName()
                              + ". Below follows information about the most recently issued CRL from this CA." + System.lineSeparator() 
                              + "CRL Number: " + lastCrlInfo.getLastCRLNumber() + System.lineSeparator() 
                              + "Expiration Date: " + lastCrlInfo.getExpireDate());
                  }
              }
              
              // TODO Handle the case where lastCrlInfo == null
              if (caInfo.getCRLOverlapTime() > 0
                      && now.getTime() + periodicInterval >= lastCrlInfo.getExpireDate().getTime() - caInfo.getCRLOverlapTime()) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should create CRL for " + caInfo.getName()
                              + " because CRL Overlap is enabled and Current Time + Periodic Interval ≥ CRL Next Update - CRL Overlap.");
                  }
                  return true;
              }
              if (caInfo.getCRLIssueInterval() == 0 && now.getTime() + periodicInterval >= lastCrlInfo.getExpireDate().getTime()) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should create CRL for " + caInfo.getName() + " because Current Time + Periodic Interval ≥ CRL Next Update.");
                  }
                  return true;
              }
              if (caInfo.getCRLIssueInterval() > 0
                      && now.getTime() + periodicInterval >= lastCrlInfo.getCreateDate().getTime() + caInfo.getCRLIssueInterval()) {
                  if (log.isDebugEnabled()) {
                      log.debug("Should create CRL for " + caInfo.getName()
                              + " because CRL Issue Interval is enabled and Current Time + Periodic Interval ≥ CRL Creation Date + CRL Issue Interval");
                  }
                  return true;
              }
              if (log.isDebugEnabled()) {
                  log.debug("Should not create CRL for " + caInfo.getName() + " yet.");
              }
              return false;
          }
      

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                bastianf Bastian Fredriksson
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Time Tracking

                  Estimated:
                  Original Estimate - 5 hours
                  5h
                  Remaining:
                  Remaining Estimate - 5 hours
                  5h
                  Logged:
                  Time Spent - Not Specified
                  Not Specified