Change semantics of --force-new-ta-certificate, make it reissue the certificate in all cases

Author: lolepezy

src/main/java/net/ripe/rpki/ta/TA.java

@@ -54,14 +54,13 @@
 
 import static net.ripe.rpki.commons.crypto.x509cert.X509CertificateInformationAccessDescriptor.*;
 
+@Getter
 @Slf4j(topic = "TA")
 public class TA {
 
     public static final IpResourceSet ALL_RESOURCES_SET = IpResourceSet.parse("AS0-AS4294967295, 0/0, 0::/0");
 
-    @Getter
     private TAState state;
-
     private final ValidityPeriods validityPeriods;
 
     public static TA initialise(Config config) throws GeneralSecurityException, IOException {
@@ -303,17 +302,22 @@ private Pair<TrustAnchorResponse, TAState> processRequest(final TrustAnchorReque
             revokeAllIssuedResourceCertificates(newTAState);
         }
 
+        // There are two cases when we need to re-issue the TA certificate:
+        // 1. We explicitly ask for it by providing the --force-new-ta-certificate option
+        // 2. The TA certificate publication point or the notification.xml URL has changed
+
         // re-issue TA certificate if some of the publication points are changed
-        final Optional<String> whyReissue = taCertificateHasToBeReIssued(request, signCtx.taState.getConfig());
-        if (whyReissue.isPresent()) {
-            if (!options.hasForceNewTaCertificate()) {
-                throw new OperationAbortedException("TA certificate has to be re-issued: " + whyReissue.get() +
-                    ", bailing out. Provide " + ProgramOptions.FORCE_NEW_TA_CERT_OPT + " option to force TA certificate re-issue.");
-            }
+        final Optional<String> differentLocations = locationsAreDifferent(request, signCtx.taState.getConfig());
 
-            // copy new SIAs to the TA config
-            updateTaConfigUrls(request, signCtx);
+        if (differentLocations.isPresent() && !options.hasForceNewTaCertificate()) {
+            throw new OperationAbortedException("TA certificate has to be re-issued: " + differentLocations.get() +
+                    ", bailing out. Provide " + ProgramOptions.FORCE_NEW_TA_CERT_OPT + " option to force TA certificate re-issue.");
+        }
 
+        if (options.hasForceNewTaCertificate() || differentLocations.isPresent()) {
+            if (differentLocations.isPresent()) {
+                updateTaConfigUrls(request, signCtx);
+            }
             final KeyPair keyPair = decoded.getLeft();
             final X509ResourceCertificate taCertificate = decoded.getRight();
             final BigInteger nextSerial = nextIssuedCertSerial(state);
@@ -322,7 +326,7 @@ private Pair<TrustAnchorResponse, TAState> processRequest(final TrustAnchorReque
                     signCtx.taState.getConfig()
             );
             final X509ResourceCertificate newTACertificate = reIssueRootCertificate(keyPair,
-                merge(ta0SiaDescriptors, request.getSiaDescriptors()), taCertificate, nextSerial);
+                    merge(ta0SiaDescriptors, request.getSiaDescriptors()), taCertificate, nextSerial);
 
             TAStateBuilder taStateBuilder = new TAStateBuilder(newTAState);
             taStateBuilder.withCrl(newTAState.getCrl());
@@ -344,7 +348,7 @@ private Pair<TrustAnchorResponse, TAState> processRequest(final TrustAnchorReque
         return Pair.of(new TrustAnchorResponse(request.getCreationTimestamp(), publishedObjects, taResponses), newTAState);
     }
 
-    private Optional<String> taCertificateHasToBeReIssued(TrustAnchorRequest taRequest, Config taConfig) {
+    private Optional<String> locationsAreDifferent(TrustAnchorRequest taRequest, Config taConfig) {
         if (!taConfig.getTaCertificatePublicationUri().equals(taRequest.getTaCertificatePublicationUri())) {
             return Optional.of("Different TA certificate location, request has '" +
                     taRequest.getTaCertificatePublicationUri() + "', config has '" + taConfig.getTaCertificatePublicationUri() + "'");

src/main/java/net/ripe/rpki/ta/config/ProgramOptions.java

@@ -48,7 +48,7 @@ public class ProgramOptions {
 
         options.addOption(Option.builder().longOpt(FORCE_NEW_TA_CERT_OPT).
             hasArg(false).
-            desc("Force re-issuing new TA certificate if there're SIA differences between config and request").
+            desc("Force re-issuing new TA certificate. This option is mandatory if there are SIA differences between request and config (or request and stored TA state)").
             build());
 
         options.addOption(Option.builder().longOpt(REVOKE_NON_REQUESTED_OBJECTS)

src/main/java/net/ripe/rpki/ta/util/ValidityPeriods.java

@@ -7,7 +7,7 @@
 
 public class ValidityPeriods {
 
-    private static final int TA_CERTIFICATE_VALIDITY_TIME_IN_YEARS = 100;
+    private static final int TA_CERTIFICATE_VALIDITY_TIME_IN_MONTHS = 6;
 
     // Since this program runs within a script, we can safely assume that all
     // calls to "now" can be replaced with a value calculated only once.
@@ -30,7 +30,7 @@ public ValidityPeriod allResourcesCertificate() {
 
     public static ValidityPeriod taCertificate() {
         final DateTime notValidBefore = ValidityPeriods.now();
-        return new ValidityPeriod(notValidBefore, notValidBefore.plusYears(TA_CERTIFICATE_VALIDITY_TIME_IN_YEARS));
+        return new ValidityPeriod(notValidBefore, notValidBefore.plusMonths(TA_CERTIFICATE_VALIDITY_TIME_IN_MONTHS));
     }
 
     public ValidityPeriod crl() {

src/test/java/net/ripe/rpki/ta/integration/MainIntegrationTest.java

@@ -101,7 +101,8 @@ public void test_process_request() throws Exception {
             run("--request=./src/test/resources/ta-request.xml --force-new-ta-certificate " +
                       "--response=" + response.getAbsolutePath() + " --env=test").exitCode);
         final TAState taState2 = reloadTaState();
-        assertEquals(BigInteger.valueOf(6L), taState2.getLastIssuedCertificateSerial());
+        // TA certificate will be reissued, so serial numbers will be incremented
+        assertEquals(BigInteger.valueOf(7L), taState2.getLastIssuedCertificateSerial());
         assertEquals(BigInteger.valueOf(2L), taState2.getLastMftSerial());
         assertEquals(BigInteger.valueOf(2L), taState2.getLastCrlSerial());
         assertEquals(2, taState2.getSignedProductionCertificates().size());
@@ -112,7 +113,8 @@ public void test_process_request() throws Exception {
             run("--request=./src/test/resources/ta-request.xml --force-new-ta-certificate " +
                       "--response=" + response.getAbsolutePath() + " --env=test").exitCode);
         final TAState taState3 = reloadTaState();
-        assertEquals(BigInteger.valueOf(8L), taState3.getLastIssuedCertificateSerial());
+        // TA certificate will be re-issued simply because of the -force-new-ta-certificate
+        assertEquals(BigInteger.valueOf(10L), taState3.getLastIssuedCertificateSerial());
         assertEquals(BigInteger.valueOf(3L), taState3.getLastMftSerial());
         assertEquals(BigInteger.valueOf(3L), taState3.getLastCrlSerial());