Nico, good day.

Thu, May 21, 2009 at 11:00:23AM +0200, Nico Golde wrote:
> * Eygene Ryabinkin <r...@codelabs.ru> [2009-05-21 10:24]:
> > Thu, May 21, 2009 at 09:50:12AM +0400, Eygene Ryabinkin wrote:
> > > Wed, May 20, 2009 at 05:39:08PM +0200, Mike Massonnet wrote:
> > > > Wow, nice! I didn't take time yet to investigate, thanks for a lot for
> > > > providing this patch. I will make a package for stable asap.
> > > 
> > > Erm, sorry, sent old patch variant that doesn't produce .Xauthority:
> > > 'quit' should be replaced with 'exit'.  Sorry, wasn't updated the
> > > patchfile.  Here is the proper one:
> > 
> > And found one more issue -- mcookie was weakened because I am blindly
> > substituted 'int r' for 'bool r'.  Fixed now.
> 

> Thanks very much, the patch looks good! While you're at it, mind to
> fix the insecure "random" hexstring generation as well?

Sure, did it already, tested and just wanted to send it out.

From 5beb217296e3074cadc5bcb3e40355f54ee705f0 Mon Sep 17 00:00:00 2001
From: Eygene Ryabinkin <r...@shadow.codelabs.ru>
Date: Thu, 21 May 2009 11:56:27 +0400
Subject: [PATCH] Create interface for random number generator and use it 
everywhere

Don't use rand()/srand() at all -- they are very weak.  Provide our
wrappers for random()/srandom() and make utility function that will
generate seed for srandom.

Rework MIT magic cookie generation: consume 4 bytes of input in one
pass -- random() should produce values that are usable for this purpose.

Signed-off-by: Eygene Ryabinkin <r...@shadow.codelabs.ru>
---
 app.cpp  |   49 ++++++++++++++++++++++++++-----------------------
 app.h    |    2 ++
 util.cpp |   37 +++++++++++++++++++++++++++++++++++++
 util.h   |    5 +++++
 4 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/app.cpp b/app.cpp
index 04caaa1..0ac8c3a 100644
--- a/app.cpp
+++ b/app.cpp
@@ -129,15 +129,18 @@ void User1Signal(int sig) {
 
 
 #ifdef USE_PAM
-App::App(int argc, char** argv):
-    pam(conv, static_cast<void*>(&LoginPanel)){
+App::App(int argc, char** argv)
+  : pam(conv, static_cast<void*>(&LoginPanel)),
 #else
-App::App(int argc, char** argv){
+App::App(int argc, char** argv)
+  :
 #endif
+    mcookiesize(32)            // Must be divisible by 4
+{
     int tmp;
     ServerPID = -1;
     testing = false;
-    mcookie = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+    mcookie = string(App::mcookiesize, 'a');
     daemonmode = false;
     force_nodaemon = false;
     firstlogin = true;
@@ -1128,13 +1131,13 @@ string App::findValidRandomTheme(const string& set)
         name = name.substr(0, name.length() - 1);
     }
 
-    srandom(getpid()+time(NULL));
+    Util::srandom(Util::makeseed());
 
     vector<string> themes;
     string themefile;
     Cfg::split(themes, name, ',');
     do {
-        int sel = random() % themes.size();
+        int sel = Util::random() % themes.size();
 
         name = Cfg::Trim(themes[sel]);
         themefile = string(THEMESDIR) +"/" + name + THEMESFILE;
@@ -1161,27 +1164,27 @@ void App::replaceVariables(string& input,
 }
 
 
+/*
+ * We rely on the fact that all bits generated by Util::random()
+ * are usable, so we are taking full words from its output.
+ */
 void App::CreateServerAuth() {
     /* create mit cookie */
-    int i, r;
-    int hexcount = 0;
-        string authfile;
-    string cmd;
+    uint16_t word;
+    uint8_t hi, lo;
+    int i;
+    string authfile;
     const char *digits = "0123456789abcdef";
-        srand( time(NULL) );
-    for ( i = 0; i < 31; i++ ) {
-        r = rand()%16;
-                mcookie[i] = digits[r];
-                if (r>9)
-                        hexcount++;
+    Util::srandom(Util::makeseed());
+    for (i = 0; i < App::mcookiesize; i+=4) {
+        word = Util::random() & 0xffff;
+        lo = word & 0xff;
+        hi = word >> 8;
+        mcookie[i] = digits[lo & 0x0f];
+        mcookie[i+1] = digits[lo >> 4];
+        mcookie[i+2] = digits[hi & 0x0f];
+        mcookie[i+3] = digits[hi >> 4];
     }
-        /* MIT-COOKIE: even occurrences of digits and hex digits */
-        if ((hexcount%2) == 0) {
-                r = rand()%10;
-        } else {
-                r = rand()%5+10;
-        }
-        mcookie[31] = digits[r];
     /* reinitialize auth file */
     authfile = cfg->getOption("authfile");
     remove(authfile.c_str());
diff --git a/app.h b/app.h
index 7b4bd10..9a44269 100644
--- a/app.h
+++ b/app.h
@@ -101,6 +101,8 @@ private:
     
     std::string themeName;
     std::string mcookie;
+
+    const int mcookiesize;
 };
 
 
diff --git a/util.cpp b/util.cpp
index 309ce4f..5ed972f 100644
--- a/util.cpp
+++ b/util.cpp
@@ -7,7 +7,13 @@
    (at your option) any later version.
 */
 
+#include <sys/types.h>
+
 #include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <unistd.h>
+
 #include "util.h"
 
 /*
@@ -30,3 +36,34 @@ bool Util::add_mcookie(const std::string &mcookie, const 
char *display,
        pclose(fp);
        return true;
 }
+
+/*
+ * Interface for random number generator.  Just now it uses ordinary
+ * random/srandom routines and serves as a wrapper for them.
+ */
+void Util::srandom(unsigned long seed)
+{
+       ::srandom(seed);
+}
+
+long Util::random(void)
+{
+       return ::random();
+}
+
+/*
+ * Makes seed for the srandom() using "random" values obtained from
+ * getpid(), time(NULL) and others.
+ */
+long Util::makeseed(void)
+{
+       struct timespec ts;
+       long pid = getpid();
+       long tm = time(NULL);
+
+       if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) {
+               ts.tv_sec = ts.tv_nsec = 0;
+       }
+
+       return pid + tm + (ts.tv_sec ^ ts.tv_nsec);
+}
diff --git a/util.h b/util.h
index 8bd52be..b8d2993 100644
--- a/util.h
+++ b/util.h
@@ -14,6 +14,11 @@
 namespace Util {
        bool add_mcookie(const std::string &mcookie, const char *display,
            const std::string &xauth_cmd, const std::string &authfile);
+
+       void srandom(unsigned long seed);
+       long random(void);
+
+       long makeseed(void);
 };
 
 #endif /* __UTIL_H__ */
-- 
1.6.3.1
-- 
rea



--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to