Rename the hybrid_encrypt/decrypt functions; label them as dangerous

We need to keep these around for TAP and old-style hidden services,
but they're obsolete, and we shouldn't encourage anyone to use them.
So I've added "obsolete" to their names, and a comment explaining
what the problem is.

Closes ticket 23026.

changes/bug23026

@@ -0,0 +1,4 @@
+  o Code simplification and refactoring:
+    - Rename the obsolete malleable hybrid_encrypt functions used in
+      TAP and old hidden services to indicate that they aren't suitable
+      for new protocols or formats. Closes ticket 23026.

src/common/crypto.c

@@ -1238,9 +1238,12 @@ crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen,
  *   - The beginning of the source data prefixed with a 16-byte symmetric key,
  *     padded and encrypted with the public key; followed by the rest of
  *     the source data encrypted in AES-CTR mode with the symmetric key.
+ *
+ * NOTE that this format does not authenticate the symmetrically encrypted
+ * part of the data, and SHOULD NOT BE USED for new protocols.
  */
 int
-crypto_pk_public_hybrid_encrypt(crypto_pk_t *env,
+crypto_pk_obsolete_public_hybrid_encrypt(crypto_pk_t *env,
                                 char *to, size_t tolen,
                                 const char *from,
                                 size_t fromlen,
@@ -1302,10 +1305,14 @@ crypto_pk_public_hybrid_encrypt(crypto_pk_t *env,
   return -1;
 }
 
-/** Invert crypto_pk_public_hybrid_encrypt. Returns the number of bytes
+/** Invert crypto_pk_obsolete_public_hybrid_encrypt. Returns the number of
- * written on success, -1 on failure. */
+ * bytes written on success, -1 on failure.
+ *
+ * NOTE that this format does not authenticate the symmetrically encrypted
+ * part of the data, and SHOULD NOT BE USED for new protocols.
+ */
 int
-crypto_pk_private_hybrid_decrypt(crypto_pk_t *env,
+crypto_pk_obsolete_private_hybrid_decrypt(crypto_pk_t *env,
                                  char *to,
                                  size_t tolen,
                                  const char *from,

src/common/crypto.h

@@ -197,11 +197,11 @@ int crypto_pk_private_sign(const crypto_pk_t *env, char *to, size_t tolen,
                            const char *from, size_t fromlen);
 int crypto_pk_private_sign_digest(crypto_pk_t *env, char *to, size_t tolen,
                                   const char *from, size_t fromlen);
-int crypto_pk_public_hybrid_encrypt(crypto_pk_t *env, char *to,
+int crypto_pk_obsolete_public_hybrid_encrypt(crypto_pk_t *env, char *to,
                                     size_t tolen,
                                     const char *from, size_t fromlen,
                                     int padding, int force);
-int crypto_pk_private_hybrid_decrypt(crypto_pk_t *env, char *to,
+int crypto_pk_obsolete_private_hybrid_decrypt(crypto_pk_t *env, char *to,
                                      size_t tolen,
                                      const char *from, size_t fromlen,
                                      int padding, int warnOnFailure);

src/or/onion_tap.c

@@ -73,7 +73,7 @@ onion_skin_TAP_create(crypto_pk_t *dest_router_key,
     goto err;
 
   /* set meeting point, meeting cookie, etc here. Leave zero for now. */
-  if (crypto_pk_public_hybrid_encrypt(dest_router_key, onion_skin_out,
+  if (crypto_pk_obsolete_public_hybrid_encrypt(dest_router_key, onion_skin_out,
                                       TAP_ONIONSKIN_CHALLENGE_LEN,
                                       challenge, DH_KEY_LEN,
                                       PK_PKCS1_OAEP_PADDING, 1)<0)
@@ -122,7 +122,7 @@ onion_skin_TAP_server_handshake(
     k = i==0?private_key:prev_private_key;
     if (!k)
       break;
-    len = crypto_pk_private_hybrid_decrypt(k, challenge,
+    len = crypto_pk_obsolete_private_hybrid_decrypt(k, challenge,
                                            TAP_ONIONSKIN_CHALLENGE_LEN,
                                            onion_skin,
                                            TAP_ONIONSKIN_CHALLENGE_LEN,

src/or/rendclient.c

@@ -286,9 +286,9 @@ rend_client_send_introduction(origin_circuit_t *introcirc,
     goto perm_err;
   }
 
-  /*XXX maybe give crypto_pk_public_hybrid_encrypt a max_len arg,
+  /*XXX maybe give crypto_pk_obsolete_public_hybrid_encrypt a max_len arg,
    * to avoid buffer overflows? */
-  r = crypto_pk_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN,
+  r = crypto_pk_obsolete_public_hybrid_encrypt(intro_key, payload+DIGEST_LEN,
                                       sizeof(payload)-DIGEST_LEN,
                                       tmp,
                                       (int)(dh_offset+DH_KEY_LEN),

src/or/rendservice.c

@@ -2732,7 +2732,7 @@ rend_service_decrypt_intro(
 
   /* Decrypt the encrypted part */
   result =
-    crypto_pk_private_hybrid_decrypt(
+    crypto_pk_obsolete_private_hybrid_decrypt(
        key, (char *)buf, sizeof(buf),
        (const char *)(intro->ciphertext), intro->ciphertext_len,
        PK_PKCS1_OAEP_PADDING, 1);

src/test/test.c

@@ -142,7 +142,8 @@ test_bad_onion_handshake(void *arg)
 
   /* Server: Case 1: the encrypted data is degenerate. */
   memset(junk_buf, 0, sizeof(junk_buf));
-  crypto_pk_public_hybrid_encrypt(pk, junk_buf2, TAP_ONIONSKIN_CHALLENGE_LEN,
+  crypto_pk_obsolete_public_hybrid_encrypt(pk,
+                               junk_buf2, TAP_ONIONSKIN_CHALLENGE_LEN,
                                junk_buf, DH_KEY_LEN, PK_PKCS1_OAEP_PADDING, 1);
   tt_int_op(-1, OP_EQ,
             onion_skin_TAP_server_handshake(junk_buf2, pk, NULL,

src/test/test_crypto.c

@@ -1252,10 +1252,10 @@ test_crypto_pk(void *arg)
   for (i = 85; i < 140; ++i) {
     memset(data2,0,1024);
     memset(data3,0,1024);
-    len = crypto_pk_public_hybrid_encrypt(pk1,data2,sizeof(data2),
+    len = crypto_pk_obsolete_public_hybrid_encrypt(pk1,data2,sizeof(data2),
                                           data1,i,PK_PKCS1_OAEP_PADDING,0);
     tt_int_op(len, OP_GE, 0);
-    len = crypto_pk_private_hybrid_decrypt(pk1,data3,sizeof(data3),
+    len = crypto_pk_obsolete_private_hybrid_decrypt(pk1,data3,sizeof(data3),
                                            data2,len,PK_PKCS1_OAEP_PADDING,1);
     tt_int_op(len,OP_EQ, i);
     tt_mem_op(data1,OP_EQ, data3,i);

src/test/test_introduce.c

@@ -355,7 +355,7 @@ make_intro_from_plaintext(
 
   /*
    * Figure out an upper bound on how big the ciphertext will be
-   * (see crypto_pk_public_hybrid_encrypt())
+   * (see crypto_pk_obsolete_public_hybrid_encrypt())
    */
   ciphertext_size = PKCS1_OAEP_PADDING_OVERHEAD;
   ciphertext_size += crypto_pk_keysize(key);
@@ -372,7 +372,7 @@ make_intro_from_plaintext(
   tt_assert(r >= 0);
 
   /* Do encryption */
-  r = crypto_pk_public_hybrid_encrypt(
+  r = crypto_pk_obsolete_public_hybrid_encrypt(
       key, cell + DIGEST_LEN, ciphertext_size,
       buf, len,
       PK_PKCS1_OAEP_PADDING, 0);