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
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
