Re: Subscribing Apple people to git-security@googlegroups.com

Re: Subscribing Apple people to git-security@googlegroups.com

[Jeff King]

On Mon, Jul 02, 2018 at 09:29:41PM +0200, Christian Couder wrote:

> When people complained a month ago about the MacOS package on
> https://git-scm.com/ not being up-to-date after the Git security
> release, I got in touch with Apple people GitLab has been working with
> to see if they could help on this.

Unfortunately I don't think this will quite solve the issue we had, just
because people get their copy of Git in various ways. So Homebrew
updated pretty promptly, but people going to git-scm.com to find a
binary package were left without help. Likewise, this will help people
getting Git as part of XCode, but not people gettin the package from
git-scm.com.

All that said, I'm happy to get as many binary packagers into the loop
as early as possible. It can only help, even if it doesn't solve all
problems. :)

> Please add these addresses to the git-security mailing list:
>     jeremyhu@apple.com
>     akilsrin@apple.com
>     dt-epm@group.apple.com

Done.

> Please add these GitHub accounts to the cabal repo:
>     jeremyhu

Done.

>     productsecurityOSSapple

I couldn't find that account. Is it maybe a team name within the apple
org or something?

> I am also personally very happy with the Apple developers' willingness
> to get involved and help.

Yes, welcome aboard!

I hope that maybe they're also interested in reducing the overall diff
between upstream Git and what ships with XCode. Last time I looked
(which was admittedly a while ago), a lot of the changes seemed like
things that could probably be considered upstream.

-Peff

Re: Subscribing Apple people to git-security@googlegroups.com

[Jeff King]

On Mon, Jul 02, 2018 at 01:58:21PM -0700, Akilsrin wrote:

> Could “ProdsecOSS " <prodsecoss@apple.com> also be added to the
> git-security mailing list. It’s another account I control to ensure my
> team and I track open source bugs.
> 
> The git repo: could you add https://github.com/product-security-OSS
> <https://github.com/product-security-OSS> and
> https://github.com/Akilsrin <https://github.com/Akilsrin> to the
> GitHub cabal repo please.

Done, done, and done.

-Peff

Re: Subscribing Apple people to git-security@googlegroups.com

[Jeff King]

On Mon, Jul 02, 2018 at 01:15:19PM -0700, Jeremy Huddleston Sequoia wrote:

> > I hope that maybe they're also interested in reducing the overall
> > diff between upstream Git and what ships with XCode. Last time I
> > looked (which was admittedly a while ago), a lot of the changes
> > seemed like things that could probably be considered upstream.
> 
> I'm very very interested in having reduced differences between what we
> ship in Xcode and what is upstream.  I've been maintaining a repo with
> our patches that I rebase as we move forward, in the hope that these
> changes might be useful to others and a derivative of them might
> eventually be accepted upstream.  See
> https://github.com/jeremyhu/git/commits/master for the current set of
> changes that are in our shipping git (currently on top of 2.17.1).

Thanks for sharing. Skimming over it, I see:

 - several of the changes look related to run-time relocation. There was
   a series that shipped in v2.18.0 related to this, so that may reduce
   your diff once you rebase.

 - The xcode_gitattributes() bits aren't likely to go upstream as-is.
   But possibly these could ship as a default $sysconfdir/gitattributes?

 - the rest look like assorted little fixes that probably could go
   upstream

-Peff

Re: Subscribing Apple people to git-security@googlegroups.com

[Jonathan Nieder]

Administrivia: do you mind if I bounce these messages to some archived
list, either git@vger.kernel.org or git-security?  Or if we'd prefer
to avoid the noise from that, do you mind if I work with Eric Wong to
get them injected in the https://public-inbox.org/ archive?

Hi,

Jeff King wrote:
> On Mon, Jul 02, 2018 at 01:15:19PM -0700, Jeremy Huddleston Sequoia wrote:

>> I'm very very interested in having reduced differences between what we
>> ship in Xcode and what is upstream.
[...]
> Thanks for sharing. Skimming over it, I see:
>
>  - several of the changes look related to run-time relocation. There was
>    a series that shipped in v2.18.0 related to this, so that may reduce
>    your diff once you rebase.
>
>  - The xcode_gitattributes() bits aren't likely to go upstream as-is.
>    But possibly these could ship as a default $sysconfdir/gitattributes?
>
>  - the rest look like assorted little fixes that probably could go
>    upstream

