Description of problem: I connect from my workstation via ssh to a build host and checkout the sources from a non-local git repository. So far so good, I have no problem when I do "git clone git.com:my_package.git my_package" by hand. When I use mock in combination with the scm option I receive an error. [calbrech@rhel6-dev-mock ~]$ mock -v --scm-enable --scm-option package=my_package DEBUG: Initializing SCM integration... DEBUG: SCM checkout command: git clone git.com:my_package.git my_package DEBUG: SCM checkout post command: git checkout master DEBUG: SCM checkout directory: /tmp/tmp3Y65mA.mock-scm.my_package DEBUG: Executing command: ['git', 'clone', 'git.com:my_package.git', 'my_package'] DEBUG: Initialized empty Git repository in /tmp/tmp3Y65mA.mock-scm.my_package/my_package/.git/ DEBUG: Permission denied, please try again. DEBUG: Permission denied, please try again. DEBUG: Permission denied (publickey,gssapi-with-mic,password). DEBUG: fatal: The remote end hung up unexpectedly DEBUG: Child returncode was: 128 ERROR: Command failed. See logs for output. # ['git', 'clone', 'git.com:my_package.git', 'my_package'] Version-Release number of selected component (if applicable): I have installed mock-1.1.21-1.el6.noarch from EPEL Possible solution: diff --git a/py/mockbuild/scm.py b/py/mockbuild/scm.py index 10ce46f..2610aa8 100644 --- a/py/mockbuild/scm.py +++ b/py/mockbuild/scm.py @@ -83,7 +83,7 @@ class scmWorker(object): # non-interactively old_home = os.environ['HOME'] os.environ['HOME'] = pwd.getpwuid(os.getuid()).pw_dir - mockbuild.util.do(shlex.split(self.get), shell=False, cwd=self.wrk_dir) + mockbuild.util.do(shlex.split(self.get), env=os.environ, shell=False, cwd=self.wrk_dir) if self.postget: mockbuild.util.do(shlex.split(self.postget), shell=False, cwd=self.src_dir) os.environ['HOME'] = old_home This patch works for me. To access a git repository via ssh public key authentification you need pass the SSH_AUTH_SOCK environment variable to the git host.
> To access a git repository via ssh public key authentification you need pass > the SSH_AUTH_SOCK environment variable to the git host. This is something that was discussed in the past but IIRC we never quite conclude what would be the best approach. Currently there is the following in scm.py: os.environ['SSH_AUTH_SOCK'] = pwd.getpwuid(os.getuid()).pw_dir + "/.ssh/auth_sock" So if you have $HOME/.ssh/auth_sock then it works already. However, this of course requires manual setup and your patch would get rid of that, being much more user friendly. Clark, do you remember anything related to this, does the above patch look safe security-wise? Iff so, I think we should update it to remove the .ssh/auth_sock line and apply. Thanks.
If you pass an empty env dict to mockbuild.util.do() the whole environment is cleared in util.py. I've tried to remove the .ssh/auth_sock line in scm.py, but it seems that the environment is cleared somewhere else before the mock.build.do() is called. So it makes sense to have the ~/.ssh/auth_sock environment variable set in scm.py.
(In reply to comment #1) > > Clark, do you remember anything related to this, does the above patch look safe > security-wise? Iff so, I think we should update it to remove the .ssh/auth_sock > line and apply. Marko, I don't think we ever came to an agreement on how to handle it. But (as Christian states in #c2), there are still places lurking where we nuke the environment. I think our best bet is to track down places where we're clearing the environment and replace them with self.env, then make sure that self.env has what we want for SSH_AUTH_SOCK.
Created attachment 570811 [details] possible fix to scm environment issue Had a thought while I was sitting drinking coffee :) What do you guys think of this where we copy the environ in the SCM constructor, change it to suit and use it always when we're doing SCM operations? This is completely untested, just wanted to see what you guys thought.
This is a good approach. Because os.environ always return a dict, I would use os.environ.copy() instead of copy.copy(). Both methods would return a shallow copy of the dict. And we keep the imports as short as possible. I test your patch this afternoon.
Yes, this looks like an improvement but for SSH_AUTH_SOCK I wonder would it be better to add SSH_AUTH_SOCK to KEEP_ENV_VARS in /etc/security/console.apps/mock?
Clark, your patch worked well for me (tested with both copy.copy() and os.environ.copy()). Marko, I have tried your suggestion from #c6 too. It doesn't work for me. But there is a chance that I made some mistake :)
(In reply to comment #7) > Marko, I have tried your suggestion from #c6 too. It doesn't work for me. But > there is a chance that I made some mistake :) Try using the following in /etc/security/console.apps/mock and uncommenting the SSH_AUTH_SOCK line in scm.py: KEEP_ENV_VARS=COLUMNS,SSH_AUTH_SOCK
(In reply to comment #8) > (In reply to comment #7) > > Marko, I have tried your suggestion from #c6 too. It doesn't work for me. But > > there is a chance that I made some mistake :) > > Try using the following in /etc/security/console.apps/mock and uncommenting the > SSH_AUTH_SOCK line in scm.py: > > KEEP_ENV_VARS=COLUMNS,SSH_AUTH_SOCK Thats what I have done in my first try. I had to pass the environment dict to mockbuild.util.do() again to get it working. With this approach the ~/.ssh/auth_sock hack isn't needed anymore.
(In reply to comment #5) > This is a good approach. > > Because os.environ always return a dict, I would use os.environ.copy() instead > of copy.copy(). Both methods would return a shallow copy of the dict. And we > keep the imports as short as possible. Doh! I completely forgot that dictionaries have a copy() method. Good catch. I've added this to my work branch and I'll see about pushing it to testing this week.
(In reply to comment #8) > (In reply to comment #7) > > Marko, I have tried your suggestion from #c6 too. It doesn't work for me. But > > there is a chance that I made some mistake :) > > Try using the following in /etc/security/console.apps/mock and uncommenting the > SSH_AUTH_SOCK line in scm.py: > > KEEP_ENV_VARS=COLUMNS,SSH_AUTH_SOCK Should we add this to etc/consolehelper/mock?
Created attachment 571387 [details] Preserve SSH_AUTH_SOCK for SCM > Should we add this to etc/consolehelper/mock? Yes, please see the attached patch. It also drops SSH_AUTH_SOCK right after SCM has grabbed it (strictly speaking probably not necessary at least currently but better safe than sorry).
Hmmm. Since we're unconditionally setting HOME in our local environment copy, shouldn't we be moving SSH_AUTH_SOCK as well? I'm not sure we should drop setting SSH_AUTH_SOCK in scm.py...
(In reply to comment #13) > Hmmm. > > Since we're unconditionally setting HOME in our local environment copy, > shouldn't we be moving SSH_AUTH_SOCK as well? I'm not sure we should drop ^^^^^ That's "setting" not moving. > setting SSH_AUTH_SOCK in scm.py...
> I'm not sure we should drop setting SSH_AUTH_SOCK in scm.py... The idea was to preserve SSH_AUTH_SOCK in the environment passed to mock and thus copy it in scm.py alongside with other variables. So if it's there we'll get it automatically. I think it could be dropped but if you have some scenario in mind where the hard-coded value would be useful then SSH_AUTH_SOCK could be set to it conditionally.
ahh, I misunderstood the intent. My thought was to use the value of SSH_AUTH_SOCK from the containing environment if it's there and to hard-code it if it's not, but upon reflection maybe hard-coding a value to be in the home directory of the account running mock is not the best idea. Here's what I have currently: diff --git a/py/mockbuild/scm.py b/py/mockbuild/scm.py index e93e39e..22d89d2 100644 --- a/py/mockbuild/scm.py +++ b/py/mockbuild/scm.py @@ -76,7 +76,8 @@ class scmWorker(object): # non-interactively self.environ['HOME'] = pwd.getpwuid(os.getuid()).pw_dir self.environ['CVS_RSH'] = "ssh" - self.environ['SSH_AUTH_SOCK'] = pwd.getpwuid(os.getuid()).pw_dir + "/.ssh/auth_sock" + if not self.environ.has_key('SSH_AUTH_SOCK'): + self.environ['SSH_AUTH_SOCK'] = pwd.getpwuid(os.getuid()).pw_dir + "/.ssh/auth_sock" decorate(traceLog()) def get_sources(self): But I'm not sure that it makes sense to add it if it's not in the original environment. Your thoughts?
Created attachment 572370 [details] updated possible fix to scm environment issue Here's what I have currently for this BZ. Please look this over and let me know if further changes are needed.
Looks good though I'd still drop SSH_AUTH_SOCK in mock.py just in case. Thanks.
Oh, you mean in os.environ, after we make the copy? Yeah that makes sense.
Created attachment 572777 [details] Final (hopefully) scm environment fix Ok, here's what I've got in my work branch. I'll try and get a new mock spun up today or tomorrow for testing.
mock-1.1.22-1.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/mock-1.1.22-1.el6
mock-1.1.22-1.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc15
mock-1.0.29-1.el5 has been submitted as an update for Fedora EPEL 5. https://admin.fedoraproject.org/updates/mock-1.0.29-1.el5
mock-1.1.22-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc17
mock-1.1.22-1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/mock-1.1.22-1.fc16
mock-1.1.22-2.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/mock-1.1.22-2.fc17
Package mock-1.1.22-2.fc17: * should fix your issue, * was pushed to the Fedora 17 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing mock-1.1.22-2.fc17' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2012-7324/mock-1.1.22-2.fc17 then log in and leave karma (feedback).
mock-1.1.22-2.1.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report.
mock-1.0.29-1.el5 on RedHat 5.8 still have this issue because usermode-1.88-3.el5.2 ignores KEEP_ENV_VARS option.
Created attachment 587386 [details] Configure environment from command line This patch adds an ability to configure environment variables using command-line switch --environment. The main use case is to pass SSH_AUTH_SOCK value when usermode package is too old to understand KEEP_ENV_VARS option: --environment SSH_AUTH_SOCK=$SSH_AUTH_SOCK
Hi Max, thanks for your patch suggestion - it'd be best if you could file a new BZ against mock/EPEL5 to have it properly reviewed.
Marko, thanks for advise. Patch is attached to new bug 828987.