Re: Support for sslverify

Magnus Hagander wrote:
> On 15 mar 2009, at 17.00, Dave Page <dpage@pgadmin.org> wrote:
>
>> On Sun, Mar 15, 2009 at 2:51 PM, Magnus Hagander <magnus@hagander.net>
>> wrote:
>>> We've seen it here and Dave reported to me on IM that he has received
>>> further reports of people getting stuck by the new 8.4 SSL code that
>>> verifies server certificates by default.
>>>
>>> I think this will happen for example for everybody who has their pg on a
>>> debian server and their client elsewhere, for example, since debian
>>> enables a snakeoil SSL cert by default (which in itself is a pretty bad
>>> idea, but it's what they do)
>>>
>>>
>>> Should we provide an option to override this (connection option
>>> sslverify) in the connection dialog? And is it something we need to do
>>> for this version (yes, I know it's already in beta..)
>>
>> There's support for this in libpq aready? If so, then please go ahead
>> and fix pgAdmin :-)
>
> Yes, that was part of the original patch. You can set to verify all
> (never before, and default), verify ca (default before *if* the root
> cert was there) or no verification at all.
>
>
>> Note that the server connection diagloue is already pretty much at the
>> maximum height, so any changes there will probably need to include
>> splitting of the tabset.
>
> Crap. That something we want to do between betas?

OK, here's a patch that tries this. Since we're in beta, I definitely
want eyes on it before I commit :-)

//Magnus

Index: include/db/pgConn.h
===================================================================
--- include/db/pgConn.h    (revision 7708)
+++ include/db/pgConn.h    (working copy)
@@ -81,7 +81,7 @@
 class pgConn
 {
 public:
-    pgConn(const wxString& server = wxT(""), const wxString& database = wxT(""), const wxString& username = wxT(""),
constwxString& password = wxT(""), int port = 5432, int sslmode=0, OID oid=0); 
+    pgConn(const wxString& server = wxT(""), const wxString& database = wxT(""), const wxString& username = wxT(""),
constwxString& password = wxT(""), int port = 5432, int sslmode=0, int sslverify=0, OID oid=0); 
     ~pgConn();

     bool HasPrivilege(const wxString &objTyp, const wxString &objName, const wxString &priv);
@@ -117,7 +117,9 @@
     wxString GetTTY() const { return wxString(PQtty(conn), *conv); }
     wxString GetOptions() const { return wxString(PQoptions(conn), *conv); }
     int GetSslMode() const { return save_sslmode; }
+    int GetSslVerifyMode() const { return save_sslverifymode;}
     wxString GetSslModeName();
+    wxString GetSslVerifyModeName();
     int GetBackendPID() const { return PQbackendPID(conn); }
     int GetStatus() const;
     int GetLastResultStatus() const { return lastResultStatus; }
@@ -172,7 +174,7 @@
     wxString reservedNamespaces;

     wxString save_server, save_database, save_username, save_password;
-    int save_port, save_sslmode;
+    int save_port, save_sslmode, save_sslverifymode;
     OID save_oid;
 };

Index: include/utils/sysSettings.h
===================================================================
--- include/utils/sysSettings.h    (revision 7708)
+++ include/utils/sysSettings.h    (working copy)
@@ -48,6 +48,8 @@
     void SetLastPort(const int newval) { Write(wxT("LastPort"), newval); }
     int GetLastSSL() const { int i; Read(wxT("LastSSL"), &i, 0); return i; }
     void SetLastSSL(const int newval) { Write(wxT("LastSSL"), newval); }
+    int GetLastSSLverify() const { int i; Read(wxT("LastSSLverify"), &i, 0); return i; }
+    void SetLastSSLverify(const int newval) { Write(wxT("LastSSLverify"), newval); }

     // Helper paths
     wxString GetSlonyPath() const { wxString s; Read(wxT("SlonyPath"), &s, wxEmptyString); return s; }
