Hi Max,

for those sections I have dropped I consider them resovled or some action wil be taken from you with a followup webrev and a JBS issue.

A general note on trace output: I found it really helpful, don't remove it!

Am 2020-04-21 um 15:36 schrieb Weijun Wang:


On Apr 2, 2020, at 12:21 AM, Osipov, Michael <michael.osi...@siemens.com> wrote:

Hi Max,

at last I took some time to evaluate you SSPI bridge. This is part one. 
Interaction evaluation will follow in a second email.

Assumptions:
* All methods, objects behave the same as with JGSS
* AcceptSecurityContext is not implemented so should everything associated with 
it fail (GSSException)
* Code analysis happens based on 
https://github.com/AdoptOpenJDK/openjdk-jdk13u/blob/master/src/java.security.jgss/windows/native/libsspi_bridge/sspi.cpp
and zulu13.29.9-ca-jdk13.0.2-win_x64

Findings C:

* sspi.cpp#L67-L68: Feels a bit awkward to permit SSPI_BRIDGE_TRACE="". Why not have "if 
(trace && *trace)"?

I think it's harmless but if every C program has that style I can follow.

Yes, it is harmless, but somewhat counter intuitive from a client perspective.

* sspi.cpp#L233: The function name says "show_oid", but I see no OID printed. 
Simply symbolic names.

Yes, it should be more like "what is this OID?". I'll think of a better name.

Exactly!

* sspi.cpp#L290: Empty string check? like (realm && *realm)

But then it's still L"".

My bad, misunderstanding.

* sspi.cpp#L482-L483: This is highly questionable. While it is true for 
Windows, it does not stick to gss_compare_name behavior. Don't know what the 
correct approach would be here.

I'll be glad if it's always true for Windows. I found the name thing most 
complicated, and here it's still only simple name and not UPN...

I would really not use the Windows approach here since you use GSS-API. Moreover, this is suspicious too:
one has no realm and
    // one has realm but they have the same name

It would consider the following equal while they aren't:
default realm: AD001.EXAMPLE.COM
gss_name_compare("michael-o", "michae...@ad002.example.com") == 1

* sspi.cpp#L618: You are resetting cred_usage passed with the function. This 
looks like a bug to me.

It's more like I ignore it. Will double check.

You really should not ignore this. It should at least match JGSS.
Tighten to MIT Kerberos behavior: https://github.com/krb5/krb5/blob/6d4eb6eb473c93f0db05409195448364382760a9/src/lib/gssapi/mechglue/g_acquire_cred.c#L73-L78?

  Same as in GSS-API:
