[SSSD] [sssd PR#5566][opened] Fix exponent padding when deriving rsapubkey to ssh

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: opened

PR body:
"""
Padding is sometimes needed if a nonstandard exponent is chosen.

The fix is just a couple of lines in cert.c

But the integration-test requires a certificate to be pushed to LDAP and the output should match the pubkey derived from THE original certificate via p11-tool and openssh.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

peptekmail commented:
"""
This problem probably arised when switching from nss to openssl.
Backporting to redhat8 would be nice.
Debian/Ubuntu is about to switch to openssl and will probably notice the problem if they use the feature.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-813011612

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: edited

Changed field: body
Original value:
"""
Padding is sometimes needed if a nonstandard exponent is chosen.

The fix is just a couple of lines in cert.c

But the integration-test requires a certificate to be pushed to LDAP and the output should match the pubkey derived from THE original certificate via p11-tool and openssh.
"""

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: reopened

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

peptekmail commented:
"""
@sumit-bose
This fix solves one of several problems with some 3:rd party certificates, this problem is not isolated to rsassa-pss but could occur om any rsa-based client-cert.
However most CA:s go for the default openssl settings to avoid this problem.
I have not found any similair code anywhere else in SSSD but someone more experienced might know better.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-815249236

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

sumit-bose commented:
"""
Hi,

thanks for the patch, I agree with the fix. You are right, this issue is not specific to rsassa-pss but is related to how BIGNUM handles negative numbers (highest bit must be set). The similar code is just in the following lines in `rsa_pub_key_to_ssh()` where the `0` is added unconditionally to the modulus. Iirc in the old NSS based code there was a similar if-block than the one you used here. I guess adding the `0` unconditionally to the exponent should work as well.

I added some inline comments as well.

bye,
Sumit


"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-815482211

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[alexey-tikhonov]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

alexey-tikhonov commented:
"""
Please, avoid `Merge branch ... ` commit (use rebase) and please squash patches where having separate commits doesn't make much sense.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-816181876

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

peptekmail commented:
"""

Post by sumit-bose
Hi,
thanks for the patch, I agree with the fix. You are right, this issue is not specific to rsassa-pss but is related to how BIGNUM handles negative numbers (highest bit must be set). The similar code is just in the following lines in `rsa_pub_key_to_ssh()` where the `0` is added unconditionally to the modulus. Iirc in the old NSS based code there was a similar if-block than the one you used here. I guess adding the `0` unconditionally to the exponent should work as well.
I added some inline comments as well.
bye,
Sumit

I tester. O

Post by sumit-bose
Hi,
thanks for the patch, I agree with the fix. You are right, this issue is not specific to rsassa-pss but is related to how BIGNUM handles negative numbers (highest bit must be set). The similar code is just in the following lines in `rsa_pub_key_to_ssh()` where the `0` is added unconditionally to the modulus. Iirc in the old NSS based code there was a similar if-block than the one you used here. I guess adding the `0` unconditionally to the exponent should work as well.
I added some inline comments as well.
bye,
Sumit

Thanks for pointing that out.
I tested both conditional an unconditional padding with '0' om both exponent and modulus but this setup is the only one that seems to work every time.
Openssh does this in a diffrent way I do not fullty grasp so if anyone gets into trouble with this change that is where to look.
But the integration test should catch that and make it easy to compare diffrent certs.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-816198991

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

peptekmail commented:
"""

Post by sumit-bose
Hi,
thanks for the patch, I agree with the fix. You are right, this issue is not specific to rsassa-pss but is related to how BIGNUM handles negative numbers (highest bit must be set). The similar code is just in the following lines in `rsa_pub_key_to_ssh()` where the `0` is added unconditionally to the modulus. Iirc in the old NSS based code there was a similar if-block than the one you used here. I guess adding the `0` unconditionally to the exponent should work as well.
I added some inline comments as well.
bye,
Sumit
Hi,
thanks for the patch, I agree with the fix. You are right, this issue is not specific to rsassa-pss but is related to how BIGNUM handles negative numbers (highest bit must be set). The similar code is just in the following lines in `rsa_pub_key_to_ssh()` where the `0` is added unconditionally to the modulus. Iirc in the old NSS based code there was a similar if-block than the one you used here. I guess adding the `0` unconditionally to the exponent should work as well.
I added some inline comments as well.
bye,
Sumit

Thanks for pointing that out.
I tested both conditional an unconditional padding with '0' om both exponent and modulus but this setup is the only one that seems to work every time.
Openssh does this in a diffrent way I do not fullty grasp so if anyone gets into trouble with this change that is where to look.
But the integration test should catch that and make it easy to compare diffrent certs.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-816198991

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566

[peptekmail]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

peptekmail commented:
"""

Post by alexey-tikhonov
Please, avoid `Merge branch ... ` commit (use rebase) and please squash patches where having separate commits doesn't make much sense.

Hopefully everything is cleansed now. One commit should be enough.

"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-816751238

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

sumit-bose commented:
"""
Hi,

thanks for the fixes and squashing everything together. ACK.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-818592153

[sumit-bose]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

Label: +Accepted

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

Label: +Ready to push

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5566

* `master`
* 0e1452421fc99c094e1c23ced4efc44993e9702b - TEST: FIX: When generating a ssh pubkey from a cert extra padding is needed if a nonstandard eponent is chosen.

"""

See the full comment at https://github.com/SSSD/sssd/pull/5566#issuecomment-818672052

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

Label: +Pushed

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

Label: -Accepted

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh

Label: -Ready to push

[pbrezina]

URL: https://github.com/SSSD/sssd/pull/5566
Author: peptekmail
Title: #5566: Fix exponent padding when deriving rsapubkey to ssh
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5566/head:pr5566
git checkout pr5566