Index: include/schema/pgServer.h
===================================================================
--- include/schema/pgServer.h    (revision 7708)
+++ include/schema/pgServer.h    (working copy)
@@ -38,7 +38,7 @@
 class pgServer : public pgObject
 {
 public:
-    pgServer(const wxString& newServer = wxT(""), const wxString& newDescription = wxT(""), const wxString&
newDatabase= wxT(""), const wxString& newUsername = wxT(""), int newPort = 5432, bool storePwd=false, bool
restore=true,int sslMode=0, const wxString &colour = wxEmptyString); 
+    pgServer(const wxString& newServer = wxT(""), const wxString& newDescription = wxT(""), const wxString&
newDatabase= wxT(""), const wxString& newUsername = wxT(""), int newPort = 5432, bool storePwd=false, bool
restore=true,int sslMode=0, int sslVerifyMode=0, const wxString &colour = wxEmptyString); 
     ~pgServer();
     int GetIconId();

@@ -99,10 +99,12 @@
     wxString GetFullIdentifier();
     int GetPort() const { return port; }
     int GetSSL() const { return ssl; }
+    int GetSSLverify() const { return sslverify; }
     bool GetConnected() const { return connected; }
     void iSetDatabase(const wxString& newVal) { database = newVal; }
     void iSetPort(int newval) { port=newval; }
     void iSetSSL(int newval) { ssl=newval; }
+    void iSetSSLverify(int newval) { sslverify=newval; }
     void iSetUsername(const wxString& newVal) { username = newVal; }
     void iSetPassword(const wxString& newVal) { password = newVal; }
     void iSetStorePwd(const bool b) { storePwd = b; }
@@ -146,7 +148,7 @@
     wxString database, username, password, ver, error;
     wxString lastDatabase, lastSchema, description, serviceId, discoveryId;
     wxDateTime upSince;
-    int port, ssl;
+    int port, ssl, sslverify;
     bool storePwd, restore, discovered, createPrivilege, superUser, createRole;
     OID lastSystemOID;
     OID dbOid;
Index: frm/frmMain.cpp
===================================================================
--- frm/frmMain.cpp    (revision 7708)
+++ frm/frmMain.cpp    (working copy)
@@ -1049,6 +1049,7 @@
             settings->Write(key + wxT("DbRestriction"), server->GetDbRestriction());
             settings->Write(key + wxT("Colour"), server->GetColour());
             settings->Write(key + wxT("SSL"), server->GetSSL());
+            settings->Write(key + wxT("SSLverify"), server->GetSSLverify());

             pgCollection *coll=browser->FindCollection(databaseFactory, server->GetId());
             if (coll)
Index: db/pgConn.cpp
===================================================================
--- db/pgConn.cpp    (revision 7708)
+++ db/pgConn.cpp    (working copy)
@@ -48,7 +48,7 @@
     ((pgConn*)arg)->Notice(message);
 }

-pgConn::pgConn(const wxString& server, const wxString& database, const wxString& username, const wxString& password,
intport, int sslmode, OID oid) 
+pgConn::pgConn(const wxString& server, const wxString& database, const wxString& username, const wxString& password,
intport, int sslmode, int sslverifymode, OID oid) 
 {
     wxString msg, hostip, hostname;

@@ -58,6 +58,7 @@
     save_password = password;
     save_port = port;
     save_sslmode = sslmode;
+    save_sslverifymode = sslverifymode;
     save_oid = oid;

     memset(features, 0, sizeof(features));
@@ -160,6 +161,15 @@
             case 2: connstr.Append(wxT(" requiressl=0"));   break;
         }
     }
+    if (libpqVersion >= 8.4)
+    {
+        switch (sslverifymode)
+        {
+            case 1: connstr.Append(wxT(" sslverify=cn"));   break;
+            case 2: connstr.Append(wxT(" sslverify=cert")); break;
+            case 3: connstr.Append(wxT(" sslverify=none")); break;
+        }
+    }
     connstr.Trim(false);

     // Open the connection
@@ -252,7 +262,7 @@

 pgConn *pgConn::Duplicate()
 {
-    return new pgConn(wxString(save_server), wxString(save_database), wxString(save_username),
wxString(save_password),save_port, save_sslmode, save_oid); 
+    return new pgConn(wxString(save_server), wxString(save_database), wxString(save_username),
wxString(save_password),save_port, save_sslmode, save_sslverifymode, save_oid); 
 }

 // Return the SSL mode name
@@ -262,22 +272,33 @@
     {
         case 1:
             return wxT("require");
-            break;
         case 2:
             return wxT("prefer");
-            break;
         case 3:
             return wxT("allow");
-            break;
         case 4:
             return wxT("disable");
-            break;
         default:
             return wxT("prefer");
-            break;
     }
 }

