KEMBAR78
support for client certificates pt 2 by technoweenie · Pull Request #1893 · git-lfs/git-lfs · GitHub
Skip to content

Conversation

@technoweenie
Copy link
Contributor

This is a continuation of #1869. It ignores a common error from git users on the mac when trying to send ssl client certs. This also lets me run the circleci builds, since they didn't on #1869 for some reason.

reponame="test-cloneClientCert"
setup_remote_repo "$reponame"
pwd
out="$(clone_repo_clientcert "$reponame" "$reponame")"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My hunch is that the directory change isn't happening since clone_repo_clientcert is being called from a sub-shell, $().

If clone_repo_clientcert doesn't generate it's own log files, I'd just | tee out.log, and change the grep to:

if [ "0" -ne "$(grep -c 'client-cert-mac-openssl' < out.log)" ]; then
  # ...
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, the sub-shell explains it. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't use tee exactly, since it swallows errors. If clone_repo_clientcert() returns an error, I want the test to halt.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't use tee exactly, since it swallows errors.

You should still be able to capture non-zero exit codes with ${PIPESTATUS[0]}:

thing_that_might_fail 2>&1 | tee foo.log
ok="${PIPESTATUS[0]}"

But I don't mind either way which approach you take here 👍

works around a known issue with git on mac, compiled against the wrong openssl or curl libs
@technoweenie
Copy link
Contributor Author

Forcing a merge here, since #1869 was approved.

@technoweenie technoweenie merged commit 7e495f5 into master Jan 26, 2017
@technoweenie technoweenie deleted the schuCriX-master branch January 26, 2017 19:53
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Feb 29, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 1, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 1, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
introduced a workaround in some of our tests of Git LFS support for
TLS/SSL client certificates to deal with an exception raised when
using the version of libcurl installed by default by macOS, specifically
by macOS 10.12 ("Sierra"), which was the most recent release of macOS
at the time.

However, since at least v7.52.0, curl has included a fix for this
problem, and that will be included in all contemporary versions of
macOS.  By the time of macOS 10.14 ("Mojave"), the version of curl
included with the OS was v7.54.0, for instance.

We can therefore drop this workaround entirely, as it is no longer
applicable to any supported versions of macOS.

