Re: SSH Tunneling Patch

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: SSH Tunneling Patch
Date: 2013-05-07 08:09:17
Message-ID: CANxoLDfGwEnrUFP0xVbwL_OpDeqMuoKW8JJkL1Vc57vi4E1yOw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi Dave

On Thu, May 2, 2013 at 10:14 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> Hi
>
> I committed the patch a little while back after testing successfully
> on Linux, Windows and Mac and adding some docs. Unfortunately, since
> then I've seen the following issues:
>
> - On OS X Tiger 10.4 (the EDB machine Tanaka), I got a compile failure:
>
> utils/sshTunnel.cpp:46: error: invalid conversion from 'const char*
> (*)(int, const void*, char*, size_t)' to 'const char* (*)(int, const
> void*, char*, socklen_t)'
> gcc -DHAVE_CONFIG_H -I. -I.. -I../pgadmin/include/libssh2
> -I../pgadmin/include -I/usr/local/pgsql-9.2/include
> -I/usr/local/pgsql-9.2/include/server -I/usr/local/pgsql-9.2/include
> -DPG_SSL -DHAVE_CONNINFO_PARSE
> -I/usr/local/lib/wx/include/mac-unicode-release-static-2.8
> -I/usr/local/include/wx-2.8 -D_FILE_OFFSET_BITS=64 -D_LARGE_FILES
> -D__WXMAC__ -DEMBED_XRC -arch i386 -arch ppc
> -I/usr/local/include/libxml2 -I/usr/local/include
> -I/usr/local/include/libxml2 -DHAVE_OPENSSL_CRYPTO -O2 -c -o
> libssh2/packet.o libssh2/packet.c
> utils/sshTunnel.cpp:46: error: invalid conversion from 'const char*
> (*)(int, const void*, char*, size_t)' to 'const char* (*)(int, const
> void*, char*, socklen_t)'
> lipo: can't figure out the architecture type of: /var/tmp//cceIHDrK.out
>
> Note that this was with a multi-arch build, configured with
> "./configure --enable-appbundle --with-pgsql=/usr/local/pgsql-9.2
> --with-arch-ppc --with-arch-i386 --disable-dependency-tracking". I
> also tried a build with "./configure --enable-appbundle
> --with-pgsql=/usr/local/pgsql-9.2 --with-arch-i386" and that failed
> similarly
>
> - The pgAdmin Jenkins build failed at the bootstrap step -
> http://developer.pgadmin.org:8080/job/pgAdmin%20master%20branch/69/console
> .
> This machine is a Debian Squeeze box. Installing Gettext (per a
> comment you made earlier in the development of this patch) fixed the
> problem, but I find it odd that that is required.
>

Ashesh has helped me to fix the issue of AC_LIB_HAVE_LINKFLAGS. With
this fix Gettext is not required. Attached is the patch file. Please review
it and if it looks good then can you please commit it.