+// Return the SSL verify mode name
+wxString pgConn::GetSslVerifyModeName()
+{
+    switch (save_sslverifymode)
+    {
+        case 1:
+            return wxT("cn");
+        case 2:
+            return wxT("cert");
+        case 3:
+            return wxT("none");
+        default:
+            return wxT("cn");
+    }
+}
+
 bool pgConn::GetIsEdb()
 {
     // to retrieve edb flag
@@ -481,9 +502,14 @@
         {
             if (!strcmp(co->keyword, "sslmode"))
             {
-                libpqVersion=7.4;
-                break;
+                if (libpqVersion < 7.4)
+                    libpqVersion=7.4;
             }
+            if (!strcmp(co->keyword, "sslverify"))
+            {
+                if (libpqVersion < 8.4)
+                    libpqVersion=8.4;
+            }
             co++;
         }
         PQconninfoFree(cio);
Index: schema/pgServer.cpp
===================================================================
--- schema/pgServer.cpp    (revision 7708)
+++ schema/pgServer.cpp    (working copy)
@@ -39,7 +39,7 @@

 #define DEFAULT_PG_DATABASE wxT("postgres")

-pgServer::pgServer(const wxString& newName, const wxString& newDescription, const wxString& newDatabase, const
wxString&newUsername, int newPort, bool _storePwd, bool _restore, int _ssl, const wxString &_colour) 
+pgServer::pgServer(const wxString& newName, const wxString& newDescription, const wxString& newDatabase, const
wxString&newUsername, int newPort, bool _storePwd, bool _restore, int _ssl, int _sslverify, const wxString &_colour) 
 : pgObject(serverFactory, newName)
 {
     description = newDescription;
@@ -47,6 +47,7 @@
     username = newUsername;
     port = newPort;
     ssl=_ssl;
+    sslverify=_sslverify;
     colour = _colour;
     serverIndex=0;

@@ -137,7 +138,7 @@
         dbName = GetDatabaseName();
         oid = dbOid;
     }
-    pgConn *conn=new pgConn(GetName(), dbName, username, password, port, ssl, oid);
+    pgConn *conn=new pgConn(GetName(), dbName, username, password, port, ssl, sslverify, oid);

     if (conn && conn->GetStatus() != PGCONN_OK)
     {
@@ -614,21 +615,21 @@

         if (database.IsEmpty())
         {
-            conn = new pgConn(GetName(), DEFAULT_PG_DATABASE, username, password, port, ssl);
+            conn = new pgConn(GetName(), DEFAULT_PG_DATABASE, username, password, port, ssl, sslverify);
             if (conn->GetStatus() == PGCONN_OK)
                 database=DEFAULT_PG_DATABASE;
             else if (conn->GetStatus() == PGCONN_BAD && conn->GetLastError().Find(
                                 wxT("database \"") DEFAULT_PG_DATABASE wxT("\" does not exist")) >= 0)
             {
                 delete conn;
-                conn = new pgConn(GetName(), wxT("template1"), username, password, port, ssl);
+                conn = new pgConn(GetName(), wxT("template1"), username, password, port, ssl, sslverify);
                 if (conn && conn->GetStatus() == PGCONN_OK)
                     database=wxT("template1");
             }
         }
         else
         {
-            conn = new pgConn(GetName(), database, username, password, port, ssl);
+            conn = new pgConn(GetName(), database, username, password, port, ssl, sslverify);
             if (!conn)
             {
                 form->EndMsg(false);
@@ -933,6 +934,17 @@
                     }
                     properties->AppendItem(_("SSL Mode"), sslMode);
                 }
+                if (sslverify > 0)
+                {
+                    wxString sslVerifyMode;
+                    switch (sslverify)
+                    {
+                        case 1: sslVerifyMode = _("Full verification"); break;
+                        case 2: sslVerifyMode = _("Certificate only"); break;
+                        case 3: sslVerifyMode = _("No verification"); break;
+                    }
+                    properties->AppendItem(_("SSL Verify Mode"), sslVerifyMode);
+                }
             }
 #endif
         }