$ sudo python3
Python 3.7.7 (default, Mar 19 2020, 21:26:00)
[Clang 9.0.1 (g...@github.com:llvm/llvm-project.git 
c1a0a213378a458fbea1a5c77b31 on freebsd12
Type "help", "copyright", "credits" or "license" for more information.
import gssapi
cred = gssapi.Credentials(usage='nonsense')
cred.usage
'both'
This also may be a bug in py-gssapi/MIT Kerberos

Reported for py-gssapi: https://github.com/pythongssapi/python-gssapi/issues/202

* sspi.cpp#L149
** Can you apply a better output like ISO 8601? In strftime(3) that would be "%FT%T, 
..."

I'll check, but the current format is most readable for me.

Really? I though that ISO 8601 style is most pleasant in Asia. Besides that, since the time is far in the future, does it make sense at all to have show_time()? I consider expiry quite pointless with such high values.

* sspi.cpp#L841: man 3 gss_import_sec_context says: GSS_S_UNAVAILABLE

I'm using GSS_S_FAILURE everywhere. I know it's not precise.

Can you file this as an issue. For a novice user this makes it really really hard to tackle down issue especially behavior you have zero debugging with SSPI compared to KRB5_TRACE.

* sspi.cpp#L977-L979: Not helpful when the SEC_E_* not mapped to major/minor. 
This likely applies to most mapping functions.

???

This is bascially the same as the one above. you map 'ss' simply to GSS_S_FAILURE. Little context here.

* sspi.cpp#L1046: Maybe like gss_export_sec_context?

Not sure what you want me to do.

* sspi.cpp#L1169: Maybe like gss_export_sec_context?

???


My bad, I gave to little context. The manpage for gss_export_sec_context() allows to return GSS_S_UNAVAILABLE. I would expect this return value for both of them.

* sspi.cpp#L729-L748: Why do you do this? The documentation for parameter 1 
says: If the process that requests the handle does not have access to the 
credentials, the function returns an error. I have verified this with py-win32: 
win32security.AcquireCredentialsHandle(). Infact, it accepts any principal and 
always returns the default one. I found these:
** https://github.com/twosigma/gsskrb5/blob/master/krb5/krb5cred.c#L127-L132
** https://github.com/twosigma/gsskrb5/blob/master/krb5/krb5cred.c#L127-L132
cred, time = win32security.AcquireCredentialsHandle("administra...@oracle.com", 
"Kerberos", win32security.SECPKG_CRED_OUTBOUND, None, None)
cred.QueryCredentialsAttributes(1)
'osipo...@ad001.siemens.net'

Is this the expected result?

Yes. You cannot acquire a cred handle for another identity unless you do protocol transition or pass the auth_identity struct which makes pszPrincipal obsolete anyway. See also here: https://github.com/requests/requests-kerberos/pull/75#issuecomment-223657799 and https://groups.google.com/d/msg/microsoft.public.platformsdk.security/Xlo23EMiH9U/r1tNRXfv08QJ

The Kerberos SSP will ignore this value. You never have access to other TGTs, this is by design.

This maybe different for kernel space, but this is just a wild guess.

Findings Java:
* GssManager#createCredential() with ACCEPT_ONLY or INITIATE_AND_ACCEPT gives 
me weird credentials with partial null nembers. I'd expect an exception here.

Oops. Will look into. Maybe I should reject.

Definitvely.

* This is one fails, but shall work:
GSSManager manager = GSSManager.getInstance();
GSSName userName = manager.createName("osipovmi", GSSName.NT_USER_NAME);
GSSCredential cred = manager.createCredential(userName, 
GSSCredential.DEFAULT_LIFETIME, krb5Oid, GSSCredential.INITIATE_ONLY);
[SSPI:1627] >>>> Calling gss_create_empty_oid_set...
[SSPI:1542] >>>> Calling gss_add_oid_set_member...
[SSPI:612] >>>> Calling gss_acquire_cred...
[SSPI:619] AcquireCredentialsHandle with 0 00000257FFB199B0
[SSPI:262] gss_OID_set.count is 1
[SSPI:237] Kerberos mech
[SSPI:628] reqKerberos
[SSPI:152] cred expiration: 09/13/30828  04:48 4294967295ld
[SSPI:732] Acquiring cred with a name. Check if it's me.
[SSPI:791] >>>> Calling gss_inquire_cred...
[SSPI:811] Allocate new name at 00000257FFB2BC40
[SSPI:428] >>>> Calling gss_compare_name...
[SSPI:437] Comparing osipo...@ad001.siemens.net and osipovmi
[SSPI:325] >>>> Calling gss_release_name 00000257FFB17830...
[SSPI:744] Comparing result: 0
[SSPI:767] >>>> Calling gss_release_cred...
[SSPI:1641] >>>> Calling gss_release_oid_set...
Exception in thread "main" GSSException: Failure unspecified at GSS-API level
        at 
java.security.jgss/sun.security.jgss.wrapper.GSSLibStub.acquireCred(Native 
Method)

The problem is that I provide a local name and expect the default realm to be 
used. It seems like #createCredential() does not take that into account. It 
also has no avail when the canonicalized form is used. See sspi.cpp#L729-L748.

Open one...

I will work somewhere next week on part 2 of the analysis.

Regards,

Michael

Reply via email to