Address pki::TestAutoRebuild flakiness (#18903)

* Address pki::TestAutoRebuild flakiness

 - Wait for a CRL change before progressing to the next step after
   we change configuration. Prior to this we would be racing against
   the CRL reloading from the configuration change.

builtin/logical/pki/crl_test.go

@@ -980,7 +980,7 @@ func TestAutoRebuild(t *testing.T) {
 	require.NoError(t, err)
 
 	crl := getCrlCertificateList(t, client, "pki")
-	lastCRLNumber := crl.Version
+	lastCRLNumber := getCRLNumber(t, crl)
 	lastCRLExpiry := crl.NextUpdate
 	requireSerialNumberInCRL(t, crl, leafSerial)
 
@@ -994,6 +994,12 @@ func TestAutoRebuild(t *testing.T) {
 	})
 	require.NoError(t, err)
 
+	// Wait for the CRL to update based on the configuration change we just did
+	// so that it doesn't grab the revocation we are going to do afterwards.
+	crl = waitForUpdatedCrl(t, client, "pki", lastCRLNumber, lastCRLExpiry.Sub(time.Now()))
+	lastCRLNumber = getCRLNumber(t, crl)
+	lastCRLExpiry = crl.NextUpdate
+
 	// Issue a cert and revoke it.
 	resp, err = client.Logical().Write("pki/issue/local-testing", map[string]interface{}{
 		"common_name": "example.com",
@@ -1041,7 +1047,7 @@ func TestAutoRebuild(t *testing.T) {
 
 	// New serial should not appear on CRL.
 	crl = getCrlCertificateList(t, client, "pki")
-	thisCRLNumber := crl.Version
+	thisCRLNumber := getCRLNumber(t, crl)
 	requireSerialNumberInCRL(t, crl, leafSerial) // But the old one should.
 	now := time.Now()
 	graceInterval, _ := time.ParseDuration(gracePeriod)
@@ -1128,6 +1134,11 @@ func TestAutoRebuild(t *testing.T) {
 				// Check if it is on the main CRL because its already regenerated.
 				mainCRL := getParsedCrlAtPath(t, client, "/v1/pki/crl").TBSCertList
 				requireSerialNumberInCRL(t, mainCRL, newLeafSerial)
+			} else {
+				referenceCrlNum := getCrlReferenceFromDelta(t, deltaCrl)
+				if lastCRLNumber < referenceCrlNum {
+					lastCRLNumber = referenceCrlNum
+				}
 			}
 		}
 	}
@@ -1138,32 +1149,9 @@ func TestAutoRebuild(t *testing.T) {
 		time.Sleep(expectedUpdate.Sub(now))
 	}
 
-	// Otherwise, the absolute latest we're willing to wait is some delta
-	// after CRL expiry (to let stuff regenerate &c).
-	interruptChan = time.After(lastCRLExpiry.Sub(now) + delta)
-	for {
-		select {
-		case <-interruptChan:
-			t.Fatalf("expected CRL to regenerate prior to CRL expiry (plus %v grace period)", delta)
-		default:
-			crl = getCrlCertificateList(t, client, "pki")
-			if crl.NextUpdate.Equal(lastCRLExpiry) {
-				// Hack to ensure we got a net-new CRL. If we didn't, we can
-				// exit this default conditional and wait for the next
-				// go-round. When the timer fires, it'll populate the channel
-				// and we'll exit correctly.
-				time.Sleep(1 * time.Second)
-				break
-			}
-
-			now := time.Now()
-			require.True(t, crl.ThisUpdate.Before(now))
-			require.True(t, crl.NextUpdate.After(now))
-			requireSerialNumberInCRL(t, crl, leafSerial)
-			requireSerialNumberInCRL(t, crl, newLeafSerial)
-			return
-		}
-	}
+	crl = waitForUpdatedCrl(t, client, "pki", lastCRLNumber, lastCRLExpiry.Sub(now)+delta)
+	requireSerialNumberInCRL(t, crl, leafSerial)
+	requireSerialNumberInCRL(t, crl, newLeafSerial)
 }
 
 func TestTidyIssuerAssociation(t *testing.T) {

builtin/logical/pki/test_helpers.go

@@ -7,11 +7,15 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"crypto/x509/pkix"
+	"encoding/asn1"
 	"encoding/pem"
 	"fmt"
 	"io"
+	"math"
+	"math/big"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/hashicorp/vault/api"
 	"github.com/hashicorp/vault/sdk/helper/certutil"
@@ -262,3 +266,61 @@ func requireSuccessNilResponse(t *testing.T, resp *logical.Response, err error,
 		require.Nilf(t, resp, msg, msgAndArgs...)
 	}
 }
+
+func getCRLNumber(t *testing.T, crl pkix.TBSCertificateList) int {
+	t.Helper()
+
+	for _, extension := range crl.Extensions {
+		if extension.Id.Equal(certutil.CRLNumberOID) {
+			bigInt := new(big.Int)
+			leftOver, err := asn1.Unmarshal(extension.Value, &bigInt)
+			require.NoError(t, err, "Failed unmarshalling crl number extension")
+			require.Empty(t, leftOver, "leftover bytes from unmarshalling crl number extension")
+			require.True(t, bigInt.IsInt64(), "parsed crl number integer is not an int64")
+			require.False(t, math.MaxInt <= bigInt.Int64(), "parsed crl number integer can not fit in an int")
+			return int(bigInt.Int64())
+		}
+	}
+
+	t.Fatalf("failed to find crl number extension")
+	return 0
+}
+
+func getCrlReferenceFromDelta(t *testing.T, crl pkix.TBSCertificateList) int {
+	t.Helper()
+
+	for _, extension := range crl.Extensions {
+		if extension.Id.Equal(certutil.DeltaCRLIndicatorOID) {
+			bigInt := new(big.Int)
+			leftOver, err := asn1.Unmarshal(extension.Value, &bigInt)
+			require.NoError(t, err, "Failed unmarshalling delta crl indicator extension")
+			require.Empty(t, leftOver, "leftover bytes from unmarshalling delta crl indicator extension")
+			require.True(t, bigInt.IsInt64(), "parsed delta crl integer is not an int64")
+			require.False(t, math.MaxInt <= bigInt.Int64(), "parsed delta crl integer can not fit in an int")
+			return int(bigInt.Int64())
+		}
+	}
+
+	t.Fatalf("failed to find delta crl indicator extension")
+	return 0
+}
+
+func waitForUpdatedCrl(t *testing.T, client *api.Client, mountPoint string, lastSeenCRLNumber int,
+	maxWait time.Duration,
+) pkix.TBSCertificateList {
+	t.Helper()
+
+	interruptChan := time.After(maxWait)
+	for {
+		select {
+		case <-interruptChan:
+			t.Fatalf("expected CRL to regenerate after %s", maxWait)
+		default:
+			crl := getCrlCertificateList(t, client, mountPoint)
+			thisCRLNumber := getCRLNumber(t, crl)
+			if thisCRLNumber > lastSeenCRLNumber {
+				return crl
+			}
+		}
+	}
+}

sdk/helper/certutil/helpers.go

@@ -78,6 +78,11 @@ var InvSignatureAlgorithmNames = map[x509.SignatureAlgorithm]string{
 	x509.PureEd25519:      "Ed25519",
 }
 
+// OID for RFC 5280 CRL Number extension.
+//
+// > id-ce-cRLNumber OBJECT IDENTIFIER ::= { id-ce 20 }
+var CRLNumberOID = asn1.ObjectIdentifier([]int{2, 5, 29, 20})
+
 // OID for RFC 5280 Delta CRL Indicator CRL extension.
 //
 // > id-ce-deltaCRLIndicator OBJECT IDENTIFIER ::= { id-ce 27 }