diff --git a/include/gmssl/sdf.h b/include/gmssl/sdf.h
index 5ccce4ae..7ec90774 100644
--- a/include/gmssl/sdf.h
+++ b/include/gmssl/sdf.h
@@ -46,20 +46,17 @@ typedef struct {
 } SDF_CBC_CTX;
 
 typedef struct {
-	SM2_Z256_POINT public_key;
 	void *session;
 	int index;
-} SDF_SIGN_KEY;
+} SDF_PRIVATE_KEY;
 
 typedef struct {
 	SM3_CTX sm3_ctx;
 	SM3_CTX saved_sm3_ctx;
-	SDF_SIGN_KEY key;
+	SDF_PRIVATE_KEY key;
 } SDF_SIGN_CTX;
 
 
-
-
 int sdf_load_library(const char *so_path, const char *vendor);
 int sdf_open_device(SDF_DEVICE *dev);
 int sdf_print_device_info(FILE *fp, int fmt, int ind, const char *lable, SDF_DEVICE *dev);
@@ -67,9 +64,9 @@ int sdf_digest_init(SDF_DIGEST_CTX *ctx, SDF_DEVICE *dev);
 int sdf_digest_update(SDF_DIGEST_CTX *ctx, const uint8_t *data, size_t datalen);
 int sdf_digest_finish(SDF_DIGEST_CTX *ctx, uint8_t dgst[SM3_DIGEST_SIZE]);
 int sdf_digest_reset(SDF_DIGEST_CTX *ctx);
-void sdf_digest_cleanup(SDF_DIGEST_CTX *ctx);
+int sdf_digest_cleanup(SDF_DIGEST_CTX *ctx);
 int sdf_generate_key(SDF_DEVICE *dev, SDF_KEY *key, const SM2_KEY *sm2_key, uint8_t *wrappedkey, size_t *wrappedkey_len);
-int sdf_import_key(SDF_DEVICE *dev, unsigned int key_index, const char *pass, const uint8_t *wrappedkey, size_t wrappedkey_len, SDF_KEY *key);
+int sdf_import_key(SDF_DEVICE *dev, unsigned int key_index, const char *pass, const uint8_t *wrappedkey, size_t wrappedkey_len, SDF_KEY *key); // XXX: Is `pass` needed? see impl in sdf.c
 int sdf_cbc_encrypt_init(SDF_CBC_CTX *ctx, const SDF_KEY *key, const uint8_t iv[16]);
 int sdf_cbc_encrypt_update(SDF_CBC_CTX *ctx, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen);
 int sdf_cbc_encrypt_finish(SDF_CBC_CTX *ctx, uint8_t *out, size_t *outlen);
@@ -79,14 +76,14 @@ int sdf_cbc_decrypt_finish(SDF_CBC_CTX *ctx, uint8_t *out, size_t *outlen);
 int sdf_destroy_key(SDF_KEY *key);
 int sdf_export_sign_public_key(SDF_DEVICE *dev, int key_index, SM2_KEY *public_key);
 int sdf_export_encrypt_public_key(SDF_DEVICE *dev, int key_index, SM2_KEY *public_key);
-int sdf_load_sign_key(SDF_DEVICE *dev, SDF_SIGN_KEY *key, int key_index, const char *pass);
-int sdf_sign(SDF_SIGN_KEY *key, const uint8_t dgst[32], uint8_t *sig, size_t *siglen);
-int sdf_sign_init(SDF_SIGN_CTX *ctx, const SDF_SIGN_KEY *key, const char *id, size_t idlen);
+int sdf_load_private_key(SDF_DEVICE *dev, SDF_PRIVATE_KEY *key, int key_index, const char *pass);
+int sdf_decrypt(const SDF_PRIVATE_KEY *key, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen);
+int sdf_sign(const SDF_PRIVATE_KEY *key, const uint8_t dgst[32], uint8_t *sig, size_t *siglen);
+int sdf_sign_init(SDF_SIGN_CTX *ctx, const SDF_PRIVATE_KEY *key, const char *id, size_t idlen);
 int sdf_sign_update(SDF_SIGN_CTX *ctx, const uint8_t *data, size_t datalen);
 int sdf_sign_finish(SDF_SIGN_CTX *ctx, uint8_t *sig, size_t *siglen);
 int sdf_sign_reset(SDF_SIGN_CTX *ctx);
