[Py3] ipa-server-install #428
Conversation
ipaserver/install/dsinstance.py
Outdated
| os.close(dmpwdfd) | ||
| NTF = tempfile.NamedTemporaryFile | ||
| dir = paths.VAR_LIB_IPA | ||
| with NTF("w", dir=dir) as dmpwdfile, NTF("w", dir=dir) as admpwdfile: |
There was a problem hiding this comment.
urks, the code is getting even less readable. dir shadows a builtin, too.
There was a problem hiding this comment.
I wanted to avoid extra indentation, I can use full form with backlashes
There was a problem hiding this comment.
Can be this?
dir_ipa = paths.VAR_LIB_IPA
with tempfile.NamedTemporaryFile("w", dir=dir_ipa) as dmpwdfile, \
tempfile.NamedTemporaryFile("w", dir=dir_ipa) as admpwdfile:
dmpwdfile.write(self.dm_password)
dmpwdfile.flush()
There was a problem hiding this comment.
How about
...
def tempf():
return tempfile.NamedTemporaryFile('w', dir=paths.VAR_LIB_IPA)
with tempf() as dmpwdfile, tempf() as admpwdfile:
...?
ipaserver/plugins/baseldap.py
Outdated
| delval = unicode(base64.b64encode(delval)) | ||
| raise errors.AttrValueNotFound(attr=attr, value=delval) | ||
| delval = base64.b64encode(delval).decode('ascii') | ||
| raise errors.AttrValueNotFound(attr=attr, value=delval) |
There was a problem hiding this comment.
I will rebase it, half of this PR is PR #427 , thats why it is WIP
configparser.get() changed in python3 and `raw` is now a keyword attribute.
Also it must be set to True, otherwise InterpolationSyntaxError is raised
'''
InterpolationSyntaxError: '%' must be followed by '%' or '(', found:
'%2fvar%2frun%2fslapd-EXAMPLE-COM.socket'
'''
https://fedorahosted.org/freeipa/ticket/4985
basedn in custodia related modules has type bytes, that causes issues in Py3 when strings were concatenated with bytes ``` malformed RDN string = "cn=custodia,cn=ipa,cn=etc,b'dc=example,dc=com'" ``` https://fedorahosted.org/freeipa/ticket/4985
python ldap requires bytes as values https://fedorahosted.org/freeipa/ticket/4985
Exception is too brod and may hide various issues that show up later. If the code expects that entry may exist, then ldap.ALREADY_EXISTS exception should be used
ldap ldif parser requires to have input file opened in textual mode https://fedorahosted.org/freeipa/ticket/4985
...and vice versa backup requires string not bytes, but ldap provide bytes thus data must be decoded and encoded from restore https://fedorahosted.org/freeipa/ticket/4985
python ldif support only bytes as values, literals must be bytes https://fedorahosted.org/freeipa/ticket/4985
Also code was rewritten to use NamedTemporaryFile with context https://fedorahosted.org/freeipa/ticket/4985
Convert function to NamedTemporaryFile with textual mode, because passwords are text. Using `with` and NamedTemporaryFile gives more security agains leaking password from tempfiles. https://fedorahosted.org/freeipa/ticket/4985
ipapython/ipautil.py
Outdated
| @@ -859,7 +859,7 @@ def ipa_generate_password(entropy_bits=256, uppercase=1, lowercase=1, digits=1, | |||
| # NSS function sftk_newPinCheck() in nss/lib/softoken/fipstokn.c. | |||
| for charclass_name in ['digits', 'uppercase', 'lowercase', 'special']: | |||
| charclass = pwd_charsets[charclass_name] | |||
| todo_characters = req_classes[charclass_name] | |||
| todo_characters = req_classes[charclass_name] or 0 | |||
There was a problem hiding this comment.
Why is this necessary? req_classes is constructed from integer arguments, no?
There was a problem hiding this comment.
No, it can contain None, None == character class will not be used
| b'groupOfPrincipals']), | ||
| ('cn', name.encode('utf-8')), | ||
| ('ipaKeyUsage', RFC5280_USAGE_MAP[usage].encode('utf-8')), | ||
| ('memberPrincipal', principal.encode('utf-8')), |
There was a problem hiding this comment.
How hard would it be to refactor this to use ipaldap (which handles encoding for you)?
There was a problem hiding this comment.
I saw that custodia related modules are not using ipalib, so I'm not sure if this is on purpose or just NIH, so I don't know. I'm afraid I dont have enough time to refactor it.
| @@ -181,7 +180,7 @@ def __init__(self, config=None, ipaconf=paths.IPA_DEFAULT_CONF): | |||
| self.realm = conf.get('global', 'realm') | |||
| self.ldap_uri = config.get('ldap_uri', None) | |||
| if self.ldap_uri is None: | |||
| self.ldap_uri = conf.get('global', 'ldap_uri', None) | |||
| self.ldap_uri = conf.get('global', 'ldap_uri', raw=True) | |||
There was a problem hiding this comment.
How hard would it be to refactor this to use ipalib.config?
There was a problem hiding this comment.
Or why not to use API when this is in ldap context? We have to find first why custodia related modules is not using api, ipalib
The one cannot compare None and Int in Py3 """ unorderable types: NoneType() > int() """ Continue when class is disabled with None value https://fedorahosted.org/freeipa/ticket/4985
This PR allows to install ipa server using PY3 (CA, no DNS, no KRA, no CA-less, no external CA) and uninstall it (ipa-server-install --uninstall)
Functionality depends on #427