Discussion:
[Freeipa-devel] [freeipa PR#490][opened] [WIP] certdb: use certutil and match_hostname for cert verification
HonzaCholasta
2017-02-21 09:24:21 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification
Action: opened

PR body:
"""
Use certutil and ssl.match_hostname calls instead of python-nss for
certificate verification.
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
tiran
2017-02-21 10:34:25 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification

tiran commented:
"""
Do we ensure that the function is always called with an IDN A-Label encoded hostname? ```ssl.match_hostname``` assumes that all parts are A-labels, not U-labels.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-281305611
HonzaCholasta
2017-02-21 11:12:13 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification

HonzaCholasta commented:
"""
@tiran, how do I ensure that?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-281313807
tiran
2017-02-21 11:18:35 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification

tiran commented:
"""
The hostname must be ASCII text. Something like ```hostname.encode('ascii')``` should catch non-ASCII text and Python 3 bytes.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-281315108
HonzaCholasta
2017-02-21 11:29:38 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
HonzaCholasta
2017-02-22 14:30:32 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
HonzaCholasta
2017-03-08 10:17:36 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: [WIP] certdb: use certutil and match_hostname for cert verification
Action: edited

Changed field: title
Original value:
"""
[WIP] certdb: use certutil and match_hostname for cert verification
"""
HonzaCholasta
2017-03-08 10:18:06 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

HonzaCholasta commented:
"""
I think this PR is ready now.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-285002490
stlaz
2017-03-27 07:06:48 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

stlaz commented:
"""
@tiran Could you please finish the review? I guess we can omit the change in `.spec.in` for the review time being.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-289370833
stlaz
2017-03-28 15:24:49 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: certdb: use certutil and match_hostname for cert verification
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
stlaz
2017-03-28 15:27:09 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

stlaz commented:
"""
I tried to use the wonderful github tool to resolve conflicts to make this more review-friendly but I guess it kind of missed the magic, it's ready for review anyway, please, finish it.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-289807247
tiran
2017-03-28 16:16:18 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

tiran commented:
"""
github magic is bad magic :/ It still shows up as 'conflicting' for me.

I'll try to find time to review the issue tomorrow, Thursday latest.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-289822893
HonzaCholasta
2017-03-29 06:04:12 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: certdb: use certutil and match_hostname for cert verification
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
tiran
2017-03-29 19:47:06 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

tiran commented:
"""
Your PR is going to remove the last import from python-nss. Awesome!

Please remove the requirement from ```ipapython/setup.py``` and ```freeipa.spec.in```, too.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-290204064
HonzaCholasta
2017-03-30 10:29:03 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: certdb: use certutil and match_hostname for cert verification
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490
HonzaCholasta
2017-03-30 12:03:04 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

HonzaCholasta commented:
"""
Awesome indeed!

As for your suggestions to improve the validation, I completely agree with them, but the focus of this PR is to refactor the current validation not to use python-nss, which it delivers. Could you please file a ticket for the improvements, so that it gets more visibility and can be properly tracked?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-290390283
tiran
2017-03-31 08:01:53 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

Label: +ack
MartinBasti
2017-03-31 10:20:58 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

MartinBasti commented:
"""
master:

* 9183cf2a7505624235b255b1406702cdaa65bb38 certdb: use certutil and match_hostname for cert verification
* 2b33230f669ca22d6948a4a351b4c92ba15222ab setup, pylint, spec file: drop python-nss dependency


"""

See the full comment at https://github.com/freeipa/freeipa/pull/490#issuecomment-290676024
MartinBasti
2017-03-31 10:21:02 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Title: #490: certdb: use certutil and match_hostname for cert verification

Label: +pushed
MartinBasti
2017-03-31 10:21:03 UTC
Permalink
URL: https://github.com/freeipa/freeipa/pull/490
Author: HonzaCholasta
Title: #490: certdb: use certutil and match_hostname for cert verification
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/490/head:pr490
git checkout pr490

Loading...