-int sdf_sm2_decrypt(const SDF_SIGN_KEY *key, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen);
-int sdf_release_sign_key(SDF_SIGN_KEY *key);
+int sdf_release_private_key(SDF_PRIVATE_KEY *key);
 int sdf_close_device(SDF_DEVICE *dev);
 void sdf_unload_library(void);
 
diff --git a/src/sdf/sdf.c b/src/sdf/sdf.c
index 73de3b57..6d9188fc 100755
--- a/src/sdf/sdf.c
+++ b/src/sdf/sdf.c
@@ -255,15 +255,16 @@ int sdf_digest_reset(SDF_DIGEST_CTX *ctx)
 	return 1;
 }
 
-void sdf_digest_cleanup(SDF_DIGEST_CTX *ctx)
+int sdf_digest_cleanup(SDF_DIGEST_CTX *ctx)
 {
 	if (ctx && ctx->session) {
-		int ret;
-		if ((ret = SDF_CloseSession(ctx->session)) != SDR_OK) {
+		if (SDF_CloseSession(ctx->session) != SDR_OK) {
 			error_print();
+			return -1;
 		}
 		ctx->session = NULL;
 	}
+	return 1;
 }
 
 static int sdf_cbc_encrypt_blocks(SDF_KEY *key, uint8_t iv[16], const uint8_t *in, size_t nblocks, uint8_t *out)
@@ -456,6 +457,10 @@ int sdf_destroy_key(SDF_KEY *key)
 	return 1;
 }
 