>
> Remaining tidy-ups required during beta are:
>
> - "Authentication by identity file failed with error code -16" error
> cleanup.
>
> - Ability to browse for the identity file in hidden (dot) directories.
>
> - The alignment of the Identity File textbox is still a little wonky on
> Mac.
>
> Thanks!!
>
> On Thu, May 2, 2013 at 2:09 PM, Akshay Joshi
> <akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
> > Hi Dave
> >
> >
> > On Wed, May 1, 2013 at 8:59 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >>
> >> Well this is sooner than expected...
> >>
> >> The good news is, that today I cannot recreate the crash I saw
> >> previously in either debug or release builds on Mac. I'm guessing
> >> something must have been messed up in my environment previously, but
> >> it seems stable now. A couple of final review issues I have picked up
> >> however:
> >>
> >> - I had to manually add -lz to LDFLAGS to get the code to compile on
> >> Mac. The configure tests seem to correctly detect that it is needed,
> >> but it never gets added to the linker flags.
> >>
> >> - The new code that has been added to acinclude.m4 is a mess. I
> >> realise that has come from the libssh2 code though, so I don't expect
> >> we want to try to rewrite it, but I do think it should be moved into
> >> an external file where it can be maintained/updated standalone. Maybe
> >> acinclude-ssh2.m4 ?
> >>
> >> - The textbox used for the identity file name on the server dialogue
> >> is badly aligned on Mac.
> >>
> >> - PG_SSL needs to be defined in the VC++ project files.
> >>
> >> From what I can see, once these and the issues I raised in my previous
> >> email are resolved, it's good to commit. I'm planning on cutting an
> >> alpha to ship with PG 9.3b1 tomorrow afternoon my time, so if you can
> >> get me a usable patch by then, so much the better. I think we need the
> >> following for alpha:
> >>
> >> - Properties display
> >> - Localhost for the local tunnel end
> >> - -lz linker issue
> >> - PG_SSL definition
> >> - acinclude.m4 cleanup
> >
> >
> > I have finished all the above mentioned task. Attached is the updated
> > patch. I have tested it on Windows and Linux, it is working fine, not
> able
> > to test it on MAC due to some problem on my machine.
> >>
> >>
> >> The rest can be fixed for beta.
> >>
> >> Thanks!
> >>
> >>
> >> On Wed, May 1, 2013 at 3:46 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >> > Some feedback based on this version:
> >> >
> >> > - On the first connection attempt, if I get the SSH password wrong,
> >> > the entire Server dialogue is dismissed, losing all the connection
> >> > details I entered. I would expect the dialogue to be re-displayed so I
> >> > could correct the error and try again.
> >
> >
> > Fixed.
> >>
> >> >
> >> > - Incorrect ownership/permissions on the identity file result in a
> >> > mysterious "Authentication by identity file failed with error code
> >> > -16" error. I think this needs to be improved - if that error code
> >> > relates purely to being unable to read the file, then we can
> >> > special-case it.
> >> >
> >> > - When browsing for the identity file, you cannot browse to a "dot"
> >> > directory, e.g. I can't use /home/dpage/.ssh/id_rsa as my identity
> >> > file, without manually editing ~/.pgadmin3
> >> >
> >> > - We seem to read/write a setting called PublicKeyFile, but from what
> >> > I can see, it's always empty and it's not something we request from
> >> > the user. Is it needed?
> >
> >
> > When we use libgcrypt instead of libssl/libcrypto then we will have to
> > get public key file from the user, without getting public key file SSH
> > tunneling won't work. With current implementation control for the public
> key
> > file will be visible when we use libgcrypt instead of libssl/libcrypto.
> So
> > that it is needed.
> >
> >>
> >> >
> >> > - SSH Tunnel options are not displayed on the Properties list for the
> >> > server.
> >> >
> >> > - On a Linux VM on my laptop, SSH tunnelling fails:
> >> >
> >> > 2013-03-15 04:03:34 INFO : Attempting to create a connection
> object...
> >> > 2013-03-15 04:03:34 STATUS : Connecting to database...
> >> > 2013-03-15 04:03:35 INFO : getaddrinfo failed with error code: -2
> >> > 2013-03-15 04:03:35 STATUS : Connecting to database... (1.37 secs)
> >> > 2013-03-15 04:03:35 INFO : pgServer object didn't initialise because
> >> > the user aborted.
> >> > 2013-03-15 04:03:35 ERROR : SSH error: Unable to resolve host:
> >> > viper-centos6.ox.uk.enterprisedb.com
> >> >
> >> > I was connecting to a host that I shall call foo.enterprisedb.com,
> >> > using an SSH tunnel to bar.enterprisedb.com. The hostname shown in
> the
> >> > error message is the hostname my VM is configured with in
> >> > /etc/sysconfig/network, but it is *not* in either /etc/hosts or DNS. I
> >> > would expect it to be using "localhost" to connect to the local
> >> > machine. Hard-coding the hostname that way (around line 204 in
> >> > sshTunnel.cpp) seems to resolve the issue.
> >> >
> >> > So, overall, nothing seems to be serious wrong on Windows or Linux
> >> > from what I can see. Now to look at Mac...
> >> >
> >> > On Wed, May 1, 2013 at 2:56 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >> >> And this time, a patch containing the right bits...
> >> >>
> >> >> On Wed, May 1, 2013 at 2:19 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
> >> >>> Attached is an update to Akshay's SSH tunnelling patch. Aside from a
> >> >>> couple of minor tweaks to messages, this fixes the build on Windows
> >> >>> which bit-rotted horribly when I committed the new debugger code.
> >> >>>
> >> >>> At this stage, the patch seems to work nicely on Windows. I'm going
> to
> >> >>> test some more on Linux, and Akshay is working on a couple of issues
> >> >>> we've seen on Mac.
> >> >>>
> >> >>> --
> >> >>> Dave Page
> >> >>> Blog: http://pgsnake.blogspot.com
> >> >>> Twitter: @pgsnake
> >> >>>
> >> >>> EnterpriseDB UK: http://www.enterprisedb.com
> >> >>> The Enterprise PostgreSQL Company
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Dave Page
> >> >> Blog: http://pgsnake.blogspot.com
> >> >> Twitter: @pgsnake
> >> >>
> >> >> EnterpriseDB UK: http://www.enterprisedb.com
> >> >> The Enterprise PostgreSQL Company
> >> >
> >> >
> >> >
> >> > --
> >> > Dave Page
> >> > Blog: http://pgsnake.blogspot.com
> >> > Twitter: @pgsnake
> >> >
> >> > EnterpriseDB UK: http://www.enterprisedb.com
> >> > The Enterprise PostgreSQL Company
> >>
> >>
> >>
> >> --
> >> Dave Page
> >> Blog: http://pgsnake.blogspot.com
> >> Twitter: @pgsnake
> >>
> >> EnterpriseDB UK: http://www.enterprisedb.com
> >> The Enterprise PostgreSQL Company
> >
> >
> >
> >
> > --
> > Akshay Joshi
> > Senior Software Engineer
> > EnterpriseDB Corporation
> > The Enterprise PostgreSQL Company
> > Phone: +91 20-3058-9522
> > Mobile: +91 976-788-8246
>
>
>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

--
*Akshay Joshi
Senior Software Engineer
EnterpriseDB Corporation
The Enterprise PostgreSQL Company
Phone: +91 20-3058-9522
Mobile: +91 976-788-8246*

Attachment Content-Type Size
Fixed_AC_LIB_HAVE_LINKFLAGS.patch application/octet-stream 45.9 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dinesh Kumar 2013-05-07 08:14:40 Procedure "RENAME, SET SCHEMA" issues patch.
Previous Message Jasmin Dizdarevic 2013-05-07 05:53:00 Re: 2 patches