Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for password on private keys #209

Open
HansH111 opened this issue Dec 31, 2022 · 9 comments
Open

Support for password on private keys #209

HansH111 opened this issue Dec 31, 2022 · 9 comments

Comments

@HansH111
Copy link
Contributor

Feature request
I noticed that I cannot put a password on a private key.
Would be nice in the future if that was somehow possible.

Have a nice 2023 everyone !

@zertawz
Copy link

zertawz commented Apr 17, 2023

Yeah I just saw that, that's sad.
I would be more secure.

For an OpenWRT used as a bastion host for example It would be an absolute banger.
Shout-out for your comment !
Have a nice day

@VA1DER
Copy link

VA1DER commented Aug 13, 2023

This shouldn't be too difficult to implement, and would be a major improvement.

@HansH111
Copy link
Contributor Author

The dbclient should have a cmdline option to pass an env.var which contains the password for decrypting the key.
I have something very simple which just encode and decode the key using xor

in cli-runopts.c:

"-E <envnm|passwd>          Password or env.varnm containing the password of the keys\n"

                                case 'E':
                                        next = &cli_opts.enckeynm;
                                        break;

in function loadidentityfile
        if (cli_opts.enckeynm != NULL) {
                stat=readenckey(cli_opts.enckeynm, filename, key, &keytype);
        } else {
                stat=readhostkey(filename, key, &keytype);
        }

in common-runopts.c

static unsigned char  *encbasestr="wMsC7.LgeFhCV0e@5SlTjoHw`p#Flv*,CpQ7xvEBlZr34n15$=~QHtPwkF;u^fS:Q00=pr.vN|gjtnR";
static unsigned char  enckeyid[200]="";
static int            enckeylen=0;

void ENC_encode(int pos, unsigned char *key, int keylen, unsigned char *buf, int buflen)
{
  unsigned char *kp;
  kp=key;
  while (buflen > 0) {
       if (pos>=keylen) {
              pos%=keylen;
              kp=key;
       }
       *buf ^= *kp++;
       buf++;
       buflen--;
  }
}
void ENC_keyencode(const char *keynm, unsigned char *buf, int buflen)
{
  if (*enckeyid==0) {
     unsigned char *key;
     strcpy(enckeyid,encbasestr);
     enckeylen=strlen(enckeyid);
     if (keynm && *keynm) {
        key=getenv(keynm);
        if (key && *key!=0) {  /* use envvar content as password */
           ENC_encode(0,key,strlen(key),enckeyid,enckeylen);
        } else {  /* use keynm as password */
           ENC_encode(0,(unsigned char *)keynm,strlen(keynm),enckeyid,enckeylen);
        }
     }
  }
  ENC_encode(0,enckeyid,enckeylen,buf,buflen);
}
/* returns success or failure, and the keytype in *type. If we want
 * to restrict the type, type can contain a type to return */
int readenckey(const char *keynm, const char * filename, sign_key * hostkey, enum signkey_type *type) {

        int ret = DROPBEAR_FAILURE;
        buffer *buf;

        buf = buf_new(MAX_PRIVKEY_SIZE);

        if (buf_readfile(buf, filename) == DROPBEAR_FAILURE) {
                goto out;
        }
        buf_setpos(buf, 0);
        ENC_keyencode(keynm, buf->data, buf->len);

        addrandom(buf_getptr(buf, buf->len), buf->len);
        if (buf_get_priv_key(buf, hostkey, type) == DROPBEAR_FAILURE) {
                goto out;
        }

        ret = DROPBEAR_SUCCESS;
out:
        buf_burn_free(buf);
        return ret;
}

I also changed some .h files and did some initialisation, but I think that is obvious...

But probably the libtomcrypt contains better ways to encrypt and decrypt the keys and that should be used.
I also think you want to recognize if the dropbear keys are encrypted or not.
It greatly depends on how dropbearkey.c is changed to integrate the password protection.

For my quick hack I used a standalone program to encode the dropbear keys with a password using the same logic.

@M95D
Copy link
Contributor

M95D commented Aug 21, 2023

I'm sorry to disagree. My arguments are these:

  • "OpenWRT used as a bastion host" doesn't need ssh keys! Open a ssh tunnel and use that to connect directly from the remote client. It's a lot simpler and faster.
  • OpenWrt is mostly for systems with very limited storage (and/or memory), not laptops that can be stolen. The posibility of an evil maid to steal the keys from a router or an IP-cam is very remote. Devices with sd/usb/emmc, a screen and a keyboard can use the OpenSSH client.
  • Features like this, while they seem useful, turn dropbear into a full OpenSSH. OpenSSH is available for probably all linuxes, why not use it? Let's not bloat the very few lean programs remaining!

If the author chooses to add this functionality, I beg of you to at least make it optional/removable.
Thank you for understanding!

@HansH111
Copy link
Contributor Author

Not everyone uses openwrt, it is about if you use a dbclient you are able to put a password on the ssh keys for security.
Seems logical to make this optional, just like the other options you have.

@VA1DER
Copy link

VA1DER commented Aug 23, 2023

This isn't a request for a new crypto algorithm which will require a lot of code. This request can, with a certainty, use crypto already imported into every dropbear build (every build needs AES, for example). So we're looking at an implementation cost of a couple hundred bytes if done with an eye to economy. It will probably take more space for the text used in user messages than for the code.

As such, this seems to be a pretty low impact addition.

Incidentally, looking through the 18k or so of text build into the binary, I see many places where some clever reuse could save quite a bit of space.

@mkj
Copy link
Owner

mkj commented Sep 6, 2023

I'd be inclined to use a proper password hashing method for encrypted keys. Otherwise most passwords will be brute-forceable by GPU unless they're a diceware passphrase or similar. A short password is false security.

If implementing good password hashing then it probably makes sense to just implement OpenSSH's newer private key format which uses bcrypt.
https://cvsweb.openbsd.org/src/usr.bin/ssh/PROTOCOL.key?annotate=HEAD

@mkj mkj mentioned this issue Sep 6, 2023
@mkj
Copy link
Owner

mkj commented Oct 10, 2023

libtom/libtomcrypt#587 adds support for OpenSSH encrypted keys, could be used once merged.

@HansH111
Copy link
Contributor Author

Attached tar file which contains patch files for dbclient and dropbearkey which supports encrypted keys.
Uses dbclient -E or dropbearkey -N for passphrase or it can be an env varnm which contains the passphrase
Functions from libtomcrypt used are : rijndael_ecb_encrypt/decrypt and sha256 functions.

patchfiles.tgz

Maybe this helps...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants