From 6dd28b20d395f7826a2da493d7fd1b0deed441fa Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Fri, 6 Apr 2018 10:46:33 +0200 Subject: [PATCH] fix get_issuer and get_in_response_to --- lasso/id-ff/login.c | 15 ++--- lasso/id-ff/profile.c | 8 +-- lasso/saml-2.0/profile.c | 9 ++- lasso/utils.h | 12 ++++ lasso/xml/private.h | 4 +- lasso/xml/tools.c | 134 +++++++++++++++++++++++++++------------ lasso/xml/xml.c | 8 +-- tests/basic_tests.c | 71 +++++++++++++++++++++ 8 files changed, 199 insertions(+), 62 deletions(-) diff --git a/lasso/id-ff/login.c b/lasso/id-ff/login.c index e59a15fc..a48f028f 100644 --- a/lasso/id-ff/login.c +++ b/lasso/id-ff/login.c @@ -1654,7 +1654,7 @@ gint lasso_login_init_request(LassoLogin *login, gchar *response_msg, LassoHttpMethod response_http_method) { - char **query_fields; + xmlChar **query_fields; gint ret = 0; int i; char *artifact_b64 = NULL, *provider_succinct_id_b64; @@ -1678,17 +1678,16 @@ lasso_login_init_request(LassoLogin *login, gchar *response_msg, /* rebuild response (artifact) */ if (response_http_method == LASSO_HTTP_METHOD_REDIRECT) { - query_fields = urlencoded_to_strings(response_msg); + query_fields = lasso_urlencoded_to_strings(response_msg); for (i=0; query_fields[i]; i++) { - if (strncmp(query_fields[i], "SAMLart=", 8) == 0) { - lasso_assign_string(artifact_b64, query_fields[i]+8); + if (strncmp((char*)query_fields[i], "SAMLart=", 8) == 0) { + lasso_assign_string(artifact_b64, (char*)query_fields[i]+8); } - if (strncmp(query_fields[i], "RelayState=", 11) == 0) { - lasso_assign_string(profile->msg_relayState, query_fields[i]+11); + if (strncmp((char*)query_fields[i], "RelayState=", 11) == 0) { + lasso_assign_string(profile->msg_relayState, (char*)query_fields[i]+11); } - xmlFree(query_fields[i]); } - lasso_release_string(query_fields); + lasso_release_array_of_xml_strings(query_fields); if (artifact_b64 == NULL) { return LASSO_PROFILE_ERROR_MISSING_ARTIFACT; } diff --git a/lasso/id-ff/profile.c b/lasso/id-ff/profile.c index 43d9fba0..af741251 100644 --- a/lasso/id-ff/profile.c +++ b/lasso/id-ff/profile.c @@ -838,7 +838,7 @@ lasso_profile_get_issuer(const char *message) char *result = NULL; int count = 0, ret; xmlChar *xml_result = NULL; - xmlChar *to_free = NULL; + char *to_free = NULL; reader = lasso_xmltextreader_from_message(message, &to_free); @@ -867,7 +867,7 @@ cleanup: if (reader) xmlFreeTextReader(reader); if (to_free) - lasso_release_xml_string(to_free); + lasso_release(to_free); return result; } @@ -887,7 +887,7 @@ lasso_profile_get_in_response_to(const char *message) int ret; int node_type = 0; xmlChar *xml_result = NULL; - xmlChar *to_free = NULL; + char *to_free = NULL; reader = lasso_xmltextreader_from_message(message, &to_free); @@ -913,7 +913,7 @@ cleanup: if (xml_result) lasso_release_xml_string(xml_result); if (to_free) - lasso_release_xml_string(to_free); + lasso_release(to_free); return result; } diff --git a/lasso/saml-2.0/profile.c b/lasso/saml-2.0/profile.c index 2de4937f..8171e79e 100644 --- a/lasso/saml-2.0/profile.c +++ b/lasso/saml-2.0/profile.c @@ -284,7 +284,7 @@ int lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, LassoProviderRole remote_provider_role, const char *msg, LassoHttpMethod method) { - char **query_fields; + xmlChar **query_fields; char *artifact_b64 = NULL; xmlChar *provider_succinct_id_b64 = NULL; char *provider_succinct_id[21]; @@ -296,14 +296,13 @@ lasso_saml20_profile_init_artifact_resolve(LassoProfile *profile, unsigned short index_endpoint = 0; if (method == LASSO_HTTP_METHOD_ARTIFACT_GET) { - query_fields = urlencoded_to_strings(msg); + query_fields = lasso_urlencoded_to_strings(msg); for (i=0; query_fields[i]; i++) { if (strncmp((char*)query_fields[i], LASSO_SAML2_FIELD_ARTIFACT "=", 8) == 0) { - lasso_assign_string(artifact_b64, query_fields[i]+8); + lasso_assign_string(artifact_b64, (char*)query_fields[i]+8); } - xmlFree(query_fields[i]); } - lasso_release(query_fields); + lasso_release_array_of_xml_strings(query_fields); if (artifact_b64 == NULL) { return LASSO_PROFILE_ERROR_MISSING_ARTIFACT; } diff --git a/lasso/utils.h b/lasso/utils.h index cb1491de..2dfba640 100644 --- a/lasso/utils.h +++ b/lasso/utils.h @@ -198,6 +198,18 @@ } \ } +#define lasso_release_array_of_xml_strings(dest) \ + { \ + xmlChar ***__tmp = &(dest);\ + if (*__tmp) {\ + int i = 0;\ + for (i = 0; (*__tmp)[i]; i++) {\ + lasso_release_xml_string((*__tmp)[i]);\ + }\ + lasso_release((*__tmp));\ + }\ + } + /* Assignment and list appending */ /* * lasso_assign_xxx macros ensure that you dot leak previous value of assigned things, they use diff --git a/lasso/xml/private.h b/lasso/xml/private.h index 6f7d911d..e989e270 100644 --- a/lasso/xml/private.h +++ b/lasso/xml/private.h @@ -230,7 +230,7 @@ char* lasso_sha384(const char *str); char* lasso_sha512(const char *str); -char** urlencoded_to_strings(const char *str); +xmlChar** lasso_urlencoded_to_strings(const char *str); int lasso_sign_node(xmlNode *xmlnode, LassoSignatureContext context, const char *id_attr_name, const char *id_value); @@ -335,7 +335,7 @@ void lasso_xmlnode_add_saml2_signature_template(xmlNode *node, LassoSignatureCon gchar* lasso_xmlnode_build_deflated_query(xmlNode *xmlnode); -xmlTextReader *lasso_xmltextreader_from_message(const char *message, xmlChar **to_free); +xmlTextReader *lasso_xmltextreader_from_message(const char *message, char **to_free); #ifdef __cplusplus } diff --git a/lasso/xml/tools.c b/lasso/xml/tools.c index 81e75b53..59ffe6ea 100644 --- a/lasso/xml/tools.c +++ b/lasso/xml/tools.c @@ -1119,24 +1119,33 @@ lasso_sha512(const char *str) return (char*)SHA512((unsigned char*)str, strlen(str), md); } -char** -urlencoded_to_strings(const char *str) + +/** + * lasso_urlencoded_to_strings: + * @str: a query string + * + * Parse a query string and separate it into an char* array of its components. + * + * The returned array must be deallocated. + */ +xmlChar** +lasso_urlencoded_to_strings(const char *str) { int i, n=1; char *st, *st2; - char **result; + xmlChar **result; + g_assert(str); /* count components */ st = (char*)str; while (*st) { if (*st == '&' || *st == ';') n++; - n++; st++; } /* allocate result array */ - result = g_new0(char*, n+1); + result = g_new0(xmlChar*, n+1); result[n] = NULL; /* tokenize */ @@ -1145,15 +1154,17 @@ urlencoded_to_strings(const char *str) while(1) { if (*st == '&' || *st == ';' || *st == '\0') { ptrdiff_t len = st - st2; + + g_assert(i < n+1); if (len) { - result[i] = xmlURIUnescapeString(st2, len, NULL); + result[i] = (xmlChar*)xmlURIUnescapeString(st2, len, NULL); } else { result[i] = g_malloc0(1); } i++; - st2 = st + 1; if (*st == '\0') break; + st2 = st + 1; } st++; } @@ -2900,24 +2911,64 @@ lasso_xmlnode_add_saml2_signature_template(xmlNode *node, LassoSignatureContext } } + +/** + * lasso_get_saml_message: + * @query_fields: a NULL terminated array of char* representing a parsed query string. + * + * Return the first SAMLRequest or SAMLResponse value in the query string array. + */ static char* -get_saml_message(char **query_fields) { - int i; - char *field, *t; +lasso_get_saml_message(xmlChar **query_fields) { + int i = 0; + char *enc = NULL; + char *message = NULL; + char *decoded_message = NULL; + xmlChar *field = NULL; + char *t = NULL; + int rc = 0; + int len = 0; for (i=0; (field=query_fields[i]); i++) { - t = strchr(field, '='); + t = strchr((char*)field, '='); if (t == NULL) continue; *t = 0; - if (strcmp(field, LASSO_SAML2_FIELD_ENCODING) == 0) { - return t+1; + if (strcmp((char*)field, LASSO_SAML2_FIELD_ENCODING) == 0) { + enc = t+1; + continue; } - if (strcmp(field, LASSO_SAML2_FIELD_REQUEST) == 0 || strcmp(field, LASSO_SAML2_FIELD_RESPONSE) == 0) { - return t+1; + if (strcmp((char*)field, LASSO_SAML2_FIELD_REQUEST) == 0 || strcmp((char*)field, LASSO_SAML2_FIELD_RESPONSE) == 0) { + message = t+1; + continue; } } - return NULL; + if (message == NULL) { + return NULL; + } + if (enc && strcmp(enc, LASSO_SAML2_DEFLATE_ENCODING) != 0) { + /* unknown encoding */ + debug("Unknown URL encoding: %64s", enc); + return NULL; + } + len = strlen(message); + decoded_message = g_malloc(len); + if (! is_base64(message)) { + debug("message is not base64"); + goto cleanup; + } + rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)decoded_message, len); + if (rc < 0) { + debug("could not decode redirect SAML message"); + goto cleanup; + } + /* rc contains the length of the result */ + message = (char*)lasso_inflate((unsigned char*) decoded_message, rc); +cleanup: + if (decoded_message) { + lasso_release(decoded_message); + } + return message; } /** @@ -2927,47 +2978,52 @@ get_saml_message(char **query_fields) { * Try to parse the passed message and create an xmlTextReader from it. */ xmlTextReader * -lasso_xmltextreader_from_message(const char *message, xmlChar **to_free) { +lasso_xmltextreader_from_message(const char *message, char **to_free) { size_t len = strlen(message); char *needle; - char **query_fields = NULL; + xmlChar **query_fields = NULL; char *decoded_message = NULL; xmlTextReader *reader = NULL; + g_assert(to_free); + /* Differentiate SOAP from others */ if (message[0] != '<') { needle = strchr(message, '='); - if (needle) { - ptrdiff_t needle_pos = (needle-message); - if (len - needle_pos > 2) { // query - query_fields = urlencoded_to_strings(needle+1); - message = get_saml_message(query_fields); - if (! message) - goto cleanup; - len = strlen(message); + /* Differentiate redirect binding from POST */ + if (needle && message[len-1] != '=') { + query_fields = lasso_urlencoded_to_strings(message); + message = *to_free = lasso_get_saml_message(query_fields); + len = strlen(message); + if (! message) { + goto cleanup; + } + } else { /* POST */ + int rc = 0; + + if (! is_base64(message)) { + debug("POST message is not base64"); + goto cleanup; } - } - if (is_base64(message)) { - int rc; decoded_message = g_malloc(len); rc = xmlSecBase64Decode((xmlChar*)message, (xmlChar*)decoded_message, len); - if (rc < 0) + if (rc < 0) { + debug("could not decode POST SAML message"); goto cleanup; + } len = rc; - message = (char*)lasso_inflate((unsigned char*) decoded_message, rc); - lasso_release_string(decoded_message); - if (! message) - goto cleanup; + decoded_message[len] = '\0'; + message = *to_free = decoded_message; + decoded_message = NULL; } } if (message[0] == '<') // XML case - reader = xmlReaderForMemory(message, len, "", NULL, XML_PARSE_NOENT | - XML_PARSE_NONET); + reader = xmlReaderForMemory(message, len, "", NULL, XML_PARSE_NONET); cleanup: if (query_fields) - lasso_release(query_fields); - if (decoded_message && to_free) - *to_free = BAD_CAST decoded_message; + lasso_release_array_of_xml_strings(query_fields); + if (decoded_message) + lasso_release_string(decoded_message); return reader; } diff --git a/lasso/xml/xml.c b/lasso/xml/xml.c index e99bb1fd..2f11bad3 100644 --- a/lasso/xml/xml.c +++ b/lasso/xml/xml.c @@ -724,20 +724,20 @@ gboolean lasso_node_init_from_query(LassoNode *node, const char *query) { LassoNodeClass *class; - char **query_fields; + xmlChar **query_fields; int i; gboolean rc; g_return_val_if_fail(LASSO_IS_NODE(node), FALSE); class = LASSO_NODE_GET_CLASS(node); - query_fields = urlencoded_to_strings(query); - rc = class->init_from_query(node, query_fields); + query_fields = lasso_urlencoded_to_strings(query); + rc = class->init_from_query(node, (char**)query_fields); for (i = 0; query_fields[i]; i++) { xmlFree(query_fields[i]); query_fields[i] = NULL; } - lasso_release(query_fields); + lasso_release_array_of_xml_strings(query_fields); return rc; } diff --git a/tests/basic_tests.c b/tests/basic_tests.c index c67aab15..8bdde3e8 100644 --- a/tests/basic_tests.c +++ b/tests/basic_tests.c @@ -2089,6 +2089,13 @@ START_TEST(test16_test_get_issuer) size_t len = 0; char *issuer = NULL; char *in_response_to = NULL; + LassoServer *spServerContext = NULL; + LassoServer *idpServerContext = NULL; + LassoLogin *spLoginContext = NULL; + LassoLogin *idpLoginContext = NULL; + LassoSamlp2AuthnRequest *request = NULL; + char *authnRequestUrl = NULL; + char *qs = NULL; g_file_get_contents(TESTSDATADIR "/response-1", &content, &len, NULL); issuer = lasso_profile_get_issuer(content); @@ -2098,6 +2105,70 @@ START_TEST(test16_test_get_issuer) check_str_equals(in_response_to, "xx"); lasso_release_string(in_response_to); lasso_release_string(content); + + spServerContext = lasso_server_new( + TESTSDATADIR "/sp5-saml2/metadata.xml", + TESTSDATADIR "/sp5-saml2/private-key.pem", + NULL, /* Secret key to unlock private key */ + NULL); + lasso_server_add_provider( + spServerContext, + LASSO_PROVIDER_ROLE_IDP, + TESTSDATADIR "/idp5-saml2/metadata.xml", + NULL, + NULL); + idpServerContext = lasso_server_new( + TESTSDATADIR "/idp5-saml2/metadata.xml", + TESTSDATADIR "/idp5-saml2/private-key.pem", + NULL, /* Secret key to unlock private key */ + NULL); + lasso_server_add_provider( + idpServerContext, + LASSO_PROVIDER_ROLE_SP, + TESTSDATADIR "/sp5-saml2/metadata.xml", + NULL, + NULL); + spLoginContext = lasso_login_new(spServerContext); + check_good_rc(lasso_login_init_authn_request(spLoginContext, "http://idp5/metadata", + LASSO_HTTP_METHOD_REDIRECT)); + request = LASSO_SAMLP2_AUTHN_REQUEST(LASSO_PROFILE(spLoginContext)->request); + request->IsPassive = 0; + lasso_assign_string(request->NameIDPolicy->Format, LASSO_SAML2_NAME_IDENTIFIER_FORMAT_PERSISTENT); + request->NameIDPolicy->AllowCreate = 1; + lasso_assign_string(request->ProtocolBinding, LASSO_SAML2_METADATA_BINDING_POST); + check_good_rc(lasso_login_build_authn_request_msg(spLoginContext)); + authnRequestUrl = LASSO_PROFILE(spLoginContext)->msg_url; + qs = index(authnRequestUrl, '?') + 1; + issuer = lasso_profile_get_issuer(qs); + check_true(lasso_strisequal(issuer, "http://sp5/metadata")); + lasso_release_string(issuer); + + idpLoginContext = lasso_login_new(idpServerContext); + + check_good_rc(lasso_login_process_authn_request_msg(idpLoginContext, qs)); + check_good_rc(lasso_login_validate_request_msg(idpLoginContext, 1, 0)); + check_good_rc(lasso_login_build_assertion(idpLoginContext, + LASSO_SAML_AUTHENTICATION_METHOD_PASSWORD, + "FIXME: authenticationInstant", + "FIXME: reauthenticateOnOrAfter", + "FIXME: notBefore", + "FIXME: notOnOrAfter")); + check_good_rc(lasso_login_build_authn_response_msg(idpLoginContext)); + check_not_null(idpLoginContext->parent.msg_body); + check_not_null(idpLoginContext->parent.msg_url); + + issuer = lasso_profile_get_issuer(idpLoginContext->parent.msg_body); + in_response_to = lasso_profile_get_in_response_to(idpLoginContext->parent.msg_body); + check_not_null(in_response_to); + check_str_equals(issuer, "http://idp5/metadata"); + check_str_equals(in_response_to, request->parent.ID); + lasso_release_string(issuer); + + lasso_release_gobject(idpLoginContext); + lasso_release_gobject(idpServerContext); + lasso_release_gobject(spLoginContext); + lasso_release_gobject(spServerContext); + } END_TEST