You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com> on 2008/03/17 09:51:42 UTC
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
2008-03-17 02:27 lgo@tigris.org <lg...@tigris.org> napisał(a):
> Author: lgo
> Date: Sun Mar 16 18:27:54 2008
> New Revision: 29939
>
> Log:
> ra_serf: add SSL server certificate validation. This adds a callback to
> override the default algorithm. Validation is based on subject & issuer,
> matching hostname (pattern) and certificate expiration dates. Any pre-installed
> CA certificates can be used. Loading custom CA certificates is not implemented
> yet.
>
> Note: if you're building svn with serf trunk, updating serf to r1176 is needed.
>
> * subversion/libsvn_ra_serf/ra_serf.h
> (struct svn_ra_serf__session_t): Store config information to setup serf.
> * subversion/include/svn_error_codes.h
> (SVN_ERR_RA_SERF_SSL_CERT_UNTRUSTED): New error code.
> * subversion/libsvn_ra_serf/serf.c
> (load_config): Load the ssl server certificate config options.
> * subversion/libsvn_ra_serf/util.c
> (serf_failure_map[],
> ssl_convert_serf_failures): Map serf failure codes to subversion codes.
> (convert_organisation_to_str): Convert subject or issuer to readable text.
> (ssl_server_cert): Callback function, allowing override of the default
> server certificate validation algorithm.
> (svn_ra_serf__conn_setup): Make serf use our callback.
>
> Modified:
> trunk/subversion/include/svn_error_codes.h
> trunk/subversion/libsvn_ra_serf/ra_serf.h
> trunk/subversion/libsvn_ra_serf/serf.c
> trunk/subversion/libsvn_ra_serf/util.c
>
> Modified: trunk/subversion/include/svn_error_codes.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_error_codes.h?pathrev=29939&r1=29938&r2=29939
> ==============================================================================
> --- trunk/subversion/include/svn_error_codes.h Sun Mar 16 13:26:00 2008 (r29938)
> +++ trunk/subversion/include/svn_error_codes.h Sun Mar 16 18:27:54 2008 (r29939)
> @@ -841,6 +841,10 @@ SVN_ERROR_START
> SVN_ERRDEF(SVN_ERR_RA_SERF_SSPI_INITIALISATION_FAILED,
> SVN_ERR_RA_SERF_CATEGORY_START + 0,
> "Initialization of SSPI library failed")
> + /** @since New in 1.5. */
> + SVN_ERRDEF(SVN_ERR_RA_SERF_SSL_CERT_UNTRUSTED,
> + SVN_ERR_RA_SERF_CATEGORY_START + 1,
> + "Server SSL certificate untrusted")
>
> /* libsvn_auth errors */
>
>
> Modified: trunk/subversion/libsvn_ra_serf/ra_serf.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/ra_serf.h?pathrev=29939&r1=29938&r2=29939
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/ra_serf.h Sun Mar 16 13:26:00 2008 (r29938)
> +++ trunk/subversion/libsvn_ra_serf/ra_serf.h Sun Mar 16 18:27:54 2008 (r29939)
> @@ -194,6 +194,10 @@ struct svn_ra_serf__session_t {
> const char *proxy_username;
> const char *proxy_password;
> int proxy_auth_attempts;
> +
> + /* SSL server certificates */
> + svn_boolean_t trust_default_ca;
> + const char *ssl_authorities;
> };
>
> /*
>
> Modified: trunk/subversion/libsvn_ra_serf/serf.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/serf.c?pathrev=29939&r1=29938&r2=29939
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/serf.c Sun Mar 16 13:26:00 2008 (r29938)
> +++ trunk/subversion/libsvn_ra_serf/serf.c Sun Mar 16 18:27:54 2008 (r29939)
> @@ -365,6 +365,13 @@ load_config(svn_ra_serf__session_t *sess
> SVN_CONFIG_OPTION_HTTP_PROXY_USERNAME, NULL);
> svn_config_get(config, &session->proxy_password, SVN_CONFIG_SECTION_GLOBAL,
> SVN_CONFIG_OPTION_HTTP_PROXY_PASSWORD, NULL);
> + /* Load the global ssl settings, if set. */
> + SVN_ERR(svn_config_get_bool(config, &session->trust_default_ca,
> + SVN_CONFIG_SECTION_GLOBAL,
> + SVN_CONFIG_OPTION_SSL_TRUST_DEFAULT_CA,
> + TRUE));
> + svn_config_get(config, &session->ssl_authorities, SVN_CONFIG_SECTION_GLOBAL,
> + SVN_CONFIG_OPTION_SSL_AUTHORITY_FILES, NULL);
> #endif
>
> server_group = svn_config_find_group(config,
> @@ -390,6 +397,14 @@ load_config(svn_ra_serf__session_t *sess
> SVN_CONFIG_OPTION_HTTP_PROXY_USERNAME, NULL);
> svn_config_get(config, &session->proxy_password, server_group,
> SVN_CONFIG_OPTION_HTTP_PROXY_PASSWORD, NULL);
> +
> + /* Load the group ssl settings. */
> + SVN_ERR(svn_config_get_bool(config, &session->trust_default_ca,
> + server_group,
> + SVN_CONFIG_OPTION_SSL_TRUST_DEFAULT_CA,
> + TRUE));
> + svn_config_get(config, &session->ssl_authorities, server_group,
> + SVN_CONFIG_OPTION_SSL_AUTHORITY_FILES, NULL);
> #endif
> }
>
>
> Modified: trunk/subversion/libsvn_ra_serf/util.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/util.c?pathrev=29939&r1=29938&r2=29939
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 13:26:00 2008 (r29938)
> +++ trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 18:27:54 2008 (r29939)
> @@ -21,6 +21,7 @@
> #define APR_WANT_STRFUNC
> #include <apr.h>
> #include <apr_want.h>
> +#include <apr_fnmatch.h>
>
> #include <serf.h>
> #include <serf_bucket_types.h>
> @@ -41,6 +42,145 @@
> #define XML_STATUS_ERROR 0
> #endif
>
> +static const apr_uint32_t serf_failure_map[][2] =
> +{
> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
> +};
> +
> +/* Convert neon's SSL failure mask to our own failure mask. */
"neon"
> +static apr_uint32_t
> +ssl_convert_serf_failures(int failures)
> +{
> + apr_uint32_t svn_failures = 0;
> + apr_size_t i;
> +
> + for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
> + {
> + if (failures & serf_failure_map[i][0])
> + {
> + svn_failures |= serf_failure_map[i][1];
> + failures &= ~serf_failure_map[i][0];
> + }
> + }
> +
> + /* Map any remaining failure bits to our OTHER bit. */
> + if (failures)
> + {
> + svn_failures |= SVN_AUTH_SSL_OTHER;
> + }
> +
> + return svn_failures;
> +}
> +
> +static char *
> +convert_organisation_to_str(apr_hash_t *org, apr_pool_t *pool)
> +{
> + char *str;
> +
> + str = apr_psprintf(pool, "%s, %s, %s, %s, %s (%s)",
> + (char*)apr_hash_get(org, "OU", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "O", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "L", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "ST", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "C", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "E", APR_HASH_KEY_STRING));
> +
> + return str;
> +}
> +
> +static apr_status_t
> +ssl_server_cert(void *baton, int failures,
> + const serf_ssl_certificate_t *cert)
> +{
> + svn_ra_serf__connection_t *conn = baton;
> + apr_pool_t *subpool;
> + svn_auth_ssl_server_cert_info_t cert_info;
> + svn_auth_cred_ssl_server_trust_t *server_creds = NULL;
> + svn_auth_iterstate_t *state;
> + const char *realmstring;
> + apr_uint32_t svn_failures;
> + svn_error_t *err;
> + apr_hash_t *issuer, *subject, *serf_cert;
> + void *creds;
> +
> + apr_pool_create(&subpool, conn->session->pool);
> +
> + /* Construct the realmstring, e.g. https://svn.collab.net:443 */
> + realmstring = apr_uri_unparse(subpool,
> + &conn->session->repos_url,
> + APR_URI_UNP_OMITPATHINFO);
> +
> + /* Extract the info from the certificate */
> + subject = serf_ssl_cert_subject(cert, subpool);
> + issuer = serf_ssl_cert_issuer(cert, subpool);
> + serf_cert = serf_ssl_cert_certificate(cert, subpool);
> +
> + cert_info.hostname = apr_hash_get(subject, "CN", APR_HASH_KEY_STRING);
> + cert_info.fingerprint = apr_hash_get(serf_cert, "sha1", APR_HASH_KEY_STRING);
> + if (! cert_info.fingerprint)
> + cert_info.fingerprint = apr_pstrdup(subpool, "<unknown>");
> + cert_info.valid_from = apr_hash_get(serf_cert, "notBefore",
> + APR_HASH_KEY_STRING);
> + if (! cert_info.valid_from)
> + cert_info.valid_from = apr_pstrdup(subpool, "[invalid date]");
> + cert_info.valid_until = apr_hash_get(serf_cert, "notAfter",
> + APR_HASH_KEY_STRING);
> + if (! cert_info.valid_until)
> + cert_info.valid_until = apr_pstrdup(subpool, "[invalid date]");
> + cert_info.issuer_dname = convert_organisation_to_str(issuer, subpool);
> + cert_info.ascii_cert = "ce"; //ascii_cert;
> +
> + svn_failures = ssl_convert_serf_failures(failures);
> +
> + /* Match server certificate CN with the hostname of the server */
> + if (cert_info.hostname)
> + {
> + if (apr_fnmatch(cert_info.hostname, conn->hostinfo,
> + APR_FNM_PERIOD) == APR_FNM_NOMATCH)
> + {
> + svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
> + }
> + }
> +
> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
> + SVN_AUTH_PARAM_SSL_SERVER_FAILURES,
> + &svn_failures);
> +
> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
> + SVN_AUTH_PARAM_SSL_SERVER_CERT_INFO,
> + &cert_info);
> +
> + err = svn_auth_first_credentials(&creds, &state,
> + SVN_AUTH_CRED_SSL_SERVER_TRUST,
> + realmstring,
> + conn->session->wc_callbacks->auth_baton,
> + subpool);
> + if (err || ! creds)
> + {
> + svn_error_clear(err);
> + }
> + else
> + {
> + server_creds = creds;
> + err = svn_auth_save_credentials(state, subpool);
> + if (err)
> + {
> + /* It would be nice to show the error to the user somehow... */
> + svn_error_clear(err);
> + }
> + }
> +
> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
> + SVN_AUTH_PARAM_SSL_SERVER_CERT_INFO, NULL);
> +
> + svn_pool_destroy(subpool);
> +
> + return server_creds ? APR_SUCCESS : SVN_ERR_RA_SERF_SSL_CERT_UNTRUSTED;
> +}
> +
> serf_bucket_t *
> svn_ra_serf__conn_setup(apr_socket_t *sock,
> void *baton,
> @@ -71,6 +211,16 @@ svn_ra_serf__conn_setup(apr_socket_t *so
> svn_ra_serf__handle_client_cert_pw,
> conn, conn->session->pool);
> #endif
> +#if SERF_VERSION_AT_LEAST(0,1,3)
> + serf_ssl_server_cert_callback_set(conn->ssl_context,
> + ssl_server_cert,
> + conn);
> + /* See if the user wants us to trust "default" openssl CAs. */
> + if (conn->session->trust_default_ca)
> + {
> + serf_ssl_use_default_certificates(conn->ssl_context);
> + }
> +#endif
> }
> }
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
>
>
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Karl Fogel <kf...@red-bean.com>.
Lieven Govaerts <sv...@mobsol.be> writes:
> Since this is a patch for ra_serf, I assume Arfrever means this
> comment should talk about ra_serf instead of neon.
Heh. I should have looked at the file lines of the patch, instead of
the diff -- I just assumed it was against ra_neon! D'oh.
> Deng, now everyone knows from where I copied this part of the patch :)
*cough* *cough*
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Arfrever Frehtes Taifersar Arahesis <ar...@gmail.com>.
2008-03-17 18:58 Lieven Govaerts <sv...@mobsol.be> napisał(a):
> Karl Fogel wrote:
> > "Arfrever Frehtes Taifersar Arahesis" <ar...@gmail.com> writes:
> >>> --- trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 13:26:00 2008 (r29938)
> >>> +++ trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 18:27:54 2008 (r29939)
> >>> @@ -41,6 +42,145 @@
> >>> #define XML_STATUS_ERROR 0
> >>> #endif
> >>>
> >>> +static const apr_uint32_t serf_failure_map[][2] =
> >>> +{
> >>> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
> >>> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
> >>> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
> >>> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
> >>> +};
> >>> +
> >>> +/* Convert neon's SSL failure mask to our own failure mask. */
> >> "neon"
> >
> > Did you accidentally send the mail early? :-)
> >
> > I was kind of puzzled by this one-word review comment, since he does
> > write "neon" in the original commit -- well, "neon's", but that makes
> > sense in context.
>
>
> Since this is a patch for ra_serf, I assume Arfrever means this comment
> should talk about ra_serf instead of neon.
Yes :)
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Lieven Govaerts <sv...@mobsol.be>.
Karl Fogel wrote:
> "Arfrever Frehtes Taifersar Arahesis" <ar...@gmail.com> writes:
>>> --- trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 13:26:00 2008 (r29938)
>>> +++ trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 18:27:54 2008 (r29939)
>>> @@ -41,6 +42,145 @@
>>> #define XML_STATUS_ERROR 0
>>> #endif
>>>
>>> +static const apr_uint32_t serf_failure_map[][2] =
>>> +{
>>> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
>>> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
>>> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
>>> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
>>> +};
>>> +
>>> +/* Convert neon's SSL failure mask to our own failure mask. */
>> "neon"
>
> Did you accidentally send the mail early? :-)
>
> I was kind of puzzled by this one-word review comment, since he does
> write "neon" in the original commit -- well, "neon's", but that makes
> sense in context.
Since this is a patch for ra_serf, I assume Arfrever means this comment
should talk about ra_serf instead of neon.
Deng, now everyone knows from where I copied this part of the patch :)
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Karl Fogel <kf...@red-bean.com>.
"Arfrever Frehtes Taifersar Arahesis" <ar...@gmail.com> writes:
>> --- trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 13:26:00 2008 (r29938)
>> +++ trunk/subversion/libsvn_ra_serf/util.c Sun Mar 16 18:27:54 2008 (r29939)
>> @@ -41,6 +42,145 @@
>> #define XML_STATUS_ERROR 0
>> #endif
>>
>> +static const apr_uint32_t serf_failure_map[][2] =
>> +{
>> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
>> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
>> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
>> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
>> +};
>> +
>> +/* Convert neon's SSL failure mask to our own failure mask. */
>
> "neon"
Did you accidentally send the mail early? :-)
I was kind of puzzled by this one-word review comment, since he does
write "neon" in the original commit -- well, "neon's", but that makes
sense in context.
Just curious,
-Karl
>> +static apr_uint32_t
>> +ssl_convert_serf_failures(int failures)
>> +{
>> + apr_uint32_t svn_failures = 0;
>> + apr_size_t i;
>> +
>> + for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
>> + {
>> + if (failures & serf_failure_map[i][0])
>> + {
>> + svn_failures |= serf_failure_map[i][1];
>> + failures &= ~serf_failure_map[i][0];
>> + }
>> + }
>> +
>> + /* Map any remaining failure bits to our OTHER bit. */
>> + if (failures)
>> + {
>> + svn_failures |= SVN_AUTH_SSL_OTHER;
>> + }
>> +
>> + return svn_failures;
>> +}
>> +
>> +static char *
>> +convert_organisation_to_str(apr_hash_t *org, apr_pool_t *pool)
>> +{
>> + char *str;
>> +
>> + str = apr_psprintf(pool, "%s, %s, %s, %s, %s (%s)",
>> + (char*)apr_hash_get(org, "OU", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "O", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "L", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "ST", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "C", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "E", APR_HASH_KEY_STRING));
>> +
>> + return str;
>> +}
>> +
>> +static apr_status_t
>> +ssl_server_cert(void *baton, int failures,
>> + const serf_ssl_certificate_t *cert)
>> +{
>> + svn_ra_serf__connection_t *conn = baton;
>> + apr_pool_t *subpool;
>> + svn_auth_ssl_server_cert_info_t cert_info;
>> + svn_auth_cred_ssl_server_trust_t *server_creds = NULL;
>> + svn_auth_iterstate_t *state;
>> + const char *realmstring;
>> + apr_uint32_t svn_failures;
>> + svn_error_t *err;
>> + apr_hash_t *issuer, *subject, *serf_cert;
>> + void *creds;
>> +
>> + apr_pool_create(&subpool, conn->session->pool);
>> +
>> + /* Construct the realmstring, e.g. https://svn.collab.net:443 */
>> + realmstring = apr_uri_unparse(subpool,
>> + &conn->session->repos_url,
>> + APR_URI_UNP_OMITPATHINFO);
>> +
>> + /* Extract the info from the certificate */
>> + subject = serf_ssl_cert_subject(cert, subpool);
>> + issuer = serf_ssl_cert_issuer(cert, subpool);
>> + serf_cert = serf_ssl_cert_certificate(cert, subpool);
>> +
>> + cert_info.hostname = apr_hash_get(subject, "CN", APR_HASH_KEY_STRING);
>> + cert_info.fingerprint = apr_hash_get(serf_cert, "sha1", APR_HASH_KEY_STRING);
>> + if (! cert_info.fingerprint)
>> + cert_info.fingerprint = apr_pstrdup(subpool, "<unknown>");
>> + cert_info.valid_from = apr_hash_get(serf_cert, "notBefore",
>> + APR_HASH_KEY_STRING);
>> + if (! cert_info.valid_from)
>> + cert_info.valid_from = apr_pstrdup(subpool, "[invalid date]");
>> + cert_info.valid_until = apr_hash_get(serf_cert, "notAfter",
>> + APR_HASH_KEY_STRING);
>> + if (! cert_info.valid_until)
>> + cert_info.valid_until = apr_pstrdup(subpool, "[invalid date]");
>> + cert_info.issuer_dname = convert_organisation_to_str(issuer, subpool);
>> + cert_info.ascii_cert = "ce"; //ascii_cert;
>> +
>> + svn_failures = ssl_convert_serf_failures(failures);
>> +
>> + /* Match server certificate CN with the hostname of the server */
>> + if (cert_info.hostname)
>> + {
>> + if (apr_fnmatch(cert_info.hostname, conn->hostinfo,
>> + APR_FNM_PERIOD) == APR_FNM_NOMATCH)
>> + {
>> + svn_failures |= SVN_AUTH_SSL_CNMISMATCH;
>> + }
>> + }
>> +
>> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
>> + SVN_AUTH_PARAM_SSL_SERVER_FAILURES,
>> + &svn_failures);
>> +
>> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
>> + SVN_AUTH_PARAM_SSL_SERVER_CERT_INFO,
>> + &cert_info);
>> +
>> + err = svn_auth_first_credentials(&creds, &state,
>> + SVN_AUTH_CRED_SSL_SERVER_TRUST,
>> + realmstring,
>> + conn->session->wc_callbacks->auth_baton,
>> + subpool);
>> + if (err || ! creds)
>> + {
>> + svn_error_clear(err);
>> + }
>> + else
>> + {
>> + server_creds = creds;
>> + err = svn_auth_save_credentials(state, subpool);
>> + if (err)
>> + {
>> + /* It would be nice to show the error to the user somehow... */
>> + svn_error_clear(err);
>> + }
>> + }
>> +
>> + svn_auth_set_parameter(conn->session->wc_callbacks->auth_baton,
>> + SVN_AUTH_PARAM_SSL_SERVER_CERT_INFO, NULL);
>> +
>> + svn_pool_destroy(subpool);
>> +
>> + return server_creds ? APR_SUCCESS : SVN_ERR_RA_SERF_SSL_CERT_UNTRUSTED;
>> +}
>> +
>> serf_bucket_t *
>> svn_ra_serf__conn_setup(apr_socket_t *sock,
>> void *baton,
>> @@ -71,6 +211,16 @@ svn_ra_serf__conn_setup(apr_socket_t *so
>> svn_ra_serf__handle_client_cert_pw,
>> conn, conn->session->pool);
>> #endif
>> +#if SERF_VERSION_AT_LEAST(0,1,3)
>> + serf_ssl_server_cert_callback_set(conn->ssl_context,
>> + ssl_server_cert,
>> + conn);
>> + /* See if the user wants us to trust "default" openssl CAs. */
>> + if (conn->session->trust_default_ca)
>> + {
>> + serf_ssl_use_default_certificates(conn->ssl_context);
>> + }
>> +#endif
>> }
>> }
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: svn-help@subversion.tigris.org
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Lieven Govaerts <sv...@mobsol.be>.
Karl Fogel wrote:
> 2008-03-17 02:27 lgo@tigris.org <lg...@tigris.org> napisał(a):
>> --- trunk/subversion/libsvn_ra_serf/util.c (r29938)
>> +++ trunk/subversion/libsvn_ra_serf/util.c (r29939)
..
>> +static apr_uint32_t
>> +ssl_convert_serf_failures(int failures)
>> +{
>> + apr_uint32_t svn_failures = 0;
>> + apr_size_t i;
>> +
>> + for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
>> + {
>> + if (failures & serf_failure_map[i][0])
>> + {
>> + svn_failures |= serf_failure_map[i][1];
>> + failures &= ~serf_failure_map[i][0];
>> + }
>> + }
>> +
>> + /* Map any remaining failure bits to our OTHER bit. */
>> + if (failures)
>> + {
>> + svn_failures |= SVN_AUTH_SSL_OTHER;
>> + }
>> +
>> + return svn_failures;
>> +}
>
> It took me a while to figure out that the line
>
> failures &= ~serf_failure_map[i][0];
>
> was to remove the current failure from the original mask, so that at the
> end we could see if any are left over, and if so set OTHER. I updated
> the doc string in r30087 to describe the fall through into OTHER.
>
Thanks for that.
>
>> +static char *
>> +convert_organisation_to_str(apr_hash_t *org, apr_pool_t *pool)
>> +{
>> + char *str;
>> +
>> + str = apr_psprintf(pool, "%s, %s, %s, %s, %s (%s)",
>> + (char*)apr_hash_get(org, "OU", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "O", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "L", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "ST", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "C", APR_HASH_KEY_STRING),
>> + (char*)apr_hash_get(org, "E", APR_HASH_KEY_STRING));
>> +
>> + return str;
>> +}
>
> I gave this one a doc string too, though I admit the need was much less.
> (I also removed the holder variable.)
>
I've updated it a bit in r30092.
>> +static apr_status_t
>> +ssl_server_cert(void *baton, int failures,
>> + const serf_ssl_certificate_t *cert)
>> +{
>
> I have no idea looking at this what it does, so I didn't document it.
> Could you?
>
It's a callback function, implementing an interface documented in the
serf API. Docstring added in r30092.
> I know I harp on this doc string thing a lot, but most of our code is
> going to be read more than it is written. Doc strings save us time over
> the long run, and prevent distraction.
No need to convince me, but thank you for the review and friendly reminder!
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r29939 - in trunk/subversion: include libsvn_ra_serf
Posted by Karl Fogel <kf...@red-bean.com>.
2008-03-17 02:27 lgo@tigris.org <lg...@tigris.org> napisał(a):
> --- trunk/subversion/libsvn_ra_serf/util.c (r29938)
> +++ trunk/subversion/libsvn_ra_serf/util.c (r29939)
> @@ -21,6 +21,7 @@
> #define APR_WANT_STRFUNC
> #include <apr.h>
> #include <apr_want.h>
> +#include <apr_fnmatch.h>
>
> #include <serf.h>
> #include <serf_bucket_types.h>
> @@ -41,6 +42,145 @@
> #define XML_STATUS_ERROR 0
> #endif
>
> +static const apr_uint32_t serf_failure_map[][2] =
> +{
> + { SERF_SSL_CERT_NOTYETVALID, SVN_AUTH_SSL_NOTYETVALID },
> + { SERF_SSL_CERT_EXPIRED, SVN_AUTH_SSL_EXPIRED },
> + { SERF_SSL_CERT_SELF_SIGNED, SVN_AUTH_SSL_UNKNOWNCA },
> + { SERF_SSL_CERT_UNKNOWNCA, SVN_AUTH_SSL_UNKNOWNCA }
> +};
> +
> +/* Convert neon's SSL failure mask to our own failure mask. */
>
> +static apr_uint32_t
> +ssl_convert_serf_failures(int failures)
> +{
> + apr_uint32_t svn_failures = 0;
> + apr_size_t i;
> +
> + for (i = 0; i < sizeof(serf_failure_map) / (2 * sizeof(apr_uint32_t)); ++i)
> + {
> + if (failures & serf_failure_map[i][0])
> + {
> + svn_failures |= serf_failure_map[i][1];
> + failures &= ~serf_failure_map[i][0];
> + }
> + }
> +
> + /* Map any remaining failure bits to our OTHER bit. */
> + if (failures)
> + {
> + svn_failures |= SVN_AUTH_SSL_OTHER;
> + }
> +
> + return svn_failures;
> +}
It took me a while to figure out that the line
failures &= ~serf_failure_map[i][0];
was to remove the current failure from the original mask, so that at the
end we could see if any are left over, and if so set OTHER. I updated
the doc string in r30087 to describe the fall through into OTHER.
Some other functions introduced in this commit do not even have doc
strings. Sure, when the behavior of a function is *completely* obvious
from its name and arguments, I agree that the doc string can be
dispensed with (but even then it's better to write it, and easy to do so
because it's obvious.) Anyway, such functions are are quite rare.
> +static char *
> +convert_organisation_to_str(apr_hash_t *org, apr_pool_t *pool)
> +{
> + char *str;
> +
> + str = apr_psprintf(pool, "%s, %s, %s, %s, %s (%s)",
> + (char*)apr_hash_get(org, "OU", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "O", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "L", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "ST", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "C", APR_HASH_KEY_STRING),
> + (char*)apr_hash_get(org, "E", APR_HASH_KEY_STRING));
> +
> + return str;
> +}
I gave this one a doc string too, though I admit the need was much less.
(I also removed the holder variable.)
> +static apr_status_t
> +ssl_server_cert(void *baton, int failures,
> + const serf_ssl_certificate_t *cert)
> +{
I have no idea looking at this what it does, so I didn't document it.
Could you?
I know I harp on this doc string thing a lot, but most of our code is
going to be read more than it is written. Doc strings save us time over
the long run, and prevent distraction.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org