@@ -1079,7 +1091,7 @@
 {
     long numServers=settings->Read(wxT("Servers/Count"), 0L);

-    long loop, port, ssl=0;
+    long loop, port, ssl=0, sslverify=0;
     wxString key, servername, description, database, username, lastDatabase, lastSchema, storePwd, restore, serviceID,
discoveryID,dbRestriction, colour; 
     pgServer *server=0;

@@ -1118,10 +1130,11 @@
         // SSL mode
 #ifdef SSL
         settings->Read(key + wxT("SSL"), &ssl, 0);
+        settings->Read(key + wxT("SSLverify"), &sslverify, 0);
 #endif

         // Add the Server node
-        server = new pgServer(servername, description, database, username, port, StrToBool(storePwd),
StrToBool(restore),ssl); 
+        server = new pgServer(servername, description, database, username, port, StrToBool(storePwd),
StrToBool(restore),ssl, sslverify); 
         server->iSetLastDatabase(lastDatabase);
         server->iSetLastSchema(lastSchema);
         server->iSetServiceID(serviceID);
Index: dlg/dlgServer.cpp
===================================================================
--- dlg/dlgServer.cpp    (revision 7708)
+++ dlg/dlgServer.cpp    (working copy)
@@ -29,6 +29,7 @@
 #define cbDatabase      CTRL_COMBOBOX("cbDatabase")
 #define txtPort         CTRL_TEXT("txtPort")
 #define cbSSL           CTRL_COMBOBOX("cbSSL")
+#define cbSSLverify     CTRL_COMBOBOX("cbSSLverify")
 #define txtUsername     CTRL_TEXT("txtUsername")
 #define stTryConnect    CTRL_STATIC("stTryConnect")
 #define chkTryConnect   CTRL_CHECKBOX("chkTryConnect")
@@ -53,6 +54,7 @@
     EVT_TEXT(XRCID("txtUsername"),                  dlgProperty::OnChange)
     EVT_TEXT(XRCID("txtDbRestriction"),             dlgServer::OnChangeRestr)
     EVT_COMBOBOX(XRCID("cbSSL"),                    dlgProperty::OnChange)
+    EVT_COMBOBOX(XRCID("cbSSLverify"),              dlgProperty::OnChange)
     EVT_CHECKBOX(XRCID("chkStorePwd"),              dlgProperty::OnChange)
     EVT_CHECKBOX(XRCID("chkRestore"),               dlgProperty::OnChange)
     EVT_CHECKBOX(XRCID("chkTryConnect"),            dlgServer::OnChangeTryConnect)
@@ -85,6 +87,8 @@
     txtPort->SetValue(NumToStr((long)settings->GetLastPort()));
     if (!cbSSL->IsEmpty())
         cbSSL->SetSelection(settings->GetLastSSL());
+    if (!cbSSLverify->IsEmpty())
+        cbSSLverify->SetSelection(settings->GetLastSSLverify());
     txtUsername->SetValue(settings->GetLastUsername());

     chkTryConnect->SetValue(true);
@@ -105,6 +109,7 @@
         settings->SetLastDatabase(cbDatabase->GetValue());
         settings->SetLastPort(StrToLong(txtPort->GetValue()));
         settings->SetLastSSL(cbSSL->GetCurrentSelection());
+        settings->SetLastSSLverify(cbSSLverify->GetCurrentSelection());
         settings->SetLastUsername(txtUsername->GetValue());
     }
 }
@@ -142,6 +147,7 @@
         }
         server->iSetPort(StrToLong(txtPort->GetValue()));
         server->iSetSSL(cbSSL->GetCurrentSelection());
+        server->iSetSSLverify(cbSSLverify->GetCurrentSelection());
         server->iSetDatabase(cbDatabase->GetValue());
         server->iSetUsername(txtUsername->GetValue());
         server->iSetStorePwd(chkStorePwd->GetValue());
@@ -215,6 +221,7 @@
 int dlgServer::Go(bool modal)
 {
     cbSSL->Append(wxT(" "));
+    cbSSLverify->Append(wxT(" "));

 #ifdef SSL
     cbSSL->Append(_("require"));
@@ -225,6 +232,13 @@
         cbSSL->Append(_("allow"));
         cbSSL->Append(_("disable"));
     }
