bruno Piguet added the comment:
I apologise for coming back to this issue lately, after its closing.
I must have misconfigured something in my tracking system.
Thank-you everybody for the work done, especiallly the careful handling and
documenting of the case only if password is present in
R. David Murray added the comment:
Nothing stops us from have a post-mortem discussion on a closed issue :)
The rationale for only doing the check for .netrc is that that is
backward-compatibility-wise fairly safe, because other tools will already be
insisting on the same security. But for
Roundup Robot added the comment:
New changeset 6396d1fc72da by R David Murray in branch '3.1':
#14984: On POSIX, enforce permissions when reading default .netrc.
http://hg.python.org/cpython/rev/6396d1fc72da
New changeset 0d9e471221da by R David Murray in branch '3.2':
Merge #14984: On POSIX,
R. David Murray added the comment:
Thanks, Benjamin. And Thank you, Bruno.
--
resolution: - fixed
stage: - committed/rejected
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
Roundup Robot added the comment:
New changeset fb3ad8a749c8 by R David Murray in branch '2.6':
#14984: only import pwd on POSIX.
http://hg.python.org/cpython/rev/fb3ad8a749c8
New changeset 88e62c43e443 by R David Murray in branch '2.7':
Merge #14984: only import pwd on POSIX.
Benjamin Peterson added the comment:
Fine for 3.1.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
___
Python-bugs-list mailing list
Roundup Robot added the comment:
New changeset 1b673e0fd8f3 by R David Murray in branch '2.6':
Add versionchanged for #14984, remove extra blank from string.
http://hg.python.org/cpython/rev/1b673e0fd8f3
New changeset 48be42b94381 by R David Murray in branch '2.7':
Merge: Add versionchanged for
R. David Murray added the comment:
Well, I got the answer to the may question, but not the can question. The
answer to that question is no:
remote: - changeset 6396d1fc72da on disallowed branch '3.1'!
remote: * Please strip the offending changeset(s)
remote: * and re-do them, if needed, on
Benjamin Peterson added the comment:
You should be able to push now.
2013/9/17 R. David Murray rep...@bugs.python.org:
R. David Murray added the comment:
Well, I got the answer to the may question, but not the can question.
The answer to that question is no:
remote: - changeset
Barry A. Warsaw added the comment:
@RDM: Please commit to 2.6 and null merge to 2.7. Thanks!
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
R. David Murray added the comment:
Well, I was planning to merge it, since 2.7 needs the fix as well.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
Barry A. Warsaw added the comment:
On Sep 16, 2013, at 03:12 PM, R. David Murray wrote:
Well, I was planning to merge it, since 2.7 needs the fix as well.
Oh yeah, that's fine of course. And thanks!
--
___
Python tracker rep...@bugs.python.org
R. David Murray added the comment:
The patch for 3.1 is very close to the 2.7 patch, and is attached.
Benjamin and Georg, I'd like to apply this to 3.1 and merge it up through
default. May I and can I?
--
Added file: http://bugs.python.org/file31800/netrc-py3.1.patch
R. David Murray added the comment:
Removing 2.6 and 2.7 from versions since it is now fixed there. I'll work on
porting it to python3.
--
versions: -Python 2.6, Python 2.7
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
Roundup Robot added the comment:
New changeset e5c4eb6b8e05 by R David Murray in branch '2.6':
#14984: On POSIX, enforce permissions when reading default .netrc.
http://hg.python.org/cpython/rev/e5c4eb6b8e05
New changeset 2e19c65d6688 by R David Murray in branch '2.7':
Merge #14984: On POSIX,
Georg Brandl added the comment:
I would welcome a versionchanged block in the docs addition.
There seems to be a stray space in the string in the last line here:
+try:
+user = pwd.getpwuid(os.getuid())[0]
+
R. David Murray added the comment:
For the security fix, the check should only be done if the file is the the
default .netrc. (Which would also make your error message correct...otherwise
it is not :) Also, it would make more sense for the 'prop =' to be inside the
'if posix'.
Barry, with
R. David Murray added the comment:
Note that I'll test it by hand before applying, and will write a test for 3.3
(where Mock is available to make testing practical).
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
Barry A. Warsaw added the comment:
On Sep 15, 2013, at 06:05 PM, R. David Murray wrote:
For the security fix, the check should only be done if the file is the the
default .netrc. (Which would also make your error message
correct...otherwise it is not :) Also, it would make more sense for the
Changes by Barry A. Warsaw ba...@python.org:
--
nosy: +benjamin.peterson, georg.brandl, larry
priority: high - release blocker
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
R. David Murray added the comment:
Here is a 2.6 specific patch. I've hand tested this.
--
keywords: +patch
Added file: http://bugs.python.org/file31776/netrc-2.6.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
R. David Murray added the comment:
I could write a 2.6 test for the permissions part, but not for the incorrect
owner part. Do you want one without the other?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
Barry A. Warsaw added the comment:
On Sep 15, 2013, at 06:51 PM, R. David Murray wrote:
I could write a 2.6 test for the permissions part, but not for the incorrect
owner part. Do you want one without the other?
Yeah, I guess you can't mock os or stat in 2.6. ;)
Let's test the permission
R. David Murray added the comment:
Hmm. Answering the doc question caused me to run into something that calls the
whole patch into question:
http://www.unix.com/unix-dummies-questions-answers/11326-netrc-refuses-password.html.
In that example, the ftp program only rejected reading the
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:
--
nosy: +Arfrever
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:
--
versions: +Python 3.1
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
Barry A. Warsaw added the comment:
FWIW, the Ubuntu manpage netrc(5) says:
password string
Supply a password. If this token is present, the auto-login
process will supply the specified string if the remote server
requires a password as part of
R. David Murray added the comment:
Here is an updated patch, with docs and test.
Turns out it actually wasn't necessary to move the check to the password, but
I'm leaving it that way anyway. The reason it wasn't necessary is that we
don't actually parse the .netrc file correctly: we require
Barry A. Warsaw added the comment:
@RDM: In netrc.rst, s/posix/POSIX/
It also looks like you're keeping the ownership test. Did I misunderstand
msg197815? I thought you were only going to keep the permission test?
--
___
Python tracker
Changes by R. David Murray rdmur...@bitdance.com:
Removed file: http://bugs.python.org/file31779/netrc-2.6.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
Changes by R. David Murray rdmur...@bitdance.com:
Added file: http://bugs.python.org/file31780/netrc-2.6.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
R. David Murray added the comment:
Yes, you did :) I was using permissions check to cover both tests, since as
you say, if the file is owned by someone other than the user running the
processes, a user other than the one running the process has permission to
modify it.
posix-POSIX fixed in
Changes by bruno Piguet bruno.pig...@gmail.com:
--
versions: +Python 3.2
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
___
Changes by Antoine Pitrou pit...@free.fr:
--
nosy: +giampaolo.rodola
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
___
___
Python-bugs-list
bruno Piguet added the comment:
I missed the 3.3 window, may I re-propose the same minimal patch against
3.4.0a1 ?
I'm not sure I follow any python standard lib coding style but the general idea
is quite simple and easy to get.
I chose to ignore the backward compatibility concern, since I
bruno Piguet bruno.pig...@gmail.com added the comment:
Do you agree that the attached patch could be a practical solution ?
The patch is for the 2.6 version of the lib. Transposition to other versions
should be trivial.
If we don't want to break backward compatibility, the solution is to add a
R. David Murray rdmur...@bitdance.com added the comment:
Thanks for the patch.
I think the extra check should be done unconditionally in the case where we've
looked up the default .netrc file. Adding a feature to 3.3 to provide an
optional check for other files (with default False) would
Changes by bruno Piguet bruno.pig...@gmail.com:
--
title: netrc module alows read of non-secured .netrc file - netrc module
allows read of non-secured .netrc file
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue14984
R. David Murray rdmur...@bitdance.com added the comment:
This seems like something we should fix for the default file read. There is a
backward compatibility concern, but I think the security aspect overrides that.
--
components: +Library (Lib)
nosy: +r.david.murray
priority: normal -
39 matches
Mail list logo
