mkj / dropbear

Dropbear SSH
https://matt.ucc.asn.au/dropbear/dropbear.html
Other
1.69k stars 402 forks source link

Support for password on private keys #209

Open HansH111 opened 1 year ago

HansH111 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

HansH111 commented 1 year ago

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 commented 1 year ago

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

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

HansH111 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 12 months ago

https://github.com/libtom/libtomcrypt/pull/587 adds support for OpenSSH encrypted keys, could be used once merged.

HansH111 commented 8 months ago

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...

hovercats commented 1 day ago

@mkj, libtom/libtomcrypt#587 was merged recently. in case you havent already noticed.