[PATCH 1/2] Introduce new SK APIs for freeing memory

Xavier Hsinyuan me at lstlx.com
Sun Dec 22 05:15:31 AEDT 2024


External SecurityKey middleware libraries may use different memory
allocation methods for SK API responses. `ssh-sk-helper` may exhibit
unexpected behavior if it frees memory allocated by external SK
libraries.

Fixed by introducing new SK APIs for freeing memory allocated by and
returned from external SK libraries.
---
 sk-api.h | 11 ++++++++++-
 ssh-sk.c | 34 +++++++++++++++++++++++++++++++---
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/sk-api.h b/sk-api.h
index 08f567a9e..b61f7235c 100644
--- a/sk-api.h
+++ b/sk-api.h
@@ -79,7 +79,7 @@ struct sk_option {
 	uint8_t required;
 };
 
-#define SSH_SK_VERSION_MAJOR		0x000a0000 /* current API version */
+#define SSH_SK_VERSION_MAJOR		0x000b0000 /* current API version */
 #define SSH_SK_VERSION_MAJOR_MASK	0xffff0000
 
 /* Return the version of the middleware API */
@@ -100,4 +100,13 @@ int sk_sign(uint32_t alg, const uint8_t *data, size_t data_len,
 int sk_load_resident_keys(const char *pin, struct sk_option **options,
     struct sk_resident_key ***rks, size_t *nrks);
 
+/* Free sk_sign_response allocated by provider */
+void sk_free_enroll_response(struct sk_enroll_response *enroll_resp);
+
+/* Free sk_sign_response allocated by provider */
+void sk_free_sign_response(struct sk_sign_response *sign_resp);
+
+/* Free sk_resident_key allocated by provider */
+void sk_free_sk_resident_keys(struct sk_resident_key **rks, size_t nrks);
+
 #endif /* _SK_API_H */
diff --git a/ssh-sk.c b/ssh-sk.c
index a2a7d7206..19ac9dda8 100644
--- a/ssh-sk.c
+++ b/ssh-sk.c
@@ -78,6 +78,15 @@ struct sshsk_provider {
 	/* Enumerate resident keys */
 	int (*sk_load_resident_keys)(const char *pin, struct sk_option **opts,
 	    struct sk_resident_key ***rks, size_t *nrks);
+
+	/* Free sk_sign_response allocated by provider */
+	void (*sk_free_enroll_response)(struct sk_enroll_response *ptr);
+
+	/* Free sk_sign_response allocated by provider */
+	void (*sk_free_sign_response)(struct sk_sign_response *ptr);
+
+	/* Free sk_resident_key allocated by provider */
+	void (*sk_free_sk_resident_keys)(struct sk_resident_key **rks, size_t nrks);
 };
 
 /* Built-in version */
@@ -171,6 +180,25 @@ sshsk_open(const char *path)
 		    "failed: %s", path, dlerror());
 		goto fail;
 	}
+
+	if ((ret->sk_free_enroll_response = dlsym(ret->dlhandle,
+	    "sk_free_enroll_response")) == NULL) {
+		error("Provider \"%s\" dlsym(sk_free_enroll_response) "
+		    "failed: %s", path, dlerror());
+		goto fail;
+	}
+	if ((ret->sk_free_sign_response = dlsym(ret->dlhandle,
+	    "sk_free_sign_response")) == NULL) {
+		error("Provider \"%s\" dlsym(sk_free_sign_response) "
+		    "failed: %s", path, dlerror());
+		goto fail;
+	}
+	if ((ret->sk_free_sk_resident_keys = dlsym(ret->dlhandle,
+	    "sk_free_sk_resident_keys")) == NULL) {
+		error("Provider \"%s\" dlsym(sk_free_sk_resident_keys) "
+		    "failed: %s", path, dlerror());
+		goto fail;
+	}
 	/* success */
 	return ret;
 fail:
@@ -568,9 +596,9 @@ sshsk_enroll(int type, const char *provider_path, const char *device,
 	r = 0;
  out:
 	sshsk_free_options(opts);
+	skp->sk_free_enroll_response(resp);
 	sshsk_free(skp);
 	sshkey_free(key);
-	sshsk_free_enroll_response(resp);
 	explicit_bzero(randchall, sizeof(randchall));
 	return r;
 }
@@ -746,8 +774,8 @@ sshsk_sign(const char *provider_path, struct sshkey *key,
 	r = 0;
  out:
 	sshsk_free_options(opts);
+	skp->sk_free_sign_response(resp);
 	sshsk_free(skp);
-	sshsk_free_sign_response(resp);
 	sshbuf_free(sig);
 	sshbuf_free(inner_sig);
 	return r;
@@ -883,8 +911,8 @@ sshsk_load_resident(const char *provider_path, const char *device,
 	r = 0;
  out:
 	sshsk_free_options(opts);
+	skp->sk_free_sk_resident_keys(rks, nrks);
 	sshsk_free(skp);
-	sshsk_free_sk_resident_keys(rks, nrks);
 	sshkey_free(key);
 	sshsk_free_resident_key(srk);
 	sshsk_free_resident_keys(srks, nsrks);
-- 
2.39.5



More information about the openssh-unix-dev mailing list