+// FIXME:
+// If SDF_ImportKeyWithISK_ECC does not need the GetPrivateKeyAccessRight
+// then we can use `key_index` or `SDF_PRIVATE_KEY` as arg
+// It's not secure to keep `pass` in memory
 int sdf_import_key(SDF_DEVICE *dev, unsigned int key_index, const char *pass,
 	const uint8_t *wrappedkey, size_t wrappedkey_len, SDF_KEY *key)
 {
@@ -491,6 +496,7 @@ int sdf_import_key(SDF_DEVICE *dev, unsigned int key_index, const char *pass,
 		error_print();
 		return -1;
 	}
+	// XXX: does import_key need the right?
 	if (SDF_GetPrivateKeyAccessRight(hSession, key_index, (unsigned char *)pass, (unsigned int)strlen(pass)) != SDR_OK) {
 		(void)SDF_CloseSession(hSession);
 		error_print();
@@ -739,8 +745,7 @@ int sdf_export_encrypt_public_key(SDF_DEVICE *dev, int key_index, SM2_KEY *sm2_k
 	return 1;
 }
 
-// 这个要改为load_key
-int sdf_load_sign_key(SDF_DEVICE *dev, SDF_SIGN_KEY *key, int key_index, const char *pass)
+int sdf_load_private_key(SDF_DEVICE *dev, SDF_PRIVATE_KEY *key, int key_index, const char *pass)
 {
 	void *hSession = NULL;
 	ECCrefPublicKey eccPublicKey;
@@ -758,27 +763,18 @@ int sdf_load_sign_key(SDF_DEVICE *dev, SDF_SIGN_KEY *key, int key_index, const c
 		error_print();
 		return -1;
 	}
-	if (SDF_ExportSignPublicKey_ECC(hSession, key_index, &eccPublicKey) != SDR_OK) {
-		(void)SDF_CloseSession(hSession);
-		error_print();
-		return -1;
-	}
 	if (SDF_GetPrivateKeyAccessRight(hSession, key_index, (unsigned char *)pass, (unsigned int)strlen(pass)) != SDR_OK) {
 		(void)SDF_CloseSession(hSession);
 		error_print();
 		return -1;
 	}
 
-	if (ECCrefPublicKey_to_SM2_Z256_POINT(&eccPublicKey, &key->public_key) != 1) {
-		error_print();
-		return -1;
-	}
 	key->session = hSession;
 	key->index = key_index;
 	return 1;
 }
 
-int sdf_sign(SDF_SIGN_KEY *key, const uint8_t dgst[32], uint8_t *sig, size_t *siglen)
+int sdf_sign(const SDF_PRIVATE_KEY *key, const uint8_t dgst[32], uint8_t *sig, size_t *siglen)
 {
 	ECCSignature ecc_sig;
 	SM2_SIGNATURE sm2_sig;
@@ -803,7 +799,7 @@ int sdf_sign(SDF_SIGN_KEY *key, const uint8_t dgst[32], uint8_t *sig, size_t *si
 	return 1;
 }
 
-int sdf_sm2_decrypt(const SDF_SIGN_KEY *key, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen)
+int sdf_decrypt(const SDF_PRIVATE_KEY *key, const uint8_t *in, size_t inlen, uint8_t *out, size_t *outlen)
 {
 	ECCCipher eccCipher;
 	SM2_CIPHERTEXT ciphertext;
@@ -842,12 +838,28 @@ int sdf_sm2_decrypt(const SDF_SIGN_KEY *key, const uint8_t *in, size_t inlen, ui
 	return 1;
 }
 
-int sdf_sign_init(SDF_SIGN_CTX *ctx, const SDF_SIGN_KEY *key, const char *id, size_t idlen)
+int sdf_sign_init(SDF_SIGN_CTX *ctx, const SDF_PRIVATE_KEY *key, const char *id, size_t idlen)
 {
+	ECCrefPublicKey eccPublicKey;
+	SM2_Z256_POINT z256_point;
+
 	if (!ctx || !key) {
 		error_print();
 		return -1;
 	}
+	if (!key->session) {
+		error_print();
+		return -1;
+	}
+
+	if (SDF_ExportSignPublicKey_ECC(key->session, key->index, &eccPublicKey) != SDR_OK) {
+		error_print();
+		return -1;
+	}
+	if (ECCrefPublicKey_to_SM2_Z256_POINT(&eccPublicKey, &z256_point) != 1) {
+		error_print();
+		return -1;
+	}
 
 	sm3_init(&ctx->sm3_ctx);
 	if (id) {
@@ -857,7 +869,7 @@ int sdf_sign_init(SDF_SIGN_CTX *ctx, const SDF_SIGN_KEY *key, const char *id, si
 			error_print();
 			return -1;
 		}
-		sm2_compute_z(z, &key->public_key, id, idlen);
+		sm2_compute_z(z, &z256_point, id, idlen);
 		sm3_update(&ctx->sm3_ctx, z, sizeof(z));
 	}
 	ctx->saved_sm3_ctx = ctx->sm3_ctx;
@@ -906,7 +918,7 @@ int sdf_sign_reset(SDF_SIGN_CTX *ctx)
 	return 1;
 }
 
-int sdf_release_key(SDF_SIGN_KEY *key)
+int sdf_release_key(SDF_PRIVATE_KEY *key)
 {
 	if (SDF_ReleasePrivateKeyAccessRight(key->session, key->index) != SDR_OK) {
 		error_print();
diff --git a/tools/sdfdigest.c b/tools/sdfdigest.c
index 792c7be0..2933a8b7 100644
--- a/tools/sdfdigest.c
+++ b/tools/sdfdigest.c
@@ -256,7 +256,7 @@ int sdfdigest_main(int argc, char **argv)
 	}
 	ret = 0;
 end:
-	sdf_digest_cleanup(&ctx);
+	(void)sdf_digest_cleanup(&ctx);
 	if (pubkeyfp) fclose(pubkeyfp);
 	if (infile && infp) fclose(infp);
 	if (outfile && outfp) fclose(outfp);
diff --git a/tools/sdfsign.c b/tools/sdfsign.c
index c4ca4626..bc83b3f8 100644
--- a/tools/sdfsign.c
+++ b/tools/sdfsign.c
@@ -54,7 +54,7 @@ int sdfsign_main(int argc, char **argv)
 	uint8_t sig[SM2_MAX_SIGNATURE_SIZE];
 	size_t siglen;
 	SDF_DEVICE dev;
-	SDF_SIGN_KEY key;
+	SDF_PRIVATE_KEY key;
 	SDF_SIGN_CTX ctx;
 
 	argc--;
@@ -125,7 +125,7 @@ int sdfsign_main(int argc, char **argv)
 		goto end;
 	}
 
-	if (sdf_load_sign_key(&dev, &key, key_index, pass) != 1) {
+	if (sdf_load_private_key(&dev, &key, key_index, pass) != 1) {
 		(void)sdf_close_device(&dev);
 		fprintf(stderr,  "gmssl %s: load signing key #%d failure\n", prog, key_index);
 		goto end;