Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

idp preauth: handle multiple State messages from ipa-otpd #6088

Closed
abbra opened this issue Mar 31, 2022 · 2 comments
Closed

idp preauth: handle multiple State messages from ipa-otpd #6088

abbra opened this issue Mar 31, 2022 · 2 comments
Assignees
Labels
Closed: Fixed Issue was closed as fixed. idp

Comments

@abbra
Copy link
Contributor

abbra commented Mar 31, 2022

When an external IdP returns a state that is longer than 254 characters, it is not possible to package it into a State response over RADIUS protocol exchange from ipa-otpd to KDC. This is the case, for example, for Microsoft's IdPs in Azure.

We already have support for Reply-Message to be split over multiple attribute sets. Add the same for State.

@abbra
Copy link
Contributor Author

abbra commented Mar 31, 2022

Example of the failing communication:

Mar 31 09:52:52 dc.ipa.test systemd[1]: Started ipa-otpd service (PID 7334/UID 0).
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: LDAP: ldapi://%2Frun%2Fslapd-IPA-TEST.socket
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: request received
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: user query start
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: user query end: uid=ab,cn=users,cn=accounts,dc=ipa,dc=test
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: idp query start: cn=azure,cn=idp,dc=ipa,dc=test
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: idp query end: azure
Mar 31 09:52:52 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: oauth2 start: Get device code
Mar 31 09:52:52 dc.ipa.test oidc_child[15036]: oidc_child started.
Mar 31 09:52:52 dc.ipa.test oidc_child[15036]: Result does not contain the 'verification_uri_complete' string.
Mar 31 09:52:53 dc.ipa.test oidc_child[15036]: oidc_child finished successful!
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: Received: [{"device_code":"VERY_LONG_STRING_LONGER_THAN_254_chars","expires_in":900,"interval":5}
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: oauth2 {"verification_uri": "https://microsoft.com/devicelogin", "user_code": "AZC5BCLEY"}
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ]
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: Failed to state to attribute set
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: Failed to handle device code reply.
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: sent: 0 data: 20
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: ..sent: 20 data: 20
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: ab@IPA.TEST: response sent: Access-Reject
Mar 31 09:52:53 dc.ipa.test ipa-otpd[15035]: Socket closed, shutting down...

@pbrezina pbrezina added the idp label Mar 31, 2022
@pbrezina pbrezina self-assigned this Mar 31, 2022
pbrezina added a commit to pbrezina/sssd that referenced this issue Apr 1, 2022
State attribute can be present only once (RFC-2865), but some IdPs can
return larger data. This patch switches to Proxy-State which make take
multiple values and concatenates these into single krb5_data like we
already do with Reply-Message.

Resolves: SSSD#6088
pbrezina added a commit to pbrezina/sssd that referenced this issue Apr 1, 2022
State attribute can be present only once (RFC-2865), but some IdPs can
return larger data. This patch switches to Proxy-State which make take
multiple values and concatenates these into single krb5_data like we
already do with Reply-Message.

Resolves: SSSD#6088
pbrezina added a commit to pbrezina/sssd that referenced this issue Apr 1, 2022
State attribute can be present only once (RFC-2865), but some IdPs can
return larger data. This patch switches to Proxy-State which make take
multiple values and concatenates these into single krb5_data like we
already do with Reply-Message.

Resolves: SSSD#6088
abbra added a commit to abbra/freeipa that referenced this issue Apr 5, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 5, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 5, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 5, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
@pbrezina
Copy link
Member

pbrezina commented Apr 8, 2022

Pushed PR: #6090

  • master
    • 74cb09e - krb5: idp method is only supported if FAST channel is available
    • 63e6365 - krb5: switch to Proxy-State in idp plugin reply
    • f853a86 - krb5: switch to Proxy-State in idp plugin

@pbrezina pbrezina added the Closed: Fixed Issue was closed as fixed. label Apr 8, 2022
abbra added a commit to abbra/freeipa that referenced this issue Apr 21, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 21, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 21, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
abbra added a commit to abbra/freeipa that referenced this issue Apr 25, 2022
For RADIUS protocol 'State' attribute value length must be 253
characters or less. Some IdPs produce states larger than this length.

In addition, 'State' attribute can only be present once in a RADIUS
packet. This means we cannot use 'State' to handle large IdP states.

Switch to use 'Proxy-State' instead. 'Proxy-State' is optional and can
be present multiple times.

Since we have control for both sides of the RADIUS communication here
(ipa-otpd and SSSD's KDC plugin for 'idp' preauth method), pass multiple
'Proxy-State' attributes with size below MAX_ATTRSIZE and reassemble a
state on the KDC side.

The same logic is already applied to 'Reply-Message' attribute.

Related: SSSD/sssd#6088
Fixes: https://pagure.io/freeipa/issue/8805

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed: Fixed Issue was closed as fixed. idp
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants