fix: pem parsing should allow single dashes in comments

stuffer/s2n_stuffer_pem.c

@@ -20,8 +20,9 @@
 #include "stuffer/s2n_stuffer.h"
 #include "utils/s2n_safety.h"
 
-#define S2N_PEM_DELIMTER_CHAR         '-'
-#define S2N_PEM_DELIMITER_MIN_COUNT   1
+#define S2N_PEM_DELIMITER_CHAR        '-'
+#define S2N_PEM_DELIMITER_TOKEN       "--"
+#define S2N_PEM_DELIMITER_MIN_COUNT   2
 #define S2N_PEM_DELIMITER_MAX_COUNT   64
 #define S2N_PEM_BEGIN_TOKEN           "BEGIN "
 #define S2N_PEM_END_TOKEN             "END "
@@ -36,11 +37,15 @@
 static int s2n_stuffer_pem_read_encapsulation_line(struct s2n_stuffer *pem, const char *encap_marker,
         const char *keyword)
 {
-    /* Skip any number of Chars until a "-" is reached */
-    POSIX_GUARD(s2n_stuffer_skip_to_char(pem, S2N_PEM_DELIMTER_CHAR));
+    /* Skip any number of Chars until a "--" is reached.
+     * We use "--" instead of "-" to account for dashes that appear in comments.
+     * We do not accept comments that contain "--".
+     */
+    POSIX_GUARD(s2n_stuffer_skip_read_until(pem, S2N_PEM_DELIMITER_TOKEN));
+    POSIX_GUARD(s2n_stuffer_rewind_read(pem, strlen(S2N_PEM_DELIMITER_TOKEN)));
 
-    /* Ensure between 1 and 64 '-' chars at start of line */
-    POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMTER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT,
+    /* Ensure between 2 and 64 '-' chars at start of line. */
+    POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMITER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT,
             S2N_PEM_DELIMITER_MAX_COUNT, NULL));
 
     /* Ensure next string in stuffer is "BEGIN " or "END " */
@@ -49,18 +54,18 @@ static int s2n_stuffer_pem_read_encapsulation_line(struct s2n_stuffer *pem, cons
     /* Ensure next string is stuffer is the keyword (Eg "CERTIFICATE", "PRIVATE KEY", etc) */
     POSIX_GUARD(s2n_stuffer_read_expected_str(pem, keyword));
 
-    /* Ensure between 1 and 64 '-' chars at end of line */
-    POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMTER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT,
+    /* Ensure between 2 and 64 '-' chars at end of line */
+    POSIX_GUARD(s2n_stuffer_skip_expected_char(pem, S2N_PEM_DELIMITER_CHAR, S2N_PEM_DELIMITER_MIN_COUNT,
             S2N_PEM_DELIMITER_MAX_COUNT, NULL));
 
     /* Check for missing newline between dashes case: "-----END CERTIFICATE----------BEGIN CERTIFICATE-----" */
     if (strncmp(encap_marker, S2N_PEM_END_TOKEN, strlen(S2N_PEM_END_TOKEN)) == 0
             && s2n_stuffer_peek_check_for_str(pem, S2N_PEM_BEGIN_TOKEN) == S2N_SUCCESS) {
-        /* Rewind stuffer by 1 byte before BEGIN, so that next read will find the dash before the BEGIN */
-        POSIX_GUARD(s2n_stuffer_rewind_read(pem, 1));
+        /* Rewind stuffer by 2 bytes before BEGIN, so that next read will find the dashes before the BEGIN */
+        POSIX_GUARD(s2n_stuffer_rewind_read(pem, S2N_PEM_DELIMITER_MIN_COUNT));
     }
 
-    /* Skip newlines and other whitepsace that may be after the dashes */
+    /* Skip newlines and other whitespace that may be after the dashes */
     return s2n_stuffer_skip_whitespace(pem, NULL);
 }
 

tests/cbmc/proofs/s2n_stuffer_certificate_from_pem/Makefile

@@ -49,6 +49,6 @@ REMOVE_FUNCTION_BODY += s2n_add_overflow
 UNWINDSET += strlen.0:5 # size of S2N_PEM_PKCS1_RSA_PRIVATE_KEY
 UNWINDSET += strncmp.0:5 # size of S2N_PEM_END_TOKEN
 UNWINDSET += __CPROVER_file_local_s2n_stuffer_pem_c_s2n_stuffer_pem_read_contents.11:$(call addone,$(MAX_BLOB_SIZE))
-UNWINDSET += s2n_stuffer_skip_to_char.3:$(call addone,$(MAX_BLOB_SIZE))
+UNWINDSET +=  s2n_stuffer_skip_read_until.10:$(call addone,$(MAX_BLOB_SIZE))
 
 include ../Makefile.common

