Improved documentation of SCT_CTX_* functions

Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>

2 files changed, 29 insertions, 15 deletions

diff --git a/crypto/ct/ct_locl.h b/crypto/ct/ct_locl.h
index d13f70cdfa..95a3299755 100644
--- a/crypto/ct/ct_locl.h
+++ b/crypto/ct/ct_locl.h
@@ -168,14 +168,18 @@ SCT_CTX *SCT_CTX_new(void);
 void SCT_CTX_free(SCT_CTX *sctx);
 
 /*
- * Sets the certificate that the SCT is being verified against.
- * This will fail if the certificate is invalid.
+ * Sets the certificate that the SCT was created for.
+ * If *cert does not have a poison extension, presigner must be NULL.
+ * If *cert does not have a poison extension, it may have a single SCT
+ * (NID_ct_precert_scts) extension.
+ * If either *cert or *presigner have an AKID (NID_authority_key_identifier)
+ * extension, both must have one.
  * Returns 1 on success, 0 on failure.
  */
 __owur int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner);
 
 /*
- * Sets the issuer of the certificate that the SCT is being verified against.
+ * Sets the issuer of the certificate that the SCT was created for.
  * This is just a convenience method to save extracting the public key and
  * calling SCT_CTX_set1_issuer_pubkey().
  * Issuer must not be NULL.
@@ -184,8 +188,8 @@ __owur int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner);
 __owur int SCT_CTX_set1_issuer(SCT_CTX *sctx, const X509 *issuer);
 
 /*
- * Sets the public key of the issuer of the certificate that the SCT is being
- * verified against.
+ * Sets the public key of the issuer of the certificate that the SCT was created
+ * for.
  * The public key must not be NULL.
  * Returns 1 on success, 0 on failure.
  */
diff --git a/crypto/ct/ct_sct_ctx.c b/crypto/ct/ct_sct_ctx.c
index 89051d2f7c..13937c75ca 100644
--- a/crypto/ct/ct_sct_ctx.c
+++ b/crypto/ct/ct_sct_ctx.c
@@ -164,13 +164,13 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
     int poison_ext_is_dup, sct_ext_is_dup;
     int poison_idx = ct_x509_get_ext(cert, NID_ct_precert_poison, &poison_ext_is_dup);
 
-    /* Duplicate poison */
+    /* Duplicate poison extensions are present - error */
     if (poison_ext_is_dup)
         goto err;
 
-    /* If no poison extension, store encoding */
+    /* If *cert doesn't have a poison extension, it isn't a precert */
     if (poison_idx == -1) {
-        /* presigner must have poison */
+        /* cert isn't a precert, so we shouldn't have a presigner */
         if (presigner != NULL)
             goto err;
 
@@ -179,20 +179,30 @@ int SCT_CTX_set1_cert(SCT_CTX *sctx, X509 *cert, X509 *presigner)
             goto err;
     }
 
-    /* See if have precert scts extension */
+    /* See if cert has a precert SCTs extension */
     idx = ct_x509_get_ext(cert, NID_ct_precert_scts, &sct_ext_is_dup);
-    /* Duplicate scts */
+    /* Duplicate SCT extensions are present - error */
     if (sct_ext_is_dup)
         goto err;
 
-    if (idx >= 0) {
-        /* Can't have both poison and scts */
-        if (poison_idx >= 0)
-            goto err;
-    } else {
+    if (idx >= 0 && poison_idx >= 0) {
+        /*
+         * cert can't both contain SCTs (i.e. have an SCT extension) and be a
+         * precert (i.e. have a poison extension).
+         */
+        goto err;
+    }
+
+    if (idx == -1) {
         idx = poison_idx;
     }
 
+    /*
+     * If either a poison or SCT extension is present, remove it before encoding
+     * cert. This, along with ct_x509_cert_fixup(), gets a TBSCertificate (see
+     * RFC5280) from cert, which is what the CT log signed when it produced the
+     * SCT.
+     */
     if (idx >= 0) {
         X509_EXTENSION *ext;