search

termux / termux-api-package

Termux package containing scripts to call functionality in Termux:API.
MIT License
1.02k stars 318 forks source link

Allow signing raw messages in keystore #106

Closed aeolwyr closed 4 years ago

aeolwyr commented 4 years ago

This change adds support for a new digest, NONE.

This was required as tergent 1.0 is now a cryptoki library. When ssh calls a cryptoki library it handles hashing internally, therefore the keys need to support this.

The list of possible values can be found in android.security.keystore.KeyProperties.

An open question here is the future of this flag (and similarly the purposes flag). Ideally, a change in this package should not be needed every time a new digest or purpose is needed. However, even allowing the users to edit these using cli parameters does not solve these issues:

It can be argued we should default to "allow all" in KeystoreAPI which should make the lifes of the users easier, but I am not sure about the security implications of that. Also that kind of implementation will need reflection as Android does not provide a way to get all possible key properties.

In any case such a change will require updates in the termux-api package too, and I wanted to keep this change as small as possible for now.

xalexalex commented 4 years ago

Hey, unfortunately I'm not knowledgeable on the subject so even though the change looks harmless, my LGTM would be quite worthless. I hope someone else can look into this.

Regarding your other question, I agree with keeping the change as small as possible now, especially if supporting new digests or purposes could be seen as premature optimization and/or may introduce security threats.

Grimler91 commented 4 years ago

Ideally, a change in this package should not be needed every time a new digest or purpose is needed.

I do not think there is a better way. An alternative could be to include an api script in the tergent package instead, but it would then be less obvious that termux-api is needed for it to work, and the script would basically be a copy of termux-keystore anyways.

Merging this now. We can wait with releasing it to termux-packages until the accompanying termux-api change is merged.

aeolwyr commented 4 years ago

Thank you for taking a look!

We can wait with releasing it to termux-packages until the accompanying termux-api change is merged.

I see that I caused some confusion here. This particular PR does not need any changes in termux-api app, it is already capable for accepting any purpose or digest.

I do not think there is a better way. An alternative could be to include an api script in the tergent package instead, but it would then be less obvious that termux-api is needed for it to work, and the script would basically be a copy of termux-keystore anyways.

True, it doesn't really make sense to maintain two of the same script. And still, it wouldn't solve the main problem: give access to all parts of Android Keystore API.

I guess in essence this script (and the corresponding code in termux-api) was supposed to be a passthrough to the Android APIs, but now I realize it is difficult to have a dynamic interface to a compiled language.

Anyway, this is probably a question for future. Thanks again for your time.