little simpler ssh code

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

little simpler ssh code

Ted Unangst-6
no change, but makes the code a little shorter.


Index: clientloop.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.287
diff -u -p -r1.287 clientloop.c
--- clientloop.c 12 Sep 2016 01:22:38 -0000 1.287
+++ clientloop.c 17 Sep 2016 01:16:46 -0000
@@ -303,7 +303,7 @@ client_x11_get_proto(const char *display
  char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
  static char proto[512], data[512];
  FILE *f;
- int got_data = 0, generated = 0, do_unlink = 0, i, r;
+ int got_data = 0, generated = 0, do_unlink = 0, r;
  struct stat st;
  u_int now, x11_timeout_real;
 
@@ -430,17 +430,16 @@ client_x11_get_proto(const char *display
  * for the local connection.
  */
  if (!got_data) {
- u_int32_t rnd = 0;
+ u_int8_t rnd[16];
+ u_int i;
 
  logit("Warning: No xauth data; "
     "using fake authentication data for X11 forwarding.");
  strlcpy(proto, SSH_X11_PROTO, sizeof proto);
- for (i = 0; i < 16; i++) {
- if (i % 4 == 0)
- rnd = arc4random();
+ arc4random_buf(rnd, sizeof(rnd));
+ for (i = 0; i < sizeof(rnd); i++) {
  snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
-    rnd & 0xff);
- rnd >>= 8;
+    rnd[i]);
  }
  }
 

Reply | Threaded
Open this post in threaded view
|

Re: little simpler ssh code

Martin Natano
On Fri, Sep 16, 2016 at 09:19:40PM -0400, Ted Unangst wrote:
> no change, but makes the code a little shorter.

Reads fine to me. arc4random() is already used in other places in ssh,
so this shouldn't be an issue for portable.


>
>
> Index: clientloop.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 clientloop.c
> --- clientloop.c 12 Sep 2016 01:22:38 -0000 1.287
> +++ clientloop.c 17 Sep 2016 01:16:46 -0000
> @@ -303,7 +303,7 @@ client_x11_get_proto(const char *display
>   char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
>   static char proto[512], data[512];
>   FILE *f;
> - int got_data = 0, generated = 0, do_unlink = 0, i, r;
> + int got_data = 0, generated = 0, do_unlink = 0, r;
>   struct stat st;
>   u_int now, x11_timeout_real;
>  
> @@ -430,17 +430,16 @@ client_x11_get_proto(const char *display
>   * for the local connection.
>   */
>   if (!got_data) {
> - u_int32_t rnd = 0;
> + u_int8_t rnd[16];
> + u_int i;
>  
>   logit("Warning: No xauth data; "
>      "using fake authentication data for X11 forwarding.");
>   strlcpy(proto, SSH_X11_PROTO, sizeof proto);
> - for (i = 0; i < 16; i++) {
> - if (i % 4 == 0)
> - rnd = arc4random();
> + arc4random_buf(rnd, sizeof(rnd));
> + for (i = 0; i < sizeof(rnd); i++) {
>   snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
> -    rnd & 0xff);
> - rnd >>= 8;
> +    rnd[i]);
>   }
>   }
>  
>

Reply | Threaded
Open this post in threaded view
|

Re: little simpler ssh code

Ted Unangst-6
In reply to this post by Ted Unangst-6
Ted Unangst wrote:
> no change, but makes the code a little shorter.

while here, another similar spot.


Index: clientloop.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/clientloop.c,v
retrieving revision 1.287
diff -u -p -r1.287 clientloop.c
--- clientloop.c 12 Sep 2016 01:22:38 -0000 1.287
+++ clientloop.c 17 Sep 2016 01:16:46 -0000
@@ -303,7 +303,7 @@ client_x11_get_proto(const char *display
  char xauthfile[PATH_MAX], xauthdir[PATH_MAX];
  static char proto[512], data[512];
  FILE *f;
- int got_data = 0, generated = 0, do_unlink = 0, i, r;
+ int got_data = 0, generated = 0, do_unlink = 0, r;
  struct stat st;
  u_int now, x11_timeout_real;
 
@@ -430,17 +430,16 @@ client_x11_get_proto(const char *display
  * for the local connection.
  */
  if (!got_data) {
- u_int32_t rnd = 0;
+ u_int8_t rnd[16];
+ u_int i;
 
  logit("Warning: No xauth data; "
     "using fake authentication data for X11 forwarding.");
  strlcpy(proto, SSH_X11_PROTO, sizeof proto);
- for (i = 0; i < 16; i++) {
- if (i % 4 == 0)
- rnd = arc4random();
+ arc4random_buf(rnd, sizeof(rnd));
+ for (i = 0; i < sizeof(rnd); i++) {
  snprintf(data + 2 * i, sizeof data - 2 * i, "%02x",
-    rnd & 0xff);
- rnd >>= 8;
+    rnd[i]);
  }
  }
 
Index: hostfile.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/hostfile.c,v
retrieving revision 1.66
diff -u -p -r1.66 hostfile.c
--- hostfile.c 4 May 2015 06:10:48 -0000 1.66
+++ hostfile.c 17 Sep 2016 03:15:29 -0000
@@ -120,14 +120,13 @@ host_hash(const char *host, const char *
  u_char salt[256], result[256];
  char uu_salt[512], uu_result[512];
  static char encoded[1024];
- u_int i, len;
+ u_int len;
 
  len = ssh_digest_bytes(SSH_DIGEST_SHA1);
 
  if (name_from_hostfile == NULL) {
  /* Create new salt */
- for (i = 0; i < len; i++)
- salt[i] = arc4random();
+ arc4random_buf(salt, len);
  } else {
  /* Extract salt from known host entry */
  if (extract_salt(name_from_hostfile, src_len, salt,

Reply | Threaded
Open this post in threaded view
|

Re: little simpler ssh code

Martin Natano
Two more loops that can be converted to arc4random_buf(). Ok?

natano


Index: channels.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/channels.c,v
retrieving revision 1.352
diff -u -p -r1.352 channels.c
--- channels.c 12 Sep 2016 01:22:38 -0000 1.352
+++ channels.c 18 Sep 2016 19:04:30 -0000
@@ -4148,7 +4148,6 @@ x11_request_forwarding_with_spoofing(int
  char *new_data;
  int screen_number;
  const char *cp;
- u_int32_t rnd = 0;
 
  if (x11_saved_display == NULL)
  x11_saved_display = xstrdup(disp);
@@ -4175,15 +4174,12 @@ x11_request_forwarding_with_spoofing(int
  */
  x11_saved_data = xmalloc(data_len);
  x11_fake_data = xmalloc(data_len);
+ arc4random_buf(x11_fake_data, data_len);
  for (i = 0; i < data_len; i++) {
  if (sscanf(data + 2 * i, "%2x", &value) != 1)
  fatal("x11_request_forwarding: bad "
     "authentication data: %.100s", data);
- if (i % 4 == 0)
- rnd = arc4random();
  x11_saved_data[i] = value;
- x11_fake_data[i] = rnd & 0xff;
- rnd >>= 8;
  }
  x11_saved_data_len = data_len;
  x11_fake_data_len = data_len;
Index: sshconnect1.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sshconnect1.c,v
retrieving revision 1.78
diff -u -p -r1.78 sshconnect1.c
--- sshconnect1.c 15 Nov 2015 22:26:49 -0000 1.78
+++ sshconnect1.c 18 Sep 2016 19:04:30 -0000
@@ -504,7 +504,6 @@ ssh_kex(char *host, struct sockaddr *hos
  u_char cookie[8];
  u_int supported_ciphers;
  u_int server_flags, client_flags;
- u_int32_t rnd = 0;
 
  debug("Waiting for server public key.");
 
@@ -563,12 +562,7 @@ ssh_kex(char *host, struct sockaddr *hos
  * random number, interpreted as a 32-byte key, with the least
  * significant 8 bits being the first byte of the key.
  */
- for (i = 0; i < 32; i++) {
- if (i % 4 == 0)
- rnd = arc4random();
- session_key[i] = rnd & 0xff;
- rnd >>= 8;
- }
+ arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
 
  /*
  * According to the protocol spec, the first byte of the session key

Reply | Threaded
Open this post in threaded view
|

Re: little simpler ssh code

Darren Tucker
On Mon, Sep 19, 2016 at 5:41 AM, Martin Natano <[hidden email]> wrote:

> Two more loops that can be converted to arc4random_buf(). Ok?
> [...]
> +               arc4random_buf(x11_fake_data, data_len);
>                 for (i = 0; i < data_len; i++) {
>

I'd put that below the for loop so it matches the order of the comment
above ("Extract real authentication data and generate fake data of the same
length.").  Or better yet, change the comment and group all of the
operations on x11_fake_data and x11_fake_data_len together.

[...]

> +       arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
>

sizeof(session_key) instead please.

with those, ok dtucker@

--
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.