Re: [PATCH 5/4] wincred: port to generic credential helper (UNTESTED)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 30, 2012 at 8:27 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote:
> On Mon, Aug 27, 2012 at 12:04 AM, Philipp A. Hartmann <pah@xxxxx> wrote:
>> From: "Philipp A. Hartmann" <pah@xxxxx>
>>
>> This patch is an experiment to port the wincred helper
>> to the generic implementation.  As of know, it is
>> completely untested.
>>
>> In addition to porting the helper to the generic API,
>> this patch clears up all passwords from memory, which
>> reduces the total amount to saved lines.
>>
>> Signed-off-by: Philipp A. Hartmann <pah@xxxxx>
>> ---
>>
>> The porting was fairly easy, but due to the lack of a testing
>> platform, it is completely untested.
>>
>> Erik: Can you try to have look?
>
> Sorry for the late reply, I'm currently in bed with pneumonia.
>
> But I gave it a quick go, but as-is it's a NACK; a wall of warnings and errors.
>
> But with this patch on top, it seems to at least compile OK:
>
> ---8<---
> diff --git a/contrib/credential/helper/credential_helper.h
> b/contrib/credential/helper/credential_helper.h
> index 7e73fc6..13b611e 100644
> --- a/contrib/credential/helper/credential_helper.h
> +++ b/contrib/credential/helper/credential_helper.h
> @@ -125,6 +125,7 @@ static inline char *xstrdup(const char *str)
>         return ret;
>  }
>
> +#ifndef NO_STRNDUP
>  static inline char *xstrndup(const char *str, size_t len)
>  {
>         char *ret = strndup(str,len);
> @@ -133,5 +134,6 @@ static inline char *xstrndup(const char *str, size_t len)
>
>         return ret;
>  }
> +#endif
>
>  #endif /* CREDENTIAL_HELPER_H_INCLUDED_ */
> diff --git a/contrib/credential/wincred/Makefile
> b/contrib/credential/wincred/Makefile
> index ee7a8ef..3900322 100644
> --- a/contrib/credential/wincred/Makefile
> +++ b/contrib/credential/wincred/Makefile
> @@ -1,9 +1,10 @@
>  MAIN:=git-credential-wincred
> -all:: $(MAIN)
> +all:: $(MAIN).exe
>
>  CC = gcc
>  RM = rm -f
>  CFLAGS = -O2 -Wall
> +CPPFLAGS = -DNO_STRNDUP
>
>  -include ../../../config.mak.autogen
>  -include ../../../config.mak
> diff --git a/contrib/credential/wincred/git-credential-wincred.c
> b/contrib/credential/wincred/git-credential-wincred.c
> index 721e59f..e26ba47 100644
> --- a/contrib/credential/wincred/git-credential-wincred.c
> +++ b/contrib/credential/wincred/git-credential-wincred.c
> @@ -6,6 +6,7 @@
>  #include <stdio.h>
>  #include <io.h>
>  #include <fcntl.h>
> +#include <credential_helper.h>
>
>  /* MinGW doesn't have wincred.h, so we need to define stuff */
>
> @@ -124,9 +125,8 @@ static int prepare_credential(struct credential *c)
>                 wusername = utf8_to_utf16_dup(c->username);
>         if (c->password)
>                 wpassword = utf8_to_utf16_dup(c->password);
> -       if (c->port) {
> -               snprintf(port_buf,"%hd",c->port);
> -       }
> +       if (c->port)
> +               snprintf(port_buf, sizeof(port_buf), "%hd", c->port);
>         return EXIT_SUCCESS;
>  }
>
> @@ -170,7 +170,7 @@ static int get_credential(struct credential *c)
>
>         /* search for the first credential that matches username */
>         for (i = 0; i < num_creds; ++i)
> -               if (match_cred(creds[i])) {
> +               if (match_cred(creds[i], c)) {
>                         cred = creds[i];
>                         break;
>                 }
> ---8<---
>
> However, I haven't been able to successfully run the tests on the
> result. When I did, I got this error:
>
> ---8<---
> rm: cannot remove `t/trash directory.t0303-credential-external/stderr': Permissi
> on denied
> rm: cannot remove `t/trash directory.t0303-credential-external/stdout': Permissi
> on denied
> rm: cannot remove directory `t/trash directory.t0303-credential-external': Direc
> tory not empty
> ---8<---
>
> And I'm not currently feeling up to debugging stuck processes or whatever it is.

OK, that was due to a stuck process, and after killing it I got to
test properly. However, three tests fail now:


$ (cd t && GIT_TEST_CREDENTIAL_HELPER=wincred ./t0303-credential-external.sh)
ok 1 - helper (wincred) has no existing data
ok 2 - helper (wincred) stores password
not ok - 3 helper (wincred) can retrieve password
#
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       --
#                       protocol=https
#                       host=example.com
#                       username=store-user
#                       password=store-pass
#                       --
#                       EOF
#
ok 4 - helper (wincred) requires matching protocol
ok 5 - helper (wincred) requires matching host
ok 6 - helper (wincred) requires matching username
ok 7 - helper (wincred) requires matching path
ok 8 - helper (wincred) can forget host
not ok - 9 helper (wincred) can store multiple users
#
#                       check approve $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       password=pass1
#                       EOF
#                       check approve $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#                       check fill $HELPER <<-\EOF &&
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user1
#                       password=pass1
#                       EOF
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#
ok 10 - helper (wincred) can forget user
not ok - 11 helper (wincred) remembers other user
#
#                       check fill $HELPER <<-\EOF
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       --
#                       protocol=https
#                       host=example.com
#                       username=user2
#                       password=pass2
#                       EOF
#
# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)
# failed 3 among 11 test(s)
1..11

So, something else is up as well.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]