(Note, too, that our CI test suite always uses a libcurl version
installed using Homebrew.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 1, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, was adjusted to write out several new files
upon startup with the values of the client certificate, key, and URL
to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function run at the start
of every test script.  The lfstest-count-tests program then invokes
the lfstest-gitserver program, when necessary, and so it inherits
the LFSTEST_CLIENT_* environment variables and uses the filenames
defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, those used for the files
containing the URLs the tests should use to contact the test server
when validating regular TLS/SSL behaviour and TLS/SSL behaviour
with a client certificate.  As such, the URL for the former would
be unavailable, as the file containing it would be overwritten by
the file containing the latter's URL.

This was likely just a small oversight after copying the code
used to create these files, so we correct that now, and supply
a different default filename when the lfstest-gitserver program
writes out the file containing the URL tests should use validating
TLS/SSL behaviour with a client certificate.  Note again, though,
that this makes no difference in the context of our test suite as it
always supplies unique, non-default values for these file's names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 3, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 3, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
introduced a workaround in some of our tests of Git LFS support for
TLS/SSL client certificates to deal with an exception raised when
using the version of libcurl installed by default by macOS, specifically
by macOS 10.12 ("Sierra"), which was the most recent release of macOS
at the time.

However, since at least v7.52.0, curl has included a fix for this
problem, and that will be included in all contemporary versions of
macOS.  By the time of macOS 10.14 ("Mojave"), the version of curl
included with the OS was v7.54.0, for instance.

We can therefore drop this workaround entirely, as it is no longer
applicable to any supported versions of macOS.

(Note, too, that our CI test suite always uses a libcurl version
installed using Homebrew.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 3, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, was adjusted to write out several new files
upon startup with the values of the client certificate, key, and URL
to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function run at the start
of every test script.  The lfstest-count-tests program then invokes
the lfstest-gitserver program, when necessary, and so it inherits
the LFSTEST_CLIENT_* environment variables and uses the filenames
defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, those used for the files
containing the URLs the tests should use to contact the test server
when validating regular TLS/SSL behaviour and TLS/SSL behaviour
with a client certificate.  As such, the URL for the former would
be unavailable, as the file containing it would be overwritten by
the file containing the latter's URL.

This was likely just a small oversight after copying the code
used to create these files, so we correct that now, and supply
a different default filename when the lfstest-gitserver program
writes out the file containing the URL tests should use validating
TLS/SSL behaviour with a client certificate.  Note again, though,
that this makes no difference in the context of our test suite as it
always supplies unique, non-default values for these file's names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 6, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 6, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
introduced a workaround in some of our tests of Git LFS support for
TLS/SSL client certificates to deal with an exception raised when
using the version of libcurl installed by default by macOS, specifically
by macOS 10.12 ("Sierra"), which was the most recent release of macOS
at the time.

However, since at least v7.52.0, curl has included a fix for this
problem, and that will be included in all contemporary versions of
macOS.  By the time of macOS 10.14 ("Mojave"), the version of curl
included with the OS was v7.54.0, for instance.

We can therefore drop this workaround entirely, as it is no longer
applicable to any supported versions of macOS.

(Note, too, that our CI test suite always uses a libcurl version
installed using Homebrew.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 6, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, was adjusted to write out several new files
upon startup with the values of the client certificate, key, and URL
to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function run at the start
of every test script.  The lfstest-count-tests program then invokes
the lfstest-gitserver program, when necessary, and so it inherits
the LFSTEST_CLIENT_* environment variables and uses the filenames
defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, those used for the files
containing the URLs the tests should use to contact the test server
when validating regular TLS/SSL behaviour and TLS/SSL behaviour
with a client certificate.  As such, the URL for the former would
be unavailable, as the file containing it would be overwritten by
the file containing the latter's URL.

This was likely just a small oversight after copying the code
used to create these files, so we correct that now, and supply
a different default filename when the lfstest-gitserver program
writes out the file containing the URL tests should use validating
TLS/SSL behaviour with a client certificate.  Note again, though,
that this makes no difference in the context of our test suite as it
always supplies unique, non-default values for these file's names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 8, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, was adjusted to write out several new files
upon startup with the values of the client certificate, key, and URL
to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function run at the start
of every test script.  The lfstest-count-tests program then invokes
the lfstest-gitserver program, when necessary, and so it inherits
the LFSTEST_CLIENT_* environment variables and uses the filenames
defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, those used for the files
containing the URLs the tests should use to contact the test server
when validating regular TLS/SSL behaviour and TLS/SSL behaviour
with a client certificate.  As such, the URL for the former would
be unavailable, as the file containing it would be overwritten by
the file containing the latter's URL.

This was likely just a small oversight after copying the code
used to create these files, so we correct that now, and supply
a different default filename when the lfstest-gitserver program
writes out the file containing the URL tests should use validating
TLS/SSL behaviour with a client certificate.  Note again, though,
that this makes no difference in the context of our test suite as it
always supplies unique, non-default values for these file's names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 8, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub in PR git-lfs#3808).

We expect to address this issue in subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing problems.
One of these occurs on the legacy CentOS 7 Linux system where the libcurl
library used by Git still depends on the libnss3 library, which has several
restrictions in the types of TLS certificates it accepts.  Specifically,
in our current builds on CentOS 7, libcurl version 7.29.0 is linked
against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both Extended Key
Usage attributes for TLS Web Server Authentication, and the Basic
Constraint setting "CA:TRUE", meaning the certificate is intended
to be used as a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's net/http/httptest
package, specifically the one from the net/http/internal/testcert package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git is dependent on the libnss3 library for TLS
certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in Travis CI.  We no longer use
Travis, as noted above, and we can fully enable this test with
"http.<url>.sslVerify" set to its default value of "true", at least
on the majority of the systems we use in our GitHub Actions CI jobs.

When we make this change as well, three tests in the t/t-clone.sh
script (the "cloneSSL", "clone ClientCert", and "clone ClientCert with
homedir certs" tests) will fail because libnss3 rejects the certifcate
provided by the Go language test server, for the reason described
above.  In all three tests, we see libnss3 return a "Certificate type not
approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of offical support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to find a solution to this problem
by generating a different certificate for our lfstest-gitserver
utility program, we simply skip these three tests if we detect
that they are running on a Linux platform where the git-remote-https
binary depends on libnss3.  We can remove these exceptions later
once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 10, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We expect to address this issue in subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing problems.
One of these occurs on the legacy CentOS 7 Linux system where the libcurl
library used by Git still depends on the libnss3 library, which has several
restrictions in the types of TLS certificates it accepts.  Specifically,
in our current builds on CentOS 7, libcurl version 7.29.0 is linked
against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both Extended Key
Usage attributes for TLS Web Server Authentication, and the Basic
Constraint setting "CA:TRUE", meaning the certificate is intended
to be used as a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's net/http/httptest
package, specifically the one from the net/http/internal/testcert package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git is dependent on the libnss3 library for TLS
certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in Travis CI.  We no longer use
Travis, as noted above, and we can fully enable this test with
"http.<url>.sslVerify" set to its default value of "true", at least
on the majority of the systems we use in our GitHub Actions CI jobs.

When we make this change as well, three tests in the t/t-clone.sh
script (the "cloneSSL", "clone ClientCert", and "clone ClientCert with
homedir certs" tests) will fail because libnss3 rejects the certifcate
provided by the Go language test server, for the reason described
above.  In all three tests, we see libnss3 return a "Certificate type not
approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of offical support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to find a solution to this problem
by generating a different certificate for our lfstest-gitserver
utility program, we simply skip these three tests if we detect
that they are running on a Linux platform where the git-remote-https
binary depends on libnss3.  We can remove these exceptions later
once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 15, 2024
The "cloneSSL" test in what is now our t/t-clone.sh test script was
added in commits 4c64e82 and
8f91a1b of PR git-lfs#1067, and since then
has performed the same basic set of checks.  A bare test repository
is first cloned using a TLS/SSL connection.  Example Git LFS objects
are then committed to the repository, and pushed, after which a
"git lfs clone" command is run with a specified, non-default directory
name argument so the repository is cloned into a new working tree
under that local directory.  The test then checks that the Git LFS
objects have also been populated into the new working tree.  Finally,
a regular "git clone" command is run (without a directory name argument),
again with a TLS/SSL connection, which should run the Git LFS "smudge"
filter and fail if that does not also succeed.

This test design was later used as a template for the "clone ClientCert"
test when it was added in commit daba49a
of PR git-lfs#1893; like the "cloneSSL" test, it performed the same series of
checks, just using a client TLS certificate and key when cloning from
the remote.

We then added support for encrypted TLS certificate keys in commit
706beca of PR git-lfs#3270, and at this
time, the "clone ClientCert" was updated to repeat the "git lfs clone"
step and checks using first an unencrypted key and then an encrypted
key.  However, the "git clone" step was not moved into the same
loop with the "git lfs clone" command, so it is only run using the
unencrypted key.  For this reason, the "http.<url>.sslKey" Git
configuration value is reset after the end of the loop, so it
contains just the unencrypted key's path.

As we expect to make several other adjustments to this test in
subsequent commits in this PR, we first bring the "git clone" command
into the same loop with the "git lfs clone" command, which means
we no longer need to reset the "http.<url>.sslKey" configuration
value, as we want to use the same value for both cloning commands.
This change also ensures we perform all the same checks with both
types of TLS certificate keys.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Sep 15, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, and ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" to allow the "clone ClientCert"
test to pass in the Travis CI environment.

However, we no longer use the Travis CI platform, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we have revised several tests in our t/t-clone.sh test script
so they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if
"http.<url>.sslVerify" is set to "true".

As well, in another commit in this PR we have altered our t/t-clone.sh
test script to require the use of OpenSSL on Windows instead of the
default Schannel backend.

With these changes to our test suite, all our tests should now pass
if we allow the "http.<url>.sslVerify" Git configuration option to
default to "true", so we now remove the call which explicitly set
that option to "false".
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, was adjusted to write out several new files
upon startup with the values of the client certificate, key, and URL
to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function run at the start
of every test script.  The lfstest-count-tests program then invokes
the lfstest-gitserver program, when necessary, and so it inherits
the LFSTEST_CLIENT_* environment variables and uses the filenames
defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, those used for the files
containing the URLs the tests should use to contact the test server
when validating regular TLS/SSL behaviour and TLS/SSL behaviour
with a client certificate.  As such, the URL for the former would
be unavailable, as the file containing it would be overwritten by
the file containing the latter's URL.

This was likely just a small oversight after copying the code
used to create these files, so we correct that now, and supply
a different default filename when the lfstest-gitserver program
writes out the file containing the URL tests should use validating
TLS/SSL behaviour with a client certificate.  Note again, though,
that this makes no difference in the context of our test suite as it
always supplies unique, non-default values for these file's names.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We expect to address this issue in subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing problems.
One of these occurs on the legacy CentOS 7 Linux system where the libcurl
library used by Git still depends on the libnss3 library, which has several
restrictions in the types of TLS certificates it accepts.  Specifically,
in our current builds on CentOS 7, libcurl version 7.29.0 is linked
against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both Extended Key
Usage attributes for TLS Web Server Authentication, and the Basic
Constraint setting "CA:TRUE", meaning the certificate is intended
to be used as a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's net/http/httptest
package, specifically the one from the net/http/internal/testcert package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git is dependent on the libnss3 library for TLS
certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in Travis CI.  We no longer use
Travis, as noted above, and we can fully enable this test with
"http.<url>.sslVerify" set to its default value of "true", at least
on the majority of the systems we use in our GitHub Actions CI jobs.

When we make this change as well, three tests in the t/t-clone.sh
script (the "cloneSSL", "clone ClientCert", and "clone ClientCert with
homedir certs" tests) will fail because libnss3 rejects the certifcate
provided by the Go language test server, for the reason described
above.  In all three tests, we see libnss3 return a "Certificate type not
approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of offical support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to find a solution to this problem
by generating a different certificate for our lfstest-gitserver
utility program, we simply skip these three tests if we detect
that they are running on a Linux platform where the git-remote-https
binary depends on libnss3.  We can remove these exceptions later
once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run in the
Travis CI environment.  The commit description from that time notes that
we could not determine why the test was failing only on that platform,
so the decision was made to just ignore the test in that case.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and as of
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh tests script were amended to skip
execution in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin ("test" or "[" or "[[").
As described in git-lfs#5658, the result is that when the TRAVIS variable
is undefined, these statements always succeed, and so their conditional
blocks run, meaning these tests are skipped on all platforms, at
least since we migrated our CI jobs to GitHub Actions in PR git-lfs#3808.

In previous commits in this PR we have addressed all the latent
bugs that have accumulated in these tests since this problem was
first introduced, and so we are now in a position to remove both
the valid and invalid checks for the TRAVIS environment variable
from all of our tests.  This should ensure that the full set of
tests in both t/t-askpass.sh and t/t-clone.sh run on all our CI
and build platforms.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
introduced a workaround in some of our tests of Git LFS support for
TLS/SSL client certificates to deal with an exception raised when
using the version of libcurl installed by default by macOS, specifically
by macOS 10.12 ("Sierra"), which was the most recent release of macOS
at the time.

However, since at least v7.52.0, curl has included a fix for this
problem, and that will be included in all contemporary versions of
macOS.  By the time of macOS 10.14 ("Mojave"), the version of curl
included with the OS was v7.54.0, for instance.

We can therefore drop this workaround entirely, as it is no longer
applicable to any supported versions of macOS.

(Note, too, that our CI test suite always uses a libcurl version
installed using Homebrew.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
The "cloneSSL" test in what is now our t/t-clone.sh test script was
added in commits 4c64e82 and
8f91a1b of PR git-lfs#1067, and since then
has performed the same basic set of checks.  A bare test repository
is first cloned using a TLS/SSL connection.  Example Git LFS objects
are then committed to the repository, and pushed, after which a
"git lfs clone" command is run with a specified, non-default directory
name argument so the repository is cloned into a new working tree
under that local directory.  The test then checks that the Git LFS
objects have also been populated into the new working tree.  Finally,
a regular "git clone" command is run (without a directory name argument),
again with a TLS/SSL connection, which should run the Git LFS "smudge"
filter and fail if that does not also succeed.

This test design was later used as a template for the "clone ClientCert"
test when it was added in commit daba49a
of PR git-lfs#1893; like the "cloneSSL" test, it performed the same series of
checks, just using a client TLS certificate and key when cloning from
the remote.

We then added support for encrypted TLS certificate keys in commit
706beca of PR git-lfs#3270, and at this
time, the "clone ClientCert" was updated to repeat the "git lfs clone"
step and checks using first an unencrypted key and then an encrypted
key.  However, the "git clone" step was not moved into the same
loop with the "git lfs clone" command, so it is only run using the
unencrypted key.  For this reason, the "http.<url>.sslKey" Git
configuration value is reset after the end of the loop, so it
contains just the unencrypted key's path.

As we expect to make several other adjustments to this test in
subsequent commits in this PR, we first bring the "git clone" command
into the same loop with the "git lfs clone" command, which means
we no longer need to reset the "http.<url>.sslKey" configuration
value, as we want to use the same value for both cloning commands.
This change also ensures we perform all the same checks with both
types of TLS certificate keys.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 7, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, and ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" to allow the "clone ClientCert"
test to pass in the Travis CI environment.

However, we no longer use the Travis CI platform, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we have revised several tests in our t/t-clone.sh test script
so they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if
"http.<url>.sslVerify" is set to "true".

As well, in another commit in this PR we have altered our t/t-clone.sh
test script to require the use of OpenSSL on Windows instead of the
default Schannel backend.

With these changes to our test suite, all our tests should now pass
if we allow the "http.<url>.sslVerify" Git configuration option to
default to "true", so we now remove the call which explicitly set
that option to "false".
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Our lfstest-gitserver test utility program was updated in commit
daba49a of PR git-lfs#1893 to help test
Git LFS's support for the use of client TLS/SSL certificates, and as
part of this change, the program was adjusted to write out several
new files upon startup with the values of the client certificate,
key, and URL to be used when testing this Git LFS feature.

In practice, the names of these files are always supplied by our
test scripts in LFSTEST_CLIENT_* environment variables, as defined
in our t/testenv.sh script and then passed to our lfstest-count-tests
test utility program in the setup() shell function that is run at the
start of every test script.  The lfstest-count-tests program then
invokes the lfstest-gitserver program, when necessary, and so the
lfstest-gitserver program inherits the LFSTEST_CLIENT_* environment
variables and uses the filenames defined by them.

However, should the lfstest-gitserver program be started manually,
without all of these environment variables, it will use a set of
default filenames for the files it creates after starting.  Two
of these default filenames are identical, specifically the default
names of the files containing the URLs the tests should use to contact
the test server when validating regular TLS/SSL behaviour and TLS/SSL
behaviour with a client certificate.  As such, the URL for the former
would be unavailable, as the file containing it would be overwritten
by the file containing the latter's URL.

We correct this minor issue now by defining a unique default name for
the file which contains the URL the tests should use when validating
TLS/SSL behaviour with a client certificate.  Note again, though, that
this makes no difference in the context of our test suite as it always
supplies unique, non-default values for all these files' names in its
environment variables.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing
problems.  One of these occurs on the legacy CentOS 7 Linux system where
the libcurl library used by Git still depends on the libnss3 library,
which has several restrictions in the types of TLS certificates it
accepts.  Specifically, in our current builds on CentOS 7, libcurl
version 7.29.0 is linked against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both an Extended Key
Usage attribute for TLS Web Server Authentication and a Basic Constraint
setting of "CA:TRUE", meaning the certificate is intended to be used as
a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's "net/http/httptest"
package, specifically the one from the "net/http/internal/testcert"
package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git on that platform is dependent on the libnss3
library for TLS certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in the Travis CI environment.  We no
longer use the Travis CI service, as noted above, but independent of
that, we want to be able to run all our tests with "http.<url>.sslVerify"
set to its default value of "true".

When we set the "http.<url>.sslVerify" Git configuration option to
"true", however, three tests in the t/t-clone.sh test script (the
"cloneSSL", "clone ClientCert", and "clone ClientCert with homedir
certs" tests) will fail on CentOS 7 because libnss3 rejects the TLS
certificate provided by the Go language test server, for the reasons
described above.  In these tests, libnss3 will return a "Certificate
type not approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of official support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to generate a different TLS certificate
for our lfstest-gitserver utility program, we simply skip the three
affected tests when we detect that they are running on a Linux platform
where the git-remote-https binary depends on libnss3.  We can remove
these exceptions later once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 9, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
Due to the issue described in git-lfs#5658, the "cloneSSL" and "clone ClientCert"
tests in our t/t-clone.sh test script do not run to completion but exit
early, as a consequence of an improper check of the TRAVIS variable (which
is no longer used since we migrated our test suite to GitHub Actions in
PR git-lfs#3808).

We will address this issue in a subsequent commit in this PR, at which
point the tests will run fully, exposing a number of long-standing
problems.  One of these occurs on the legacy CentOS 7 Linux system where
the libcurl library used by Git still depends on the libnss3 library,
which has several restrictions in the types of TLS certificates it
accepts.  Specifically, in our current builds on CentOS 7, libcurl
version 7.29.0 is linked against version 3.90 of libnss3.

The libnss3 library rejects certificates which have both an Extended Key
Usage attribute for TLS Web Server Authentication and a Basic Constraint
setting of "CA:TRUE", meaning the certificate is intended to be used as
a Certificate Authority:

  https://security.stackexchange.com/questions/176177/why-does-curl-nss-encryption-library-not-allow-a-ca-with-the-extended-key-usage

However, certificates with both these settings are accepted by most
modern TLS libraries, and the self-signed certificate used by our
lfstest-gitserver utility program has both attributes.  Note that this
certificate is the one provided by the Go language's "net/http/httptest"
package, specifically the one from the "net/http/internal/testcert"
package:

  https://github.com/golang/go/blob/7529d09a11496a77ccbffe245607fbd256200991/src/net/http/internal/testcert/testcert.go#L10-L33

Thus when we enable our "clone ClientCert" test, the second part of the
test, where it checks the use of an encrypted client key, will fail
on CentOS 7 because Git on that platform is dependent on the libnss3
library for TLS certificate validation.

Moreover, we would also like to reverse the change made in commit
1cf7d6b of PR git-lfs#1893, when the
"http.<url>.sslVerify" Git configuration option was set to "false"
in the setup() shell function of our t/testhelpers.sh library.  This
was done, according to the commit description, to try to get the
"clone ClientCert" test working in the Travis CI environment.  We no
longer use the Travis CI service, as noted above, but independent of
that, we want to be able to run all our tests with "http.<url>.sslVerify"
set to its default value of "true".

When we set the "http.<url>.sslVerify" Git configuration option to
"true", however, three tests in the t/t-clone.sh test script (the
"cloneSSL", "clone ClientCert", and "clone ClientCert with homedir
certs" tests) will fail on CentOS 7 because libnss3 rejects the TLS
certificate provided by the Go language test server, for the reasons
described above.  In these tests, libnss3 will return a "Certificate
type not approved for application" SEC_ERROR_INADEQUATE_CERT_TYPE error.

We expect to drop support for CentOS 7 soon, given that it has
reached the end of official support in all but one distribution
(SUSE Linux Enterprise Server 12 SP5, for which general support
ends on October 31 of 2024).  As well, CentOS 8 and more recent
related distributions have switched away from building libcurl
against libnss3; in fact, support for that TLS library has been
dropped by the curl project entirely as of v8.3.0, per commit
curl/curl@7c8bae0.

Therefore, rather than try to generate a different TLS certificate
for our lfstest-gitserver utility program, we simply skip the three
affected tests when we detect that they are running on a Linux platform
where the git-remote-https binary depends on libnss3.  We can remove
these exceptions later once we drop support for CentOS 7.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
In commit d2c1c0f of PR git-lfs#1067 the
"cloneSSL" test in what is now our t/t-clone.sh test script was changed
with the intent that the test should be skipped when it was run by the
Travis CI service.  The commit description from that time notes that
we could not determine why the test was failing on just that platform,
so the decision was made to simply ignore the test in the Travis CI
environment.

Subsequently, similar checks were added to three other tests.  First,
the "clone ClientCert" test was introduced in PR git-lfs#1893, and since
commit b471566 in that PR it has
included an initial check for a Travis CI environment with the intent
of skipping the test when it is run there.

Next, in commits 9da65c0 of PR git-lfs#2500
and e53e34e of PR git-lfs#2609 the "askpass:
push with core.askPass" and "askpass: push with SSH_ASKPASS" tests
in what is now our t/t-askpass.sh test script were amended to also
skip executing in the Travis CI context.

In the case of the latter two tests, this check works as designed;
however, in the case of the two tests in t/t-clone.sh, the check is
written as a simple "if $TRAVIS" shell statement, rather than one
which uses a shell test command or builtin (i.e., "test" or "[" or
"[[").  As described in git-lfs#5658, the result is that when the TRAVIS
variable is undefined, these statements always succeed, and so their
conditional blocks run, meaning these tests are skipped on every
platform and system.

In previous commits in this PR we have addressed all the latent bugs
that have accumulated in these tests since this problem was first
introduced, and so we are now in a position to remove both the valid
and invalid checks for the TRAVIS environment variable from all four
of the tests.  This will ensure that our CI jobs always run the
entire set of tests from both of the t/t-askpass.sh and t/t-clone.sh
test scripts.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 29, 2024
In PR git-lfs#1893 we introduced support for TLS client certificates, and
while that was largely successful, some of our CI test jobs ran on the
Travis CI platform, and those encountered problems when validating the
TLS certificate generated by our lfstest-gitserver test utility program.
Several attempts were made to resolve this issue, but ultimately in
commit 1cf7d6b the "http.<url>.sslVerify"
Git configuration value was set to "false" for our entire test suite,
just to allow the "clone ClientCert" test to pass in the Travis CI
environment.

However, we no longer use the Travis CI service, as we migrated our
CI jobs to GitHub Actions in PR git-lfs#3808.  Further, in a prior commit in
this PR we revised several tests in our t/t-clone.sh test script so
they do not run on the legacy CentOS 7 platform, where the libcurl
library used by Git depends on the libnss3 library.  The libnss3 library
does not accept the TLS certificate generated by our lfstest-gitserver
program, so these tests would otherwise fail on CentOS 7 if the
"http.<url>.sslVerify" option is set to "true".

As well, in another commit in this PR we altered the same tests in our
t/t-clone.sh test script to require that Git and libcurl use OpenSSL for
TLS verification on Windows, rather than the default Schannel backend.
This will prevent the tests from failing if TLS verification is enabled,
since Schannel rejects the TLS certificate that we use in our
lfstest-gitserver test utility, but OpenSSL accepts it.

With these changes, all our tests should now pass if we allow the
"http.<url>.sslVerify" Git configuration option to default to "true".
Therefore we now remove the command which set that option to "false"
for our whole test suite.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 30, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
introduced a workaround in some of our tests of Git LFS support for
TLS/SSL client certificates to deal with an exception raised when
using the version of libcurl installed by default by macOS, specifically
by macOS 10.12 ("Sierra"), which was the most recent release of macOS
at the time.

However, since at least v7.52.0, curl has included a fix for this
problem, and that will be included in all contemporary versions of
macOS.  By the time of macOS 10.14 ("Mojave"), the version of curl
included with the OS was v7.54.0, for instance.

We can therefore drop this workaround entirely, as it is no longer
applicable to any supported versions of macOS.

(Note, too, that our CI test suite always uses a libcurl version
installed using Homebrew.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 30, 2024
The clone_repo*() family of helper functions are used extensively
throughout our test suite; they generally run "git clone" and capture
its output, and then change the working directory to the newly cloned
repository and write the "git clone" output into a log file there
after first calling "git config".

The first introduction of a clone.log file in clone_repo() was in
commit bc025a8 of PR git-lfs#679, and
similar log files are generated by the other functions that were
added later.

One consequence of the way these functions operate now is that,
if the "git clone" command should fail, then because we run all our
tests with the "set -e" option, no log file is generated.  The
function returns immediately, and as the log file is only created
at the end of the function, after changing directories, that step is
skipped.

A partial attempt to work around this behaviour was introduced in
commit 9a00eb3 of PR git-lfs#1893, but only
for the clone_repo_clientcert() function.  The "set +e" option is
temporarily set before running the "git clone" and then the exit code
captured before setting "set -e" again.  This allows the function to
grep for a specific error string in the log file which, if present,
indicates a problem that will not be treated as cause for immediate
failure.

Under most circumstances the current functionality in the other
functions is reasonable as any error messages "git clone" outputs
will appear in the stdout and stderr streams captured by our test
framework for debugging purposes.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want to
be able to search clone.log even after a failure.

Therefore, in order to ensure that log traces are output and saved
in all cases, we change all of the clone_repo*() functions to pipe
the output of "git clone" into a "tee" command, which writes the
log file immediately.  Because the "tee" command is that last one
in the new command pipeline, this means any errors encountered by
"git clone" will not halt the function.

Instead, we capture the "git clone" exit code using PIPESTATUS[0],
and then return if it is non-zero.  If the clone was successful,
however, we proceed to change directory, move the log file into the
new repository (where many existing tests expect it to be available),
and then return after calling "git config".

In the clone_repo_clientcert() function specifically, this change
also allows us to remove the "set +e" temporary setting as it is
no longer useful, and reduce the number of places where we need
to write the log file.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Oct 30, 2024
The "cloneSSL" test in what is now our t/t-clone.sh test script was
added in commits 4c64e82 and
8f91a1b of PR git-lfs#1067, and since then
has performed the same basic set of checks.  A bare test repository
is first cloned using a TLS/SSL connection.  Example Git LFS objects
are then committed to the repository, and pushed, after which a
"git lfs clone" command is run with a specified, non-default directory
name argument so the repository is cloned into a new working tree
under that local directory.  The test then checks that the Git LFS
objects have also been populated into the new working tree.  Finally,
a regular "git clone" command is run (without a directory name argument),
again with a TLS/SSL connection, which should run the Git LFS "smudge"
filter and fail if that does not also succeed.

This test design was later used as a template for the "clone ClientCert"
test when it was added in commit daba49a
of PR git-lfs#1893; like the "cloneSSL" test, it performed the same series of
checks, just using a client TLS certificate and key when cloning from
the remote.

We then added support for encrypted TLS certificate keys in commit
706beca of PR git-lfs#3270, and at this
time, the "clone ClientCert" was updated to repeat the "git lfs clone"
step and checks using first an unencrypted key and then an encrypted
key.  However, the "git clone" step was not moved into the same
loop with the "git lfs clone" command, so it is only run using the
unencrypted key.  For this reason, the "http.<url>.sslKey" Git
configuration value is reset after the end of the loop, so it
contains just the unencrypted key's path.

As we expect to make several other adjustments to this test in
subsequent commits in this PR, we first bring the "git clone" command
into the same loop with the "git lfs clone" command, which means
we no longer need to reset the "http.<url>.sslKey" configuration
value, as we want to use the same value for both cloning commands.
This change also ensures we perform all the same checks with both
types of TLS certificate keys.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 5, 2024
In commit 9a00eb3 of PR git-lfs#1893 we
altered our "clone ClientCert" test of the Git LFS client's support
for TLS/SSL client certificates to deal with an exception raised by
the version of libcurl provided by macOS at the time.

As was reported in curl/curl#1105, the libcurl installed as part of
macOS 10.12 ("Sierra") failed to find some TLS/SSL client certificates
and instead raised an NSInvalidArgumentException.  See the notes in
our PR git-lfs#1869 for more details:

  git-lfs#1869 (comment)

To work around this issue, we skip the "clone ClientCert" test if any
NSInvalidArgumentException messages are found in the output of the Git
clone operation performed by the clone_repo_clientcert() function
of our t/testhelpers.sh shell library.

The same workaround was then copied into the "clone ClientCert with
homedir certs" test when it was added to our t/t-clone.sh test script
in commit c54c9da of PR git-lfs#5657.

The underlying problem in libcurl was resolved by the changes in commit
curl/curl@7c9b9ad, which was included
with libcurl version 7.52.0 and so is available in all contemporary
versions of macOS.  For instance, by the time of macOS 10.14 ("Mojave"),
the version of libcurl packaged with the OS was v7.54.0.

We can therefore drop our workarounds, as they are no longer applicable
to any supported versions of macOS.

(Note, too, that the GitHub Actions runners we now use for our CI jobs
provide a libcurl version installed by Homebrew, which takes precedence
over the one supplied by macOS.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 5, 2024
The clone_repo*() family of helper functions defined in our
t/testhelpers.sh shell library are used extensively throughout our
test suite to perform "git clone" commands and log their output for
verification.  After running the "git clone" command, these functions
change the current working directory to the cloned repository's working
tree and then write the log of the command's output into that directory.
Before creating the log file, however, they call the "git config"
command to register our git-credential-lfstest test utility as the
program Git should use to store and retrieve credentials.

One consequence of the way the majority of these functions operate is
that if the "git clone" command should fail, then because we run all
our tests with the "set -e" shell option, no log file is generated.
Instead, the functions return immediately and do not change the
working directory or write a log file.

The clone_repo_clientcert() function is an exception to this
pattern, as a result of the changes introduced to it in commit
9a00eb3 of PR git-lfs#1893, which were made
to work around a problem caused by the version of libcurl installed
with macOS version 10.12 ("Sierra").

In the clone_repo_clientcert() function, the "set +e" shell option was
temporarily set before running the "git clone" command, and command's
exit code captured before setting the "set -e" option again.  This
allowed the function to check the command's output for the macOS error
message specific to the libcurl problem, and if it was present, add a
flag string to the log file and not return an error code.  The calling
test could then simply skip the remainder of its normal actions if it
detected the flag string in the log file.

In a prior commit in this PR we reversed some of these changes because
the underlying issue is fixed in all recent macOS and libcurl releases,
and so we no longer need to try to detect specific exceptions in the
output of the clone operation in the clone_repo_clientcert() function.

We did not reverse the use of the "set +e" shell option, though, because
there is some value in trying to avoid an immediate test failure if
the "git clone" command does not succeed, as will occur with all the
other clone_repo*() functions.

Under most circumstances the behaviour of the other functions is
reasonable, as any error messages from the "git clone" commands will
still appear in the stdout and stderr streams captured by our CI job
test framework, and so may be analyzed there.

However, this leaves no way for tests using these functions to
programmatically check the output of a failed "git clone" attempt.
For instance, if they are looking for a specific string to indicate
that the clone was unable to find a valid Git executable and ran
an invalid one instead (as per CVE-2022-24826), we may want a test
to be able to search the log file from a clone operation even after
it fails.

Therefore, in order to ensure that clone operation log traces are saved
in all cases, we change all of the clone_repo*() functions to pipe the
output of the "git clone" command into a tee(1) command, which then
writes the log file.  The advantage of the "tee" command over a simple
redirection of stdout is that when the "set -e" option is set, the shell
only considers the exit code of the final command in a pipeline to
determine the success of the pipeline, so a non-zero exit code from the
"git clone" command will not cause the function to return immediately.

We then retrieve the "git clone" command's exit code from the shell's
PIPESTATUS[0] variable and return if the code is non-zero.  If the clone
operation was successful, though, we proceed to change the working
directory and move the log file into the new repository's working tree,
where our tests expect to find it, and then return after calling
the "git config" command.

In the clone_repo_clientcert() function specifically, this change now
allows us to remove the use of the "set +e" shell option, as we are
able to read the exit code from the "git clone" command without it.
We also no longer need separate statements to write the log file under
both success and failure conditions.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
In commit b9602f3 of PR git-lfs#5616 we
resolved a problem which caused flaky test results in our CI jobs
by ensuring that the t/t-credential.sh test script used a unique
directory for the credential record files we provide to our
git-credential-lfstest helper program.

This change ensured that when the "clone (HTTP server/proxy require
cookies)" test in our t/t-clone.sh script creates a record file named
"localhost" in the common directory specified by the CREDSDIR
environment variable, specifically a copy of the "127.0.0.1" file
created by the setup_creds() shell library function, this does not
conflict with the "credentials from netrc" test and other related
tests in the t/t-credential.sh test script, which depend on there
being no credential records associated with the "localhost" hostname.

However, we already made a previous attempt to resolve this problem
in commits bb87944 and
bcb9eb3 of PR git-lfs#3825, but that change
assumes that our test scripts always run sequentially rather than in
parallel.  In PR git-lfs#3825 the "clone (HTTP server/proxy require cookies)"
test was added, and at the end of the test, it removes the "localhost"
credential record file it creates at the beginning of the test.

Of course, if the test ever fails, then it leaves the "localhost" record
file in place, so even if our test scripts always ran sequentially
there would still be a potential conflict with the tests in the
t/t-credential.sh script.

We can more comprehensively ensure that these two test scripts do
not conflict again in the future by using the same technique applied
in commit b9602f3 to the
t/t-credential.sh script in the t/t-clone.sh script as well.

First, we add a call to setup_creds() at the start of the t/t-clone.sh
script after setting the CREDSDIR environment variable to a path
unique to the t/t-clone.sh script.  This ensures that a separate copy
of the default credential record file for the 127.0.0.1 hostname is
available for the exclusive use of the tests in the script.

Next, we move the creation of the credential record files associated
with the TLS/SSL client certificate used in the "clone ClientCert"
test from the generic setup_creds() function into the test itself.
We would otherwise need to define the "certpath" and "keypath" variables,
as their values are interpolated by the function into the names of
these credential record files.

The TLS/SSL client certificate and key files are generated by our
lfstest-gitserver utility when it is first executed.  Their locations
are defined by the LFSTEST_CLIENT_* environment variables passed by
the setup() function in our t/testhelpers.sh shell library to the
lfstest-count-tests utility, which then runs the lfstest-gitserver
program.  The values of these environment variables are set from the
LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE* variables defined by
our t/testenv.sh script.  We now use these variables in the
"clone ClientCert" test when we call the write_creds_file() function
directly, instead of allowing the setup_creds() function to perform
those calls.

We could define the "certpath" and "keypath" variables (using the
LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE_ENCRYPTED variables)
prior to calling setup_creds() at the start of the t/t-clone.sh script.
However, the "clone ClientCert" test is the only one which actually
makes use of these credential record files, as this is the only test
which actively checks the use of an encrypted TLS/SSL client certificate,
and so is the only place where we need to create these record files.

Further, we can also move into the test the "git config" commands that
set the "http.<url>.sslCert" and "http.<url>.sslKey" Git configuration
options with the locations of the TLS/SSL client certificate files.
Previously, we set these configuration options in the setup() function
in our t/testhelpers.sh shell library, so they were configured for all
tests in all test scripts, although only the "clone ClientCert" test
makes use of them.

Finally, the "clone (HTTP server/proxy require cookies)" test no
longer needs to attempt to delete the credential record file for
the "localhost" hostname after the test is complete, so we can
simply remove that code.

Note that the "clone ClientCert" test was first introduced in
commit daba49a of PR git-lfs#1893, at
which time the "git config" commands to set the "http.<url>.sslCert"
and "http.<url>.sslKey" options were added to the setup() function,
and the LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE variables
were defined in what was then the test/testenv.sh script.  Later,
in commit 52f94c2 of PR git-lfs#3270,
the LFS_CLIENT_KEY_FILE_ENCRYPTED variable was added along with
support in the lfstest-gitserver program to generate an encrypted
certificate key file.  In another commit in PR git-lfs#3270, commit
706beca, the "clone ClientCert"
test was then expanded to validate use of the encrypted key file
with the "git lfs clone" command.

(Note too that the following test, the "clone ClientCert with homedir
certs" test, appears to also depend on credential record files for
the TLS/SSL client certificate files it creates in the dedicated home
directory used by our tests.  However, because this test does not
set the "http.sslCertPasswordProtected" Git configuration option or
the GIT_SSL_CERT_PASSWORD_PROTECTED environment variable, Git does
not attempt to retrieve a passphrase for the certificate files, and
so the associated credential record files are never actually used.
We will address this issue in a subsequent commit in this PR.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
The "cloneSSL" test in what is now our t/t-clone.sh test script was
added in commits 4c64e82 and
8f91a1b of PR git-lfs#1067, and since then
has performed the same basic set of checks.  A bare test repository
is first cloned using a TLS/SSL connection.  Example Git LFS objects
are then committed to the repository, and pushed, after which a
"git lfs clone" command is run with a specified, non-default directory
name argument so the repository is cloned into a new working tree
under that local directory.  The test then checks that the Git LFS
objects have also been populated into the new working tree.  Finally,
a regular "git clone" command is run (without a directory name argument),
again with a TLS/SSL connection, which should run the Git LFS "smudge"
filter and fail if that does not also succeed.

This test design was later used as a template for the "clone ClientCert"
test when it was added in commit daba49a
of PR git-lfs#1893; like the "cloneSSL" test, it performed the same series of
checks, just using a client TLS/SSL certificate and key when cloning
from the remote.

We then added support for encrypted TLS/SSL certificate keys in commit
706beca of PR git-lfs#3270, and at this
time, the "clone ClientCert" was updated to repeat the "git lfs clone"
step and checks using first an unencrypted key and then an encrypted
key.  However, the "git clone" step was not moved into the same
loop with the "git lfs clone" command, so it is only run using the
unencrypted key.  For this reason, the "http.<url>.sslKey" Git
configuration option is reset after the end of the loop to contain
the unencrypted key file's path.

As we expect to make several other adjustments to this test in
subsequent commits in this PR, we first bring the "git clone" command
into the same loop with the "git lfs clone" command, which means
we no longer need to reset the "http.<url>.sslKey" configuration
option, as we want to use the same value for both cloning commands.
This change also ensures we perform all the same checks with both
types of TLS/SSL certificate keys.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
In commit b9602f3 of PR git-lfs#5616 we
resolved a problem which caused flaky test results in our CI jobs
by ensuring that the t/t-credential.sh test script used a unique
directory for the credential record files we provide to our
git-credential-lfstest helper program.

This change ensured that when the "clone (HTTP server/proxy require
cookies)" test in our t/t-clone.sh script creates a record file named
"localhost" in the common directory specified by the CREDSDIR
environment variable, specifically a copy of the "127.0.0.1" file
created by the setup_creds() shell library function, this does not
conflict with the "credentials from netrc" test and other related
tests in the t/t-credential.sh test script, which depend on there
being no credential records associated with the "localhost" hostname.

However, we already made a previous attempt to resolve this problem
in commits bb87944 and
bcb9eb3 of PR git-lfs#3825, but that change
assumes that our test scripts always run sequentially rather than in
parallel.  In PR git-lfs#3825 the "clone (HTTP server/proxy require cookies)"
test was added, and at the end of the test, it removes the "localhost"
credential record file it creates at the beginning of the test.

Of course, if the test ever fails, then it leaves the "localhost" record
file in place, so even if our test scripts always ran sequentially
there would still be a potential conflict with the tests in the
t/t-credential.sh script.

We can more comprehensively ensure that these two test scripts do
not conflict again in the future by using the same technique applied
in commit b9602f3 to the
t/t-credential.sh script in the t/t-clone.sh script as well.

First, we add a call to setup_creds() at the start of the t/t-clone.sh
script after setting the CREDSDIR environment variable to a path
unique to the t/t-clone.sh script.  This ensures that a separate copy
of the default credential record file for the 127.0.0.1 hostname is
available for the exclusive use of the tests in the script.

Next, we move the creation of the credential record files associated
with the TLS/SSL client certificate used in the "clone ClientCert"
test from the generic setup_creds() function into the test itself.
We would otherwise need to define the "certpath" and "keypath" variables,
as their values are interpolated by the function into the names of
these credential record files.

The TLS/SSL client certificate and key files are generated by our
lfstest-gitserver utility when it is first executed.  Their locations
are defined by the LFSTEST_CLIENT_* environment variables passed by
the setup() function in our t/testhelpers.sh shell library to the
lfstest-count-tests utility, which then runs the lfstest-gitserver
program.  The values of these environment variables are set from the
LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE* variables defined by
our t/testenv.sh script.  We now use these variables in the
"clone ClientCert" test when we call the write_creds_file() function
directly, instead of allowing the setup_creds() function to perform
those calls.

We could define the "certpath" and "keypath" variables (using the
LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE_ENCRYPTED variables)
prior to calling setup_creds() at the start of the t/t-clone.sh script.
However, the "clone ClientCert" test is the only one which actually
makes use of these credential record files, as this is the only test
which actively checks the use of an encrypted TLS/SSL client certificate,
and so is the only place where we need to create these record files.

Further, we can also move into the test the "git config" commands that
set the "http.<url>.sslCert" and "http.<url>.sslKey" Git configuration
options with the locations of the TLS/SSL client certificate files, and
we do the same for the "create lock with server using client cert" test
in the t/t-lock.sh test script.

Previously, we set these configuration options in the setup() function
in our t/testhelpers.sh shell library, so they were configured for all
tests in all test scripts, although only the "clone ClientCert" test and
the "create lock with server using client cert" test make use of them.

Finally, the "clone (HTTP server/proxy require cookies)" test no
longer needs to attempt to delete the credential record file for
the "localhost" hostname after the test is complete, so we can
simply remove that code.

Note that the "clone ClientCert" test was first introduced in
commit daba49a of PR git-lfs#1893, at
which time the "git config" commands to set the "http.<url>.sslCert"
and "http.<url>.sslKey" options were added to the setup() function,
and the LFS_CLIENT_CERT_FILE and LFS_CLIENT_KEY_FILE variables
were defined in what was then the test/testenv.sh script.  Later,
in commit 52f94c2 of PR git-lfs#3270,
the LFS_CLIENT_KEY_FILE_ENCRYPTED variable was added along with
support in the lfstest-gitserver program to generate an encrypted
certificate key file.  In another commit in PR git-lfs#3270, commit
706beca, the "clone ClientCert"
test was then expanded to validate use of the encrypted key file
with the "git lfs clone" command.

(Note too that the following test, the "clone ClientCert with homedir
certs" test, appears to also depend on credential record files for
the TLS/SSL client certificate files it creates in the dedicated home
directory used by our tests.  However, because this test does not
set the "http.sslCertPasswordProtected" Git configuration option or
the GIT_SSL_CERT_PASSWORD_PROTECTED environment variable, Git does
not attempt to retrieve a passphrase for the certificate files, and
so the associated credential record files are never actually used.
We will address this issue in a subsequent commit in this PR.)
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
The "cloneSSL" test in what is now our t/t-clone.sh test script was
added in commits 4c64e82 and
8f91a1b of PR git-lfs#1067, and since then
has performed the same basic set of checks.  A bare test repository
is first cloned using a TLS/SSL connection.  Example Git LFS objects
are then committed to the repository, and pushed, after which a
"git lfs clone" command is run with a specified, non-default directory
name argument so the repository is cloned into a new working tree
under that local directory.  The test then checks that the Git LFS
objects have also been populated into the new working tree.  Finally,
a regular "git clone" command is run (without a directory name argument),
again with a TLS/SSL connection, which should run the Git LFS "smudge"
filter and fail if that does not also succeed.

This test design was later used as a template for the "clone ClientCert"
test when it was added in commit daba49a
of PR git-lfs#1893; like the "cloneSSL" test, it performed the same series of
checks, just using a client TLS/SSL certificate and key when cloning
from the remote.

We then added support for encrypted TLS/SSL certificate keys in commit
706beca of PR git-lfs#3270, and at this
time, the "clone ClientCert" was updated to repeat the "git lfs clone"
step and checks using first an unencrypted key and then an encrypted
key.  However, the "git clone" step was not moved into the same
loop with the "git lfs clone" command, so it is only run using the
unencrypted key.  For this reason, the "http.<url>.sslKey" Git
configuration option is reset after the end of the loop to contain
the unencrypted key file's path.

As we expect to make several other adjustments to this test in
subsequent commits in this PR, we first bring the "git clone" command
into the same loop with the "git lfs clone" command, which means
we no longer need to reset the "http.<url>.sslKey" configuration
option, as we want to use the same value for both cloning commands.
This change also ensures we perform all the same checks with both
types of TLS/SSL certificate keys.
chrisd8088 added a commit to chrisd8088/git-lfs that referenced this pull request Nov 18, 2024
In previous commits in this PR we have refactored a number of the
initial tests in our t/t-clone.sh script so they are more consistent
with each other and perform a more complete set of checks of the
"git lfs clone" command when it is used with HTTP and HTTPS remote
URLs and with TLS/SSL client certificates.

All four of these tests run one or more "git lfs clone" commands.  After
each, they check the logs from the command, and then verify that a local
Git repository has been created with the expected Git LFS hooks installed
and with files in the working tree populated from the contents of Git LFS
objects that have been downloaded.

Three of these four tests, other than the very first "clone" test, also
follow a pattern added in commits 4c64e82
and 8f91a1b of PR git-lfs#1067, in which they
run a "git clone" command after the "git lfs clone" command.  This test
design was introduced first for the "cloneSSL" test, and then replicated
in the "clone ClientCert" test when that was added in commit
daba49a of PR git-lfs#1893.  In prior commits
in this PR we have now replicated it again into the "clone ClientCert
with homedir certs" test, and have also ensured that it runs twice in
both that test and the "clone ClientCert" test, once with an unencrypted
TLS/SSL private key file and once with an encrypted one.

However, although these three tests run a "git clone" command, they
simply check that it does not exit with a non-zero error code (because
we use the "set -e" option in our tests, so if the command failed it
would cause the tests to fail).  This is less than ideal, given that the
"git lfs clone" command is now deprecated, and almost all Git LFS users
will use the regular "git clone" command instead.

As suggested by larsxschneider during the review of this PR, we can
improve all four of these tests by adding checks following each "git clone"
command similar to those we perform after the "git lfs clone" commands.

We can also add a "git clone" command to the end of the initial "clone"
test so that it now follows the same pattern as the other three tests.

In each of these tests, we now confirm that the "git clone" command
exits with a non-zero value, but also that it outputs a "Cloning into"
message.  We then check that Git LFS hooks were installed in the cloned
local repository, that the expected number of Git LFS objects were
downloaded, and that files in the working tree have been populated
with the contents of those objects.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants