Apply upstream refactor to sshkey.c

In PR #159 we resolved a massive merge conflict in `sshkey.c` by simply taking the upstream version and removing all PQ SSH key support from the trunk. This broke the build/tests and obviously the ability to use PQ SSH keys. This PR restores PQ SSH key support for "pure" PQ algorithms. A follow-up PR will bring back support for hybrid-PQ. This was done to break up the PRs and do things more incrementally.

At a high-level, there was a major refactor of `sshkey` upstream which moved algorithm implementations to an OOP abstraction via the `sshkey_impl` struct. This allowed them to get rid of all the switch statements, encapsulate algorithm specific code, and standardize on a single factory method to get the right implementation. Some other minor things like the function signatures for sign/verify were updated to accept new optional arguments.

Getting OQS over to this new system was a matter of defining these `sshkey_impl` structs and factoring out the relevant OQS bits to the proper functions. This involved a number of changes/renames/deletions to the templates. I also performed a 3-way merge to pull-in changes that weren't directly impacted by the refactor.

There is no support for hybrid SSH Key support for now. This is to make sure that the baseline refactor is in a good shape before committing to a certain path. To support hybrid, I propose implementing generic hybrid versions of these classes. The only tricky thing is figuring out how to expose the RSA/EC implementations to the `ssh-oqs` file (probably declaring externs?). I considered doing this "on the outside", but that seems to go against what the refactor aimed to do and adds a lot more complexity to the sshkey source than seems necessary.

What's working now?
- `build_openssh.sh` now compiles and installs successfully 🎉
- `make tests` runs through a lot of tests before failing now. This is my next line of inquiry to chase down.

Signed-off-by: Gerardo Ravago <[email protected]>

kexgen.c