tests/cbmc/proofs/s2n_stuffer_dhparams_from_pem/Makefile

@@ -49,6 +49,6 @@ REMOVE_FUNCTION_BODY += s2n_add_overflow
 UNWINDSET += strlen.0:5 # size of S2N_PEM_PKCS1_RSA_PRIVATE_KEY
 UNWINDSET += strncmp.0:5 # size of S2N_PEM_END_TOKEN
 UNWINDSET += __CPROVER_file_local_s2n_stuffer_pem_c_s2n_stuffer_pem_read_contents.11:$(call addone,$(MAX_BLOB_SIZE))
-UNWINDSET += s2n_stuffer_skip_to_char.3:$(call addone,$(MAX_BLOB_SIZE))
+UNWINDSET +=  s2n_stuffer_skip_read_until.10:$(call addone,$(MAX_BLOB_SIZE))
 
 include ../Makefile.common

tests/cbmc/proofs/s2n_stuffer_private_key_from_pem/Makefile

@@ -52,5 +52,6 @@ UNWINDSET += strlen.0:5 # size of S2N_PEM_PKCS1_RSA_PRIVATE_KEY
 UNWINDSET += strncmp.0:5 # size of S2N_PEM_END_TOKEN
 UNWINDSET += __CPROVER_file_local_s2n_stuffer_pem_c_s2n_stuffer_pem_read_contents.11:$(call addone,$(MAX_BLOB_SIZE))
 UNWINDSET += s2n_stuffer_skip_to_char.0:$(call addone,$(MAX_BLOB_SIZE))
+UNWINDSET += s2n_stuffer_skip_read_until.10:$(call addone,$(MAX_BLOB_SIZE))
 
 include ../Makefile.common

tests/pems/rsa_2048_weird_dashes_cert.pem

@@ -1,4 +1,4 @@
--BEGIN CERTIFICATE-
+--BEGIN CERTIFICATE--
 MIICrTCCAZUCAn3VMA0GCSqGSIb3DQEBBQUAMB4xHDAaBgNVBAMME3MyblRlc3RJ
 bnRlcm1lZGlhdGUwIBcNMTYwMzMwMTg1NzQzWhgPMjExNjAzMDYxODU3NDNaMBgx
 FjAUBgNVBAMMDXMyblRlc3RTZXJ2ZXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAw
@@ -14,7 +14,7 @@ jxUvy7UQvXrPqaHbODrHe+7f7r1YCzerujiP5SSHphY3GQq88KemfFczp/4GnYas
 sE50OYe7DQcB4zvnxmAXp51JIN4ooktUU9oKIM5y2cgEWdmJzeqPANYxf0ZIPlTg
 ETknKw1Dzf8wlK5mFbbG4LPQh1mkDVcwQV3ogG6kGMRa7neH+6SFkNpAKuPCoje4
 NAE+WQ5ve1wk7nIRTQwDAF4=
--END CERTIFICATE-
+--END CERTIFICATE--
 -----------------------BEGIN CERTIFICATE------------------------
 MIIDKTCCAhGgAwIBAgICVxYwDQYJKoZIhvcNAQEFBQAwFjEUMBIGA1UEAwwLczJu
 VGVzdFJvb3QwIBcNMTYwMzMwMTg1NzA5WhgPMjExNjAzMDYxODU3MDlaMB4xHDAa
@@ -33,7 +33,7 @@ kNhs5xYprdU82AqcaWwEd0kDrhC5rEvs6fj1J0NKmmhbovYxuDboj0a7If7HEqX0
 NizyU3M3JONPZgadchZ+F5DosatF1Bpt/gsQRy383IogQ0/FS+juHCCc4VIUemuk
 YY1J8o5XdrGWrPBBiudTWqCobe+N541b+YLWbajT5UKzvSqJmcqpPTniJGc9eZxc
 z3cCNd3cKa9bK51stEnQSlA7PQXYs3K+TD3EmSn/G2x6Hmfr7lrpbIhEaD+y
--END CERTIFICATE--BEGIN CERTIFICATE-
+--END CERTIFICATE--BEGIN CERTIFICATE--
 MIIDATCCAemgAwIBAgIJANDUkH+UYdz1MA0GCSqGSIb3DQEBCwUAMBYxFDASBgNV
 BAMMC3MyblRlc3RSb290MCAXDTE2MDMzMDE4NTYzOVoYDzIxMTYwMzA2MTg1NjM5
 WjAWMRQwEgYDVQQDDAtzMm5UZXN0Um9vdDCCASIwDQYJKoZIhvcNAQEBBQADggEP