+
+    if (pgConn::GetLibpqVersion() >= 8.4)
+    {
+        cbSSLverify->Append(_("Full verification"));
+        cbSSLverify->Append(_("Certificate only"));
+        cbSSLverify->Append(_("No verification"));
+    }
 #endif

     if (server)
@@ -235,6 +249,7 @@
         txtService->SetValue(server->GetServiceID());
         txtPort->SetValue(NumToStr((long)server->GetPort()));
         cbSSL->SetSelection(server->GetSSL());
+        cbSSLverify->SetSelection(server->GetSSLverify());
         cbDatabase->SetValue(server->GetDatabaseName());
         txtUsername->SetValue(server->GetUsername());
         chkStorePwd->SetValue(server->GetStorePwd());
@@ -250,6 +265,7 @@
             cbDatabase->Disable();
             txtPort->Disable();
             cbSSL->Disable();
+            cbSSLverify->Disable();
             txtUsername->Disable();
             chkStorePwd->Disable();
         }
@@ -282,7 +298,8 @@
     pgObject *obj=new pgServer(GetName(), txtDescription->GetValue(), cbDatabase->GetValue(),
         txtUsername->GetValue(), StrToLong(txtPort->GetValue()),
         chkTryConnect->GetValue() && chkStorePwd->GetValue(),
-        chkRestore->GetValue(), cbSSL->GetCurrentSelection(), txtColour->GetValue());
+        chkRestore->GetValue(), cbSSL->GetCurrentSelection(),
+        cbSSLverify->GetCurrentSelection(), txtColour->GetValue());

     return obj;
 }
@@ -320,6 +337,7 @@
                || cbDatabase->GetValue() != server->GetDatabaseName()
                || txtUsername->GetValue() != server->GetUsername()
                || cbSSL->GetCurrentSelection() != server->GetSSL()
+               || cbSSLverify->GetCurrentSelection() != server->GetSSLverify()
                || chkStorePwd->GetValue() != server->GetStorePwd()
                || chkRestore->GetValue() != server->GetRestore()
                || txtDbRestriction->GetValue() != server->GetDbRestriction()
@@ -332,6 +350,7 @@
 #else
     bool isPipe = (name.IsEmpty() || name.StartsWith(wxT("/")));
     cbSSL->Enable(!isPipe);
+    cbSSLverify->Enable(!isPipe);
 #endif
     CheckValid(enable, !txtDescription->GetValue().IsEmpty(), _("Please specify description."));
     CheckValid(enable, StrToLong(txtPort->GetValue()) > 0, _("Please specify port."));
Index: ui/dlgServer.xrc
===================================================================
--- ui/dlgServer.xrc    (revision 7708)
+++ ui/dlgServer.xrc    (working copy)
@@ -66,12 +66,28 @@
                   <border>4</border>
                 </object>
                 <object class="sizeritem">
-                  <object class="wxComboBox" name="cbSSL">
+                 <object class="wxGridSizer">
+                  <cols>2</cols>
+                  <rows>2</rows>
+                  <growablecols>1</growablecols>
+                  <object class="sizeritem">
+                   <object class="wxComboBox" name="cbSSL">
                     <style>wxCB_READONLY|wxCB_DROPDOWN</style>
                     <content/>
+                   </object>
+                   <flag>wxEXPAND|wxALIGN_CENTRE_VERTICAL|wxTOP|wxLEFT|wxRIGHT</flag>
+                   <border>4</border>
                   </object>
-                  <flag>wxEXPAND|wxALIGN_CENTRE_VERTICAL|wxTOP|wxLEFT|wxRIGHT</flag>
-                  <border>4</border>
+                  <object class="sizeritem">
+                   <object class="wxComboBox" name="cbSSLverify">
+                    <style>wxCB_READONLY|wxCB_DROPDOWN</style>
+                    <content/>
+                   </object>
+                   <flag>wxEXPAND|wxALIGN_CENTRE_VERTICAL|wxTOP|wxLEFT|wxRIGHT</flag>
+                   <border>4</border>
+                  </object>
+                 </object>
+                 <flag>wxGROW</flag>
                 </object>
                 <object class="sizeritem">
                   <object class="wxStaticText" name="stDatabase">
@@ -258,4 +274,4 @@
       </object>
     </object>
   </object>
-</resource>
\ No newline at end of file
+</resource>