Merge pull request #18713 from enj/enj/i/serial_file_start

Automatic merge from submit-queue.

Guarantee that SerialFileGenerator starts at 2

This changes makes it so that SerialFileGenerator never returns a value less than 2 for a call to Next.  This guarantees that certificates generated using it do not have a serial number that conflicts with the CA's serial number of 1.  This behavior was lost in 266aa46.

Signed-off-by: Monis Khan <[email protected]>

/kind bug
/assign @liggitt

Supersedes #18710

@openshift/sig-security

Author: openshift-merge-robot

pkg/cmd/server/crypto/crypto.go

@@ -327,10 +327,22 @@ func NewSerialFileGenerator(serialFile string) (*SerialFileGenerator, error) {
 		return nil, err
 	}
 
-	return &SerialFileGenerator{
+	generator := &SerialFileGenerator{
 		Serial:     serial,
 		SerialFile: serialFile,
-	}, nil
+	}
+
+	// 0 is unused and 1 is reserved for the CA itself
+	// Thus we need to guarantee that the first external call to SerialFileGenerator.Next returns 2+
+	// meaning that SerialFileGenerator.Serial must not be less than 1 (it is guaranteed to be non-negative)
+	if generator.Serial < 1 {
+		// fake a call to Next so the file stays in sync and Serial is incremented
+		if _, err := generator.Next(&x509.Certificate{}); err != nil {
+			return nil, err
+		}
+	}
+
+	return generator, nil
 }
 
 // Next returns a unique, monotonically increasing serial number and ensures the CA on disk records that value.

test/cmd/certs.sh

@@ -18,7 +18,7 @@ os::cmd::expect_success_and_not_text \
                                 --overwrite=true" \
     'WARNING: .* is greater than 5 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '00'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '01'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*5)) days" +'%Y')"
@@ -38,7 +38,7 @@ os::cmd::expect_success \
             --signer-key='${CERT_DIR}/ca.key' \
             --signer-serial='${CERT_DIR}/ca.serial.txt'"
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '01'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '02'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 # oc adm ca create-signer-cert should generate certificate with specified number of days and show warning
@@ -50,7 +50,7 @@ os::cmd::expect_success_and_text \
                                 --expire-days=$((365*6))" \
     'WARNING: .* is greater than 5 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '00'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '01'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*6)) days" +'%Y')"
@@ -78,7 +78,7 @@ os::cmd::expect_success_and_not_text \
             --signer-serial='${CERT_DIR}/ca.serial.txt'" \
     'WARNING: .* is greater than 2 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '02'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '03'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*2)) days" +'%Y')"
@@ -107,7 +107,7 @@ os::cmd::expect_success_and_text \
             --expire-days=$((365*3))" \
     'WARNING: .* is greater than 2 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '04'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '05'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*3)) days" +'%Y')"
@@ -133,7 +133,7 @@ os::cmd::expect_success_and_not_text \
     'WARNING: .* is greater than 2 years'
 
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '05'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '06'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*2)) days" +'%Y')"
@@ -155,7 +155,7 @@ os::cmd::expect_success_and_text \
             --expire-days=$((365*3))" \
     'WARNING: .* is greater than 2 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '06'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '07'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*3)) days" +'%Y')"
@@ -176,7 +176,7 @@ os::cmd::expect_success_and_not_text \
                                 --key='${CERT_DIR}/example.org.key'" \
     'WARNING: .* is greater than 2 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '07'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '08'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*2)) days" +'%Y')"
@@ -197,7 +197,7 @@ os::cmd::expect_success_and_text \
                                 --expire-days=$((365*3))" \
     'WARNING: .* is greater than 2 years'
 
-os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '08'
+os::cmd::expect_success_and_text "cat '${CERT_DIR}/ca.serial.txt'" '09'
 os::cmd::expect_success_and_text "tail -c 1 '${CERT_DIR}/ca.serial.txt' | wc -l" '1'  # check for newline at end
 
 expected_year="$(TZ=GMT date -d "+$((365*3)) days" +'%Y')"