Bug 2982 - gssapi_cleanup: supported mechs should be freed via gss_release_oid_set
Summary: gssapi_cleanup: supported mechs should be freed via gss_release_oid_set
Status: RESOLVED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 7.9p1
Hardware: All Windows 10
: P5 normal
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_9_6
  Show dependency treegraph
 
Reported: 2019-03-15 20:37 AEDT by Markus Schmidt
Modified: 2023-10-12 13:22 AEDT (History)
2 users (show)

See Also:


Attachments
free 'supported mechs' through gss_release_oid_set() call (580 bytes, application/octet-stream)
2019-03-15 20:37 AEDT, Markus Schmidt
no flags Details
better version (989 bytes, patch)
2019-04-23 19:05 AEST, Markus Schmidt
no flags Details | Diff
use existing cleanup mechanism (593 bytes, patch)
2019-07-12 14:05 AEST, Damien Miller
no flags Details | Diff
cleanup GSSAPI mechanisms at end of authentication (915 bytes, patch)
2021-07-02 14:36 AEST, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Schmidt 2019-03-15 20:37:26 AEDT
Created attachment 3254 [details]
free 'supported mechs' through gss_release_oid_set() call

Attached is a small patch that should be applied before the 8.0 release.

It fixes a problem with a recent patch (authored by me), where gssapi_cleanup was introduced and gssapi resources are freed.

It turns out that the supported_mechs should not be just freed but instead freed through gss_release_oid_set.

The error is probably irrelevant in the *ix/bsd environments, but turned out to be an error under Windows if a dynamic lib (gssapi.dll) from MIT Kerbereros is used.
Comment 1 Markus Schmidt 2019-04-23 19:05:38 AEST
Created attachment 3269 [details]
better version


It turns out that the mechs should not only be freed via gss_release_oid_set.

I also found that that it would be better to do this at the end of the userauth2() function because gss_cleanup is called multiple times when more than one method is reported/tried.
Comment 2 Damien Miller 2019-07-12 14:05:17 AEST
Created attachment 3299 [details]
use existing cleanup mechanism

I think we should reuse the existing cleanup mechanism here. I don't have a working krb/gssapi installation ATM so I can't really test this though.
Comment 3 Markus Schmidt 2019-07-12 17:30:48 AEST
> I think we should reuse the existing cleanup mechanism here. I don't
> have a working krb/gssapi installation ATM so I can't really test
> this though.

I have debugged this and found that in some circumstances, gssapi_cleanup() is called multiple times (see comment1).

If the gssapi system reports multiple mechs it goes through the cycle init/cleanup for each mech.  

So the supported-mechs-list is on sort of a higher level than a single gssapi auth attempt.

Hence, releasing the mechs-list itself should not be done in the gssapi_cleanup function but at the very end of authentication.
Comment 4 Markus Schmidt 2019-07-12 17:32:59 AEST
(If you are not comfortable with this change, please go back to the old behavior (I think in 7.8 before gssapi_cleanup was introducted) and let the supported-mechs list leak instead.)
Comment 5 Damien Miller 2019-10-09 15:07:27 AEDT
Retarget these bugs to 8.2 release
Comment 6 Damien Miller 2020-02-04 11:44:23 AEDT
Prepare for 8.2 release; retarget bugs
Comment 7 Damien Miller 2020-05-08 13:39:16 AEST
Retarget bugs to 8.4 release
Comment 8 Damien Miller 2021-03-04 09:47:03 AEDT
retarget to 8.6
Comment 9 Damien Miller 2021-04-23 14:50:13 AEST
retarget after 8.6p1 release
Comment 10 Damien Miller 2021-07-02 14:36:41 AEST
Created attachment 3533 [details]
cleanup GSSAPI mechanisms at end of authentication
Comment 11 Damien Miller 2023-10-12 13:22:39 AEDT
This has been committed and will be in openssh-9.6, due around the end of the year. Thanks!