@@ -619,31 +619,31 @@ input_kex_gen_init(int type, u_int32_t seq, struct ssh *ssh)
 ///// OQS_TEMPLATE_FRAGMENT_ADD_INIT_SWITCH_CASES_START
 	case KEX_KEM_FRODOKEM_640_AES_SHA256:
 		r = kex_kem_frodokem_640_aes_enc(kex, client_pubkey,
-										 &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_FRODOKEM_976_AES_SHA384:
 		r = kex_kem_frodokem_976_aes_enc(kex, client_pubkey,
-										 &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_FRODOKEM_1344_AES_SHA512:
 		r = kex_kem_frodokem_1344_aes_enc(kex, client_pubkey,
-										  &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_FRODOKEM_640_SHAKE_SHA256:
 		r = kex_kem_frodokem_640_shake_enc(kex, client_pubkey,
-										   &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_FRODOKEM_976_SHAKE_SHA384:
 		r = kex_kem_frodokem_976_shake_enc(kex, client_pubkey,
-										   &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_FRODOKEM_1344_SHAKE_SHA512:
 		r = kex_kem_frodokem_1344_shake_enc(kex, client_pubkey,
-											&server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_KYBER_512_SHA256:
 		r = kex_kem_kyber_512_enc(kex, client_pubkey,
-								  &server_pubkey, &shared_secret);
+		    &server_pubkey, &shared_secret);
 		break;
 	case KEX_KEM_KYBER_768_SHA384:
 		r = kex_kem_kyber_768_enc(kex, client_pubkey,

monitor.c

@@ -1775,7 +1775,7 @@ monitor_apply_keystate(struct ssh *ssh, struct monitor *pmonitor)
 	kex->kex[KEX_KEM_HQC_192_SHA384] = kex_gen_server;
 	kex->kex[KEX_KEM_HQC_256_SHA512] = kex_gen_server;
 #ifdef WITH_OPENSSL
-	#ifdef OPENSSL_HAS_ECC
+#ifdef OPENSSL_HAS_ECC
 	kex->kex[KEX_KEM_FRODOKEM_640_AES_ECDH_NISTP256_SHA256] = kex_gen_server;
 	kex->kex[KEX_KEM_FRODOKEM_976_AES_ECDH_NISTP384_SHA384] = kex_gen_server;
 	kex->kex[KEX_KEM_FRODOKEM_1344_AES_ECDH_NISTP521_SHA512] = kex_gen_server;

oqs-template/ssh-keygen.c/print_resource_records.fragment

@@ -1,11 +1,11 @@
 {%- for sig in config['sigs'] %}
 			n += do_print_resource_record(pw,
 			    _PATH_HOST_{{ sig['name']|upper }}_KEY_FILE, rr_hostname,
-			    print_generic);
+			    print_generic, opts, nopts);
     {%- for alg in sig['mix_with'] %}
 			n += do_print_resource_record(pw,
 			    _PATH_HOST_{{ alg['name']|upper }}_{{ sig['name']|upper }}_KEY_FILE, rr_hostname,
-			    print_generic);
+			    print_generic, opts, nopts);
     {%- endfor %}
 {%- endfor %}
 

oqs-template/ssh-oqs.c/define_sig_functions.fragment

@@ -1,37 +1,116 @@
 {%- for sig in config['sigs'] %}
+{%- set symbol_base_name = sig['name']|replace('_','') %}
 /*---------------------------------------------------
  * {{ sig['name']|upper }} METHODS
  *---------------------------------------------------
  */
-int ssh_{{ sig['name']|replace('_','') }}_sign(const struct sshkey *key,
+static int ssh_{{ symbol_base_name }}_generate(struct sshkey *k, int bits)
+{
+  k->oqs_pk_len = oqs_sig_pk_len(k->type);
+  k->oqs_sk_len = oqs_sig_sk_len(k->type);
+  if ((k->oqs_pk = malloc(k->oqs_pk_len)) == NULL ||
+      (k->oqs_sk = malloc(k->oqs_sk_len)) == NULL) {
+    return SSH_ERR_ALLOC_FAIL;
+  }
+  return OQS_SIG_{{ sig['name'] }}_keypair(k->oqs_pk, k->oqs_sk);
+}
+
+int ssh_{{ symbol_base_name }}_sign(const struct sshkey *key,
                      u_char **sigp,
                      size_t *lenp,
                      const u_char *data,
                      size_t datalen,
+                     const char *alg,
+                     const char *sk_provider,
+                     const char *sk_pin,
                      u_int compat)
 {
     OQS_SIG *sig = OQS_SIG_new(OQS_SIG_alg_{{ sig['name'] }});
     if (sig == NULL) {
         return SSH_ERR_ALLOC_FAIL;
     }
-    int r = ssh_generic_sign(sig, "{{ sig['name']|replace('_','') }}", key, sigp, lenp, data, datalen, compat);
+    int r = ssh_generic_sign(sig, "{{ symbol_base_name }}", key, sigp, lenp, data, datalen, compat);
     OQS_SIG_free(sig);
     return r;
 }
-int ssh_{{ sig['name']|replace('_','') }}_verify(const struct sshkey *key,
+
+int ssh_{{ symbol_base_name }}_verify(const struct sshkey *key,
                        const u_char *signature,
                        size_t signaturelen,
                        const u_char *data,
                        size_t datalen,
+                       const char *alg,
                        u_int compat)
 {
     OQS_SIG *sig = OQS_SIG_new(OQS_SIG_alg_{{ sig['name'] }});
     if (sig == NULL) {
         return SSH_ERR_ALLOC_FAIL;
     }
-    int r = ssh_generic_verify(sig, "{{ sig['name']|replace('_','') }}", key, signature, signaturelen, data, datalen, compat);
+    int r = ssh_generic_verify(sig, "{{ symbol_base_name }}", key, signature, signaturelen, data, datalen, compat);
     OQS_SIG_free(sig);
     return r;
 }
+
+static const struct sshkey_impl_funcs sshkey_{{ symbol_base_name }}_funcs = {
+  /* .size = */ ssh_generic_size,
+  /* .alloc = */ ssh_generic_alloc,
+  /* .cleanup = */ ssh_generic_cleanup,
+  /* .equal = */ ssh_generic_equal,
+  /* .ssh_serialize_public = */ ssh_generic_serialize_public,
+  /* .ssh_deserialize_public = */ ssh_generic_deserialize_public,
+  /* .ssh_serialize_private = */ ssh_generic_serialize_private,
+  /* .ssh_deserialize_private = */ ssh_generic_deserialize_private,
+  /* .generate = */ ssh_{{ symbol_base_name }}_generate,
+  /* .copy_public = */ ssh_generic_copy_public,
+  /* .sign = */ ssh_{{ symbol_base_name }}_sign,
+  /* .verify = */ ssh_{{ symbol_base_name }}_verify,
+};
+
+const struct sshkey_impl sshkey_{{ symbol_base_name }}_impl = {
+  /* .name = */ "ssh-{{ symbol_base_name }}",
+  /* .shortname = */ "{{ symbol_base_name|upper }}",
+  /* .sigalg = */ NULL,
+  /* .type = */ KEY_{{ sig['name']|upper }},
+  /* .nid = */ 0,
+  /* .cert = */ 0,
+  /* .sigonly = */ 0,
+  /* .keybits = */ 256, // TODO - What should be here?
+  /* .funcs = */ &sshkey_{{ symbol_base_name }}_funcs,
+};
+{%- endfor %}
+
+#ifdef HYBRID_IMPLEMENTATION_EXISTS
+// #ifdef WITH_OPENSSL
+{%- for sig in config['sigs'] %}
+{%- for alg in sig['mix_with'] if alg['rsa'] %}
+{%- set symbol_base_name = alg['name']|replace('_','') + '_' + sig['name']|replace('_','') %}
+static const struct sshkey_impl_funcs sshkey_{{ symbol_base_name }}_funcs = {
+  /* .size = */ ssh_generic_size,
+  /* .alloc = */ ssh_generic_alloc,
+  /* .cleanup = */ ssh_generic_cleanup,
+  /* .equal = */ ssh_generic_equal,
+  /* .ssh_serialize_public = */ ssh_generic_serialize_public,
+  /* .ssh_deserialize_public = */ ssh_generic_deserialize_public,
+  /* .ssh_serialize_private = */ ssh_generic_serialize_private,
+  /* .ssh_deserialize_private = */ ssh_generic_deserialize_private,
+  /* .generate = */ ssh_{{ symbol_base_name }}_generate,
+  /* .copy_public = */ ssh_generic_copy_public,
+  /* .sign = */ ssh_{{ symbol_base_name }}_sign,
+  /* .verify = */ ssh_{{ symbol_base_name }}_verify,
+};
+
+const struct sshkey_impl sshkey_{{ symbol_base_name }}_impl = {
+  /* .name = */ "ssh-{{ symbol_base_name }}",
+  /* .shortname = */ "{{ symbol_base_name|upper }}",
+  /* .sigalg = */ NULL,
+  /* .type = */ KEY_{{ sig['name']|upper }},
+  /* .nid = */ 0,
+  /* .cert = */ 0,
+  /* .sigonly = */ 0,
+  /* .keybits = */ 256, // TODO - What should be here?
+  /* .funcs = */ &sshkey_{{ symbol_base_name }}_funcs,
+};
+{%- endfor %}
 {%- endfor %}
+#endif /* WITH_OPENSSL */
 

oqs-template/ssh-oqs.c/return_pk_len.fragment

@@ -0,0 +1,8 @@
+{%- for sig in config['sigs'] %}
+    case KEY_{{ sig['name']|upper }}:
+  {%- for alg in sig['mix_with'] %}
+    case KEY_{{ alg['name']|upper }}_{{ sig['name']|upper }}:
+  {%- endfor -%}
+      return OQS_SIG_{{ sig['name'] }}_length_public_key;
+{%- endfor %}
+

oqs-template/ssh-oqs.c/return_sk_len.fragment

@@ -0,0 +1,8 @@
+{%- for sig in config['sigs'] %}
+    case KEY_{{ sig['name']|upper }}:
+  {%- for alg in sig['mix_with'] %}
+    case KEY_{{ alg['name']|upper }}_{{ sig['name']|upper }}:
+  {%- endfor %}
+      return OQS_SIG_{{ sig['name'] }}_length_secret_key;
+{%- endfor %}
+

oqs-template/sshkey.c/define_keytypes.fragment

@@ -1,20 +1,18 @@
 {%- for sig in config['sigs'] %}
-	{ "ssh-{{ sig['name']|replace('_','') }}", "{{ sig['name']|replace('_','')|upper }}", NULL,
-	    KEY_{{ sig['name']|upper }}, 0, 0, 0 },
+	&sshkey_{{ sig['name']|replace('_','') }}_impl,
 {%- endfor %}
-#ifdef WITH_OPENSSL
+#ifdef HYBRID_IMPLEMENTATION_EXISTS
+// #ifdef WITH_OPENSSL
 {%- for sig in config['sigs'] %}
-    {%- for alg in sig['mix_with'] if alg['rsa'] %}
-	{ "ssh-{{ alg['name'] }}-{{ sig['name']|replace('_','') }}", "{{ alg['name']|upper }}_{{ sig['name']|replace('_','')|upper }}", NULL,
-	    KEY_{{ alg['name']|upper }}_{{ sig['name']|upper }}, 0, 0, 0 },
-    {%- endfor %}
+	{%- for alg in sig['mix_with'] if alg['rsa'] %}
+	&sshkey_{{ alg['name']|replace('_','') }}_{{ sig['name']|replace('_','') }}_impl,
+	{%- endfor %}
 {%- endfor %}
 #ifdef OPENSSL_HAS_ECC
 {%- for sig in config['sigs'] %}
-    {%- for alg in sig['mix_with'] if not alg['rsa'] %}
-	{ "ssh-{{ alg['name']|replace('_','-') }}-{{ sig['name']|replace('_','') }}", "{{ alg['name']|upper }}_{{ sig['name']|replace('_','')|upper }}", NULL,
-	    KEY_{{ alg['name']|upper }}_{{ sig['name']|upper }}, {{ alg['openssl_nid'] }}, 0, 0 },
-    {%- endfor %}
+	{%- for alg in sig['mix_with'] if not alg['rsa'] %}
+	&sshkey_{{ alg['name']|replace('_','') }}_{{ sig['name']|replace('_','') }}_impl,
+	{%- endfor %}
 {%- endfor %}
 #endif /* OPENSSL_HAS_ECC */
 #endif /* WITH_OPENSSL */

oqs-template/sshkey.c/extern_key_impls.fragment

@@ -0,0 +1,20 @@
+{%- for sig in config['sigs'] %}
+extern const struct sshkey_impl sshkey_{{ sig['name']|replace('_','') }}_impl;
+{%- endfor %}
+
+#ifdef HYBRID_IMPLEMENTATION_EXISTS
+// #ifdef WITH_OPENSSL
+{%- for sig in config['sigs'] %}
+  {%- for alg in sig['mix_with'] if alg['rsa'] %}
+extern const struct sshkey_impl sshkey_{{ alg['name']|replace('_','') }}_{{ sig['name']|replace('_','') }}_impl;
+  {%- endfor %}
+{%- endfor %}
+#ifdef OPENSSL_HAS_ECC
+{%- for sig in config['sigs'] %}
+  {%- for alg in sig['mix_with'] if not alg['rsa'] %}
+extern const struct sshkey_impl sshkey_{{ alg['name']|replace('_','') }}_{{ sig['name']|replace('_','') }}_impl;
+  {%- endfor %}
+{%- endfor %}
+#endif /* OPENSSL_HAS_ECC */
+#endif /* WITH_OPENSSL */
+