I agree with Peff's assessment.  I'd also like to emphasize that
upstream is happy to see an [FYI/PATCH] when you have a divergence,
which would provide a thread to reply to to figure out whether there's
some generalization that is suitable for upstream.  (For example,
maybe we want some Makefile knob to allow setting some baked-in
attributes.)

Thanks,
Jonathan

Re: Subscribing Apple people to git-security@googlegroups.com

[Jeff King]

On Tue, Jul 03, 2018 at 08:48:14AM -0700, Jonathan Nieder wrote:

> Administrivia: do you mind if I bounce these messages to some archived
> list, either git@vger.kernel.org or git-security?  Or if we'd prefer
> to avoid the noise from that, do you mind if I work with Eric Wong to
> get them injected in the https://public-inbox.org/ archive?

I don't mind at all. I'm actually going to work later today on preparing
other messages from the security list to go to the public-inbox.org
archive, so that might pave the way.

-Peff

Re: Subscribing Apple people to git-security@googlegroups.com

[Jonathan Nieder]

+git@vger
Jeff King wrote:
> On Tue, Jul 03, 2018 at 08:48:14AM -0700, Jonathan Nieder wrote:

>> Administrivia: do you mind if I bounce these messages to some archived
>> list, either git@vger.kernel.org or git-security?  Or if we'd prefer
>> to avoid the noise from that, do you mind if I work with Eric Wong to
>> get them injected in the https://public-inbox.org/ archive?
>
> I don't mind at all. I'm actually going to work later today on preparing
> other messages from the security list to go to the public-inbox.org
> archive, so that might pave the way.

Thanks, done.

This doesn't work as a trial run for
https://public-inbox.org/meta/20180703160910.GB51821@aiede.svl.corp.google.com,
since I just bounced the mails.  So it won't work for threads like the
326-message thread you mentioned, but it should work in more modest
cases.

It seems that vger only delivered some of the messages.  I suspect
it's related to DMARC.

Jonathan

Re: Subscribing Apple people to git-security@googlegroups.com

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 3, 2018 at 3:36 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Jul 02, 2018 at 01:15:19PM -0700, Jeremy Huddleston Sequoia wrote:
>
>> > I hope that maybe they're also interested in reducing the overall
>> > diff between upstream Git and what ships with XCode. Last time I
>> > looked (which was admittedly a while ago), a lot of the changes
>> > seemed like things that could probably be considered upstream.
>>
>> I'm very very interested in having reduced differences between what we
>> ship in Xcode and what is upstream.  I've been maintaining a repo with
>> our patches that I rebase as we move forward, in the hope that these
>> changes might be useful to others and a derivative of them might
>> eventually be accepted upstream.  See
>> https://github.com/jeremyhu/git/commits/master for the current set of
>> changes that are in our shipping git (currently on top of 2.17.1).
>
> Thanks for sharing. Skimming over it, I see:
>
>  - several of the changes look related to run-time relocation. There was
>    a series that shipped in v2.18.0 related to this, so that may reduce
>    your diff once you rebase.
>
>  - The xcode_gitattributes() bits aren't likely to go upstream as-is.
>    But possibly these could ship as a default $sysconfdir/gitattributes?
>
>  - the rest look like assorted little fixes that probably could go
>    upstream

Jeremy, could you elaborate on what
https://github.com/jeremyhu/git/commit/61b42bc5d2 was about? I.e.
where was this discussed & tests for this refused?

Seems sensible to me to have this in some form, but the test as-is
seems to be a general regression test, not Apple-specific, so it would
need to be changed somewhat, or does it only happen with some other
custom patch of yours?

Re: Subscribing Apple people to git-security@googlegroups.com

[Johannes Schindelin]

Hi Peff,

On Mon, 2 Jul 2018, Jeff King wrote:

> On Mon, Jul 02, 2018 at 09:29:41PM +0200, Christian Couder wrote:
> 
> > When people complained a month ago about the MacOS package on
> > https://git-scm.com/ not being up-to-date after the Git security
> > release, I got in touch with Apple people GitLab has been working with
> > to see if they could help on this.
> 
> Unfortunately I don't think this will quite solve the issue we had, just
> because people get their copy of Git in various ways. So Homebrew
> updated pretty promptly, but people going to git-scm.com to find a
> binary package were left without help. Likewise, this will help people
> getting Git as part of XCode, but not people gettin the package from
> git-scm.com.

Indeed. The fix for that would be to automate those official Git for macOS
builds. I suggested exactly that to Tim:
https://github.com/gitgitgadget/git/pull/7#issuecomment-403820169

Ciao,
Dscho

