Currently error handling in pykerberos is weak, AUTH_GSS_ERROR is returned in almost any case. With this patch pykerberos throws exception with error messages and codes as returned by the GSS api functions. diff --git a/src/kerberos.c b/src/kerberos.c index 419827c..05cfac4 100644 --- a/src/kerberos.c +++ b/src/kerberos.c @@ -21,6 +21,10 @@ #include #include "kerberosbasic.h" #include "kerberosgss.h" +PyObject *KrbException_class; +PyObject *BasicAuthException_class; +PyObject *GssException_class; + static PyObject *checkPassword(PyObject *self, PyObject *args) { const char *user; @@ -37,7 +41,7 @@ static PyObject *checkPassword(PyObject if (result) return Py_INCREF(Py_True), Py_True; else - return Py_INCREF(Py_False), Py_False; + return NULL; } static PyObject *authGSSClientInit(PyObject *self, PyObject *args) @@ -54,6 +58,9 @@ static PyObject *authGSSClientInit(PyObj pystate = PyCObject_FromVoidPtr(state, NULL); result = authenticate_gss_client_init(service, state); + if(result == AUTH_GSS_ERROR) { + return NULL; + } return Py_BuildValue("(iO)", result, pystate); } @@ -82,8 +89,8 @@ static PyObject *authGSSClientClean(PyOb static PyObject *authGSSClientStep(PyObject *self, PyObject *args) { gss_client_state *state; - PyObject *pystate; - char *challenge; + PyObject *pystate; + char *challenge; int result = 0; if (!PyArg_ParseTuple(args, "Os", &pystate, &challenge) || !PyCObject_Check(pystate)) @@ -94,6 +101,9 @@ static PyObject *authGSSClientStep(PyObj return NULL; result = authenticate_gss_client_step(state, challenge); + if(result == AUTH_GSS_ERROR) { + return NULL; + } return Py_BuildValue("i", result); } @@ -142,6 +152,9 @@ static PyObject *authGSSServerInit(PyObj pystate = PyCObject_FromVoidPtr(state, NULL); result = authenticate_gss_server_init(service, state); + if(result == AUTH_GSS_ERROR) { + return NULL; + } return Py_BuildValue("(iO)", result, pystate); } @@ -182,6 +195,9 @@ static PyObject *authGSSServerStep(PyObj return NULL; result = authenticate_gss_server_step(state, challenge); + if(result == AUTH_GSS_ERROR) { + return NULL; + } return Py_BuildValue("i", result); } @@ -216,7 +232,7 @@ static PyObject *authGSSServerUserName(P return Py_BuildValue("s", state->username); } -static PyMethodDef SpamMethods[] = { +static PyMethodDef KerberosMethods[] = { {"checkPassword", checkPassword, METH_VARARGS, "Check the supplied user/password against Kerberos KDC."}, {"authGSSClientInit", authGSSClientInit, METH_VARARGS, @@ -242,7 +258,35 @@ static PyMethodDef SpamMethods[] = { {NULL, NULL, 0, NULL} /* Sentinel */ }; + PyMODINIT_FUNC initkerberos(void) { - (void) Py_InitModule("kerberos", SpamMethods); + PyObject *m,*d; + + m=Py_InitModule("kerberos", KerberosMethods); + + d = PyModule_GetDict(m); + /* create the base exception class */ + if(!(KrbException_class = PyErr_NewException("kerberos.KrbError", NULL, NULL))) + goto error; + PyDict_SetItemString( d, "KrbError", KrbException_class); + Py_INCREF(KrbException_class); + + /* ...and the derived exceptions */ + if(!(BasicAuthException_class = PyErr_NewException("kerberos.BasicAuthError", KrbException_class, NULL))) + goto error; + Py_INCREF(BasicAuthException_class); + PyDict_SetItemString( d, "BasicAuthError", BasicAuthException_class); + + if(!(GssException_class = PyErr_NewException("kerberos.GSSError", KrbException_class, NULL))) + goto error; + Py_INCREF(GssException_class); + PyDict_SetItemString( d, "GSSError", GssException_class); + +error: + if (PyErr_Occurred()) + { + PyErr_SetString(PyExc_ImportError, "kerberos: init failed"); + } } + diff --git a/src/kerberosbasic.c b/src/kerberosbasic.c index faa91e2..3b20704 100644 --- a/src/kerberosbasic.c +++ b/src/kerberosbasic.c @@ -16,6 +16,7 @@ * DRI: Cyrus Daboo, cdaboo@apple.com **/ +#include #include "kerberosbasic.h" #include @@ -24,6 +25,9 @@ #include #undef PRINTFS +extern PyObject *BasicAuthException_class; +static void set_basicauth_error(krb5_context context, krb5_error_code code); + static krb5_error_code verify_krb5_user(krb5_context context, krb5_principal principal, const char *password, krb5_principal server); int authenticate_user_krb5pwd(const char *user, const char *pswd, const char *service, const char *default_realm) @@ -39,9 +43,8 @@ int authenticate_user_krb5pwd(const char code = krb5_init_context(&kcontext); if (code) { -#ifdef PRINTFS - printf("Cannot initialize Kerberos5 context (%d)\n", code); -#endif + PyErr_SetObject(BasicAuthException_class, Py_BuildValue("((s:i))", + "Cannot initialize Kerberos5 context", code)); return 0; } @@ -49,9 +52,7 @@ #endif if (ret) { -#ifdef PRINTFS - printf("Error parsing server name (%s): %s\n", service, krb5_get_err_text(kcontext, ret)); -#endif + set_basicauth_error(kcontext, ret); ret = 0; goto end; } @@ -59,9 +60,7 @@ #endif code = krb5_unparse_name(kcontext, server, &name); if (code) { -#ifdef PRINTFS - printf("krb5_unparse_name() failed: %s\n", krb5_get_err_text(kcontext, code)); -#endif + set_basicauth_error(kcontext, code); ret = 0; goto end; } @@ -85,9 +84,7 @@ #endif code = krb5_parse_name(kcontext, name, &client); if (code) { -#ifdef PRINTFS - printf("krb5_parse_name() failed: %s\n", krb5_get_err_text(kcontext, code)); -#endif + set_basicauth_error(kcontext, code); ret = 0; goto end; } @@ -113,7 +110,6 @@ #endif if (server) krb5_free_principal(kcontext, server); krb5_free_context(kcontext); - return ret; } @@ -139,9 +135,7 @@ #endif ret = krb5_get_init_creds_password(context, &creds, principal, (char *)password, NULL, NULL, 0, NULL, NULL); if (ret) { -#ifdef PRINTFS - printf("krb5_get_init_creds_password() failed: %s\n", krb5_get_err_text(context, ret)); -#endif + set_basicauth_error(context, ret); goto end; } @@ -151,3 +145,7 @@ end: return ret; } +static void set_basicauth_error(krb5_context context, krb5_error_code code) +{ + PyErr_SetObject(BasicAuthException_class, Py_BuildValue("(s:i)", krb5_get_err_text(context, code), code)); +} diff --git a/src/kerberosgss.c b/src/kerberosgss.c index 7200786..fb5129d 100644 --- a/src/kerberosgss.c +++ b/src/kerberosgss.c @@ -16,6 +16,7 @@ * DRI: Cyrus Daboo, cdaboo@apple.com **/ +#include #include "kerberosgss.h" #include "base64.h" @@ -24,17 +25,15 @@ #include #include #include -#undef PRINTFS +static void set_gss_error(OM_uint32 err_maj, OM_uint32 err_min); -static const char *get_gss_error(char *p, int psize, OM_uint32 err_maj, OM_uint32 err_min, char *prefix); +extern PyObject *GssException_class; +extern PyObject *KrbException_class; int authenticate_gss_client_init(const char* service, gss_client_state *state) { OM_uint32 maj_stat; OM_uint32 min_stat; -#ifdef PRINTFS - char buf[1024]; -#endif gss_buffer_desc name_token = GSS_C_EMPTY_BUFFER; int ret = AUTH_GSS_COMPLETE; @@ -45,15 +44,13 @@ #endif // Import server name first name_token.length = strlen(service); - name_token.value = (char *)service; + name_token.value = (char *)service; maj_stat = gss_import_name(&min_stat, &name_token, gss_krb5_nt_service_name, &state->server_name); if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_import_name() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -90,9 +87,6 @@ int authenticate_gss_client_step(gss_cli { OM_uint32 maj_stat; OM_uint32 min_stat; -#ifdef PRINTFS - char buf[1024]; -#endif gss_buffer_desc input_token = GSS_C_EMPTY_BUFFER; gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER; int ret = AUTH_GSS_CONTINUE; @@ -129,9 +123,7 @@ #endif if ((maj_stat != GSS_S_COMPLETE) && (maj_stat != GSS_S_CONTINUE_NEEDED)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_init_sec_context() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -151,9 +143,7 @@ #endif maj_stat = gss_inquire_context(&min_stat, state->context, &gssuser, NULL, NULL, NULL, NULL, NULL, NULL); if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_inquire_context() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -167,9 +157,7 @@ #endif gss_release_buffer(&min_stat, &name_token); gss_release_name(&min_stat, &gssuser); -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_display_name() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -194,9 +182,6 @@ int authenticate_gss_server_init(const c { OM_uint32 maj_stat; OM_uint32 min_stat; -#ifdef PRINTFS - char buf[1024]; -#endif gss_buffer_desc name_token = GSS_C_EMPTY_BUFFER; int ret = AUTH_GSS_COMPLETE; @@ -216,9 +201,7 @@ #endif if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_import_name() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -229,9 +212,7 @@ #endif if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_acquire_cred() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -274,9 +255,6 @@ int authenticate_gss_server_step(gss_ser { OM_uint32 maj_stat; OM_uint32 min_stat; -#ifdef PRINTFS - char buf[1024]; -#endif gss_buffer_desc input_token = GSS_C_EMPTY_BUFFER; gss_buffer_desc output_token = GSS_C_EMPTY_BUFFER; int ret = AUTH_GSS_CONTINUE; @@ -297,9 +275,7 @@ #endif } else { -#ifdef PRINTFS - printf("No challenge parameter in request from client\n"); -#endif + PyErr_SetString(KrbException_class, "No challenge parameter in request from client"); ret = AUTH_GSS_ERROR; goto end; } @@ -318,9 +294,7 @@ #endif if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_accept_sec_context() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -335,9 +309,7 @@ #endif maj_stat = gss_display_name(&min_stat, state->client_name, &output_token, NULL); if (GSS_ERROR(maj_stat)) { -#ifdef PRINTFS - printf("%s\n", get_gss_error(buf, 1024, maj_stat, min_stat, "gss_display_name() failed")); -#endif + set_gss_error(maj_stat, min_stat); ret = AUTH_GSS_ERROR; goto end; } @@ -355,13 +327,15 @@ end: return ret; } -const char *get_gss_error(char *p, int psize, OM_uint32 err_maj, OM_uint32 err_min, char *prefix) + +static void set_gss_error(OM_uint32 err_maj, OM_uint32 err_min) { OM_uint32 maj_stat, min_stat; OM_uint32 msg_ctx = 0; gss_buffer_desc status_string; + char buf_maj[512]; + char buf_min[512]; - strncpy(p, prefix, psize); do { maj_stat = gss_display_status (&min_stat, @@ -372,8 +346,7 @@ const char *get_gss_error(char *p, int p &status_string); if (GSS_ERROR(maj_stat)) break; - strncat(p, ": ", psize); - strncat(p, (char*) status_string.value, psize); + strncpy(buf_maj, (char*) status_string.value, sizeof(buf_maj)); gss_release_buffer(&min_stat, &status_string); maj_stat = gss_display_status (&min_stat, @@ -384,12 +357,11 @@ const char *get_gss_error(char *p, int p &status_string); if (!GSS_ERROR(maj_stat)) { - strncat(p, " (", psize); - strncat(p, (char*) status_string.value, psize); - strncat(p, ")", psize); + strncpy(buf_min, (char*) status_string.value, sizeof(buf_min)); gss_release_buffer(&min_stat, &status_string); } } while (!GSS_ERROR(maj_stat) && msg_ctx != 0); - return p; + PyErr_SetObject(GssException_class, Py_BuildValue("((s:i)(s:i))", buf_maj, err_maj, buf_min, err_min)); } + diff --git a/test.py b/test.py index 311593e..88f9d7e 100644 --- a/test.py +++ b/test.py @@ -28,10 +28,11 @@ def main(): pswd = "" service = "http@caldav.apple.com" host = "localhost" + realm ="APPLECONNECT.APPLE.COM" port = 8008 ssl = False - options, args = getopt.getopt(sys.argv[1:], "u:p:s:h:i:") + options, args = getopt.getopt(sys.argv[1:], "u:p:s:h:i:r:") for option, value in options: if option == "-u": @@ -44,11 +45,13 @@ def main(): host = value elif option == "-i": port = value + elif option == "-r": + realm = value # Run tests if (len(user) != 0) and (len(pswd) != 0): print "\n*** Running basic test" - testCheckpassword(user, pswd) + testCheckpassword(user, pswd, service, realm) else: print "\n*** Skipping basic test: no user or password specified" @@ -60,12 +63,13 @@ def main(): print "\n*** Done\n" -def testCheckpassword(user, pswd): - result = kerberos.checkPassword(user, pswd, "http/web.apple.com", "APPLECONNECT.APPLE.COM") - if result: - print "Kerberos authentication for %s succeeded" % user +def testCheckpassword(user, pswd, service, realm): + try: + kerberos.checkPassword(user, pswd, service, realm) + except kerberos.BasicAuthError, e: + print "Kerberos authentication for %s failed: %s" % (user, e[0]) else: - print "Kerberos authentication for %s failed" % user + print "Kerberos authentication for %s succeeded" % user def testGSSAPI(service): def statusText(r): @@ -148,15 +152,17 @@ def testHTTP(host, port, ssl, service): print "Incorrect www-authenticate header in initial HTTP response: %s" % hdr return - rc, vc = kerberos.authGSSClientInit(service); - if rc != 1: - print "Could not initialize GSSAPI" - return + try: + rc, vc = kerberos.authGSSClientInit(service); + except kerberos.GSSError, e: + print "Could not initialize GSSAPI: %s/%s" % (e[0][0], e[1][0]) + return - rc = kerberos.authGSSClientStep(vc, ""); - if rc != 0: - print "Could not do GSSAPI setp with continue" - return + try: + kerberos.authGSSClientStep(vc, ""); + except kerberos.GSSError, e: + print "Could not do GSSAPI setp with continue: %s/%s" % (e[0][0], e[1][0]) + return hdrs = {} hdrs["Authorization"] = "negotiate %s" % kerberos.authGSSClientResponse(vc) @@ -169,7 +175,7 @@ def testHTTP(host, port, ssl, service): return if response.status/100 != 2: - print "Second HTTP request did not result in a 2xx response" + print "Second HTTP request did not result in a 2xx response: %d" % (response.status,) return hdrs = response.msg.getheaders("www-authenticate") @@ -177,22 +183,24 @@ def testHTTP(host, port, ssl, service): print "No www-authenticate header in second HTTP response." return if len(hdrs) != 1: - print "Too many www-authenticate headers in second HTTP response." + print "Too many www-authenticate headers in second HTTP response: %d." % (len(hdrs),) return hdr = hdrs[0].strip() splits = hdr.split(' ', 1) if (len(splits) != 2) or (splits[0].lower() != "negotiate"): - print "Incorrect www-authenticate header in second HTTP response: %s" % hdr + print "Incorrect www-authenticate header in second HTTP response: %s" % hdr return - rc = kerberos.authGSSClientStep(vc, splits[1]); - if rc != 1: - print "Could not verify server www-authenticate header in second HTTP response" + try: + kerberos.authGSSClientStep(vc, splits[1]) + except kerberos.GSSError, e: + print "Could not verify server www-authenticate header in second HTTP response: %s/%s" % (e[0][0], e[1][0]) return - rc = kerberos.authGSSClientClean(vc); - if rc != 1: - print "Could not clean-up GSSAPI" + try: + rc = kerberos.authGSSClientClean(vc); + except kerberos.GSSError, e: + print "Could not clean-up GSSAPI: %s/%s" % (e[0][0], e[1][0]) return return