Re: Subscribing Apple people to git-security@googlegroups.com

[Jeremy Huddleston Sequoia]

[-- Attachment #1: Type: text/plain, Size: 2799 bytes --]



> On Jul 10, 2018, at 5:27 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> 
> On Tue, Jul 3, 2018 at 3:36 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Jul 02, 2018 at 01:15:19PM -0700, Jeremy Huddleston Sequoia wrote:
>> 
>>>> I hope that maybe they're also interested in reducing the overall
>>>> diff between upstream Git and what ships with XCode. Last time I
>>>> looked (which was admittedly a while ago), a lot of the changes
>>>> seemed like things that could probably be considered upstream.
>>> 
>>> I'm very very interested in having reduced differences between what we
>>> ship in Xcode and what is upstream.  I've been maintaining a repo with
>>> our patches that I rebase as we move forward, in the hope that these
>>> changes might be useful to others and a derivative of them might
>>> eventually be accepted upstream.  See
>>> https://github.com/jeremyhu/git/commits/master for the current set of
>>> changes that are in our shipping git (currently on top of 2.17.1).
>> 
>> Thanks for sharing. Skimming over it, I see:
>> 
>> - several of the changes look related to run-time relocation. There was
>>   a series that shipped in v2.18.0 related to this, so that may reduce
>>   your diff once you rebase.
>> 
>> - The xcode_gitattributes() bits aren't likely to go upstream as-is.
>>   But possibly these could ship as a default $sysconfdir/gitattributes?
>> 
>> - the rest look like assorted little fixes that probably could go
>>   upstream
> 
> Jeremy, could you elaborate on what
> https://github.com/jeremyhu/git/commit/61b42bc5d2 was about? I.e.
> where was this discussed & tests for this refused?
> 
> Seems sensible to me to have this in some form, but the test as-is
> seems to be a general regression test, not Apple-specific, so it would
> need to be changed somewhat, or does it only happen with some other
> custom patch of yours?

It was a bug in upstream git and not a bug specific to an Apple change.  We haven't traditionally had many custom changes on our end.  The few we have, we didn't feel they were appropriate or were often rejected when we tried (eg: using CommonCrypto and Security.framework, this one, etc.).

For this particular case, I discussed the bug with the committer (Carlo) and reviewer (Junio) of the commit (18e051a3981f38db08521bb61ccf7e4571335353) via email back in October 2011.  My proposed fix and test were never accepted.  As such, we continued to ship my patch in Xcode's git and MacPorts' git until the underlying bug was actually fixed by someone else in 2014 (ddc2a6281595fd24ea01497c496f88c40a59562f + 655ee9ea3e6c0af57d320e84723ec3bf656cdbf7).  I kept the test in our test suite to ensure we didn't regress.  Here's the final post from that thread after the fix in 2014:

[-- Attachment #2: Re: [PATCH git] setup: Do not strip trailing _ from paths.eml --]
[-- Type: message/rfc822, Size: 11990 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 5007 bytes --]

FWIW, it seems that this bug was addressed by ddc2a6281595fd24ea01497c496f88c40a59562f

Thanks Martin, now we're no longer carrying around an extra patch for our build of git ;)

--Jeremy

> On Oct 17, 2011, at 14:55, Jeremy Huddleston <jeremyhu@apple.com> wrote:
> 
> ping.  Did you get my response below with extra details?  I just got a duplicate bug report, so it apparently effects people...
> 
> Please let me know if I can be of further assistance.
> 
> On Oct 11, 2011, at 2:17 PM, Jeremy Huddleston wrote:
> 
>> Thanks for your response Junio.  The text of the original bug report is below.
>> 
>> I created a git bisect test script which bisected the problem and found out that the difference was that the trailing / was removed by your code change.  git treats paths with a trailing / differently.  I don't know *why* it treats them differently, but it does.
>> 
>> There's nothing "special" about JustDoItGit.tar.bz2 except that it contains a .git dir and has a file layout that works with the bisect script I wrote.  You can test this yourself by:
>> 
>> mkdir -p ~/tmp/PR-10238070
>> cd ~/tmp/PR-10238070
>> tar xjf JustDoItGit.tar.bz2
>> cd ~/git-checkout
>> /path/to/test_10238070.sh
>> 
>> Here's the original report:
>> 
>> I've tracked the cause of '<rdar://problem/10160992> ##snipped title##' down to a regression in git.
>> 
>> Unzip the attached JustDoItGit.zip project and replace the path in the following commands to the unzipped location on your system:
>> 
>> #delete git in /usr/bin/git
>> sudo rm -r /usr/bin/git
>> #link it to /usr/local/bin/git since that's where ditto will place the new bits
>> sudo ln -s /usr/local/bin/git /usr/bin/git
>> 
>> # first, install git 1.7.3.2 to verify that the bug does not reproduce
>> sudo ditto ~rc/Software/Slate/Roots/Git/Git-14~19.root/ /
>> sudo rm -r /Users/<you>/MyGitRepo.gitdir
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir init --bare --quiet
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir --work-tree=/ add -- /Users/<you>/Desktop/JustDoItGit/ /Users/<you>/Desktop/JustDoItGit/JustDoItGit/JustDoItGitAppDelegate.h /Users/<you>/Desktop/JustDoItGit/JustDoItGitTests
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir --work-tree=/ commit -m "Hello."
>> 
>> The expected result of the commit is something like "18 files changed, 7364 insertions". If that's what you get, great, now keep going.
>> 
>> sudo rm -r /Users/<you>/MyGitRepo.gitdir
>> # install the slate version of git, 1.7.5.4
>> sudo ditto ~rc/Software/Slate/Roots/Git/Git-19.root~2/ /
>> sudo rm -r /Users/<you>/MyGitRepo.gitdir
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir init --bare --quiet
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir --work-tree=/ add -- /Users/<you>/Desktop/JustDoItGit/ /Users/<you>/Desktop/JustDoItGit/JustDoItGit/JustDoItGitAppDelegate.h /Users/<you>/Desktop/JustDoItGit/JustDoItGitTests
>> git --git-dir=/Users/<you>/MyGitRepo.gitdir --work-tree=/ commit -m "Hello."
>> 
>> The expected result is what's above, something like "18 files changed, 7364 insertions". But the actual result is that only the root folder "/Users/<you>/Desktop/JustDoItGit is added
>> 
>> This is a problem because it subsequently causes <rdar://problem/10160992> ##snipped title##
>> 
>> … and therefore breaks Xcode's snapshots feature.
>> 
>> <JustDoItGit.tar.bz2><test_10238070.sh>
>> 
>> On Oct 11, 2011, at 10:45, Junio C Hamano wrote:
>> 
>>> Jeremy Huddleston <jeremyhu@apple.com> writes:
>>> 
>>>> real_path will strip the trailing / from provided paths.  This fixes
>>>> a regression introduced in 18e051a3981f38db08521bb61ccf7e4571335353
>>> 
>>> What is the breakage? The above does not explain why stripping the '/' is
>>> a wrong thing, and which caller that used to work is broken by that
>>> behaviour.
>>> 
>>> A new test block in some of the t/t[0-9]*.sh script to demonstrate the
>>> breakage and fix to explain and justify your fix better, please?
>>> 
>>>> 
>>>> Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
>>>> ---
>>>> 
>>>> Here's an updated version that should be a bit more portable and warning-free.
>>>> 
>>>> setup.c |   10 +++++++++-
>>>> 1 files changed, 9 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/setup.c b/setup.c
>>>> index 61c22e6..e3a8ae3 100644
>>>> --- a/setup.c
>>>> +++ b/setup.c
>>>> @@ -10,8 +10,16 @@ char *prefix_path(const char *prefix, int len, const char *path)
>>>> 	char *sanitized;
>>>> 	if (is_absolute_path(orig)) {
>>>> 		const char *temp = real_path(path);
>>>> -		sanitized = xmalloc(len + strlen(temp) + 1);
>>>> +		sanitized = xmalloc(len + strlen(temp) + 2);
>>>> 		strcpy(sanitized, temp);
>>>> +
>>>> +		temp = strrchr(path, '\0');
>>>> +		temp--;
>>>> +		if (*temp == '/') {
>>>> +			char *s = strrchr(sanitized, '\0');
>>>> +			s[0] = '/';
>>>> +			s[1] = '\0';
>>>> +		}
>>>> 	} else {
>>>> 		sanitized = xmalloc(len + strlen(path) + 1);
>>>> 		if (len)
>>> 
>> 
> 


[-- Attachment #2.1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4109 bytes --]

[-- Attachment #3: Type: text/plain, Size: 358 bytes --]




Once I rebase on top of 2.18, I'll send out the full set of changes to git@vger as a starting point for discussion again.  I imagine many are not acceptable in current form but might be a starting point for additional discussion (eg: adding options for vendor-specific version rather than the hard coded "Apple Git-##" string).

Thanks,
Jeremy