Re: Using UPN from subjectAltName with SSLUserName
On Sun, Aug 02, 2015 at 09:33:48AM +0200, Kaspar Brand wrote: On 19.07.2015 17:24, Kaspar Brand wrote: But, to be on the safe side, I think we could a) move the OBJ_create() call to ssl_hook_pre_config and b) omit OBJ_cleanup(). Do you concur? For the record: I have now committed this to trunk with r1693792. Sorry - yeah, looks good to me like that. Thanks Kaspar! Regards, Joe
Re: Using UPN from subjectAltName with SSLUserName
On Sun, Aug 02, 2015 at 09:33:48AM +0200, Kaspar Brand wrote: On 19.07.2015 17:24, Kaspar Brand wrote: But, to be on the safe side, I think we could a) move the OBJ_create() call to ssl_hook_pre_config and b) omit OBJ_cleanup(). Do you concur? For the record: I have now committed this to trunk with r1693792. Thank you! -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat
Re: Using UPN from subjectAltName with SSLUserName
On 19.07.2015 17:24, Kaspar Brand wrote: But, to be on the safe side, I think we could a) move the OBJ_create() call to ssl_hook_pre_config and b) omit OBJ_cleanup(). Do you concur? For the record: I have now committed this to trunk with r1693792. Kaspar
Re: Using UPN from subjectAltName with SSLUserName
On 13.07.2015 16:03, Joe Orton wrote: On Sat, Jul 11, 2015 at 04:40:20PM +0200, Kaspar Brand wrote: @@ -1902,5 +1907,7 @@ apr_status_t ssl_init_ModuleKill(void *data) free_dh_params(); +OBJ_cleanup(); + return APR_SUCCESS; From being burnt previously three or four times, I get scared by OpenSSL process global stuff. Have you worked out that it's safe to do that call there? It looks odd to do that there rather than alongside other global cleanups in ssl_cleanup_pre_config, so it's at least worth a comment if you really want this here. Thanks for the heads up. After further digging, I believe it's not really comparable to the cases you're probably referring to (only found these two): 1) ERR_load_crypto_strings() / ERR_free_strings() Originally addressed in https://svn.apache.org/r101624, it was subsequently fixed in OpenSSL (0.9.8e and later, see [1]), and in late 2014 also adjusted for more recent OpenSSL versions (PR 53435 / https://bz.apache.org/bugzilla/show_bug.cgi?id=53435). 2) CRYPTO_new_ex_data() / CRYPTO_cleanup_all_ex_data() Addressed in https://svn.apache.org/r654119 (by dropping the CRYPTO_cleanup_all_ex_data call), see also PR 44975 / https://bz.apache.org/bugzilla/show_bug.cgi?id=44975). OBJ_create() / OBJ_cleanup() doesn't seem to suffer from these problems, but on the other hand, I'm also fine with dropping the OBJ_cleanup() call, since we only call OBJ_create() if the table entry doesn't exist yet (i.e. even repeated calls in ssl_init_Module / reloads do not cause any leak). There is some complicated interaction between EVP_cleanup() and OBJ_cleanup() which I haven't tried to decipher, but it looks like EVP_cleanup() will actually do the cleanup call? This was added with 1.0.0, apparently [2]. The code would apply if OBJ_cleanup were called *before* EVP_cleanup (in all other cases, EVP_cleanup does not call OBJ_cleanup). But, to be on the safe side, I think we could a) move the OBJ_create() call to ssl_hook_pre_config and b) omit OBJ_cleanup(). Do you concur? Kaspar [1] https://git.openssl.org/?p=openssl.git;a=commitdiff;h=35e59297fca389b68bcad29876927666300ce971 [2] https://git.openssl.org/?p=openssl.git;a=commitdiff;h=246e09319c1d2a8140ffe1e5aeb1be26015696f0
Re: Using UPN from subjectAltName with SSLUserName
On Sat, Jul 11, 2015 at 04:40:20PM +0200, Kaspar Brand wrote: On 29.06.2015 15:14, Jan Pazdziora wrote: How about just passing char * and doing all the mapping logic including possible OBJ_create in parse_otherName_value? My goal here is to have all the hard work of determining the semantics isolated in one place. Please see patch attached. You're right, an ASN1_OBJECT * as an argument for modssl_X509_getSAN makes handling of otherName entries relatively awkward. In the attached patch, I have switched to a string for specifying the requested otherName form (similar to what you did in your patch). OBJ_create adds new entries to a process-wide table, so instead of checking for the presence of a specific entry at each request (in parse_otherName_value), I consider it more appropriate and efficient to do this only once, in ssl_init_Module. Barring feedback against this approach (or the observation of bugs in the implementation), I intend to commit it to trunk in the next few days (including mod_ssl.xml changes and a CHANGES item). I've tried your patch and it works find for me. So I'm happy with your plan of committing it to trunk. ;-) Thank you! -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat
Re: Using UPN from subjectAltName with SSLUserName
On Sat, Jul 11, 2015 at 04:40:20PM +0200, Kaspar Brand wrote: @@ -1902,5 +1907,7 @@ apr_status_t ssl_init_ModuleKill(void *data) free_dh_params(); +OBJ_cleanup(); + return APR_SUCCESS; From being burnt previously three or four times, I get scared by OpenSSL process global stuff. Have you worked out that it's safe to do that call there? It looks odd to do that there rather than alongside other global cleanups in ssl_cleanup_pre_config, so it's at least worth a comment if you really want this here. There is some complicated interaction between EVP_cleanup() and OBJ_cleanup() which I haven't tried to decipher, but it looks like EVP_cleanup() will actually do the cleanup call? Regards, Joe
Re: Using UPN from subjectAltName with SSLUserName
On 29.06.2015 15:14, Jan Pazdziora wrote:
How about just passing char * and doing all the mapping logic
including possible OBJ_create in parse_otherName_value? My goal here
is to have all the hard work of determining the semantics isolated
in one place.
Please see patch attached.
You're right, an ASN1_OBJECT * as an argument for modssl_X509_getSAN
makes handling of otherName entries relatively awkward. In the attached
patch, I have switched to a string for specifying the requested
otherName form (similar to what you did in your patch).
OBJ_create adds new entries to a process-wide table, so instead of
checking for the presence of a specific entry at each request (in
parse_otherName_value), I consider it more appropriate and efficient to
do this only once, in ssl_init_Module.
Barring feedback against this approach (or the observation of bugs in
the implementation), I intend to commit it to trunk in the next few days
(including mod_ssl.xml changes and a CHANGES item).
Kaspar
Index: modules/ssl/ssl_engine_init.c
===
--- modules/ssl/ssl_engine_init.c (revision 1690371)
+++ modules/ssl/ssl_engine_init.c (working copy)
@@ -352,6 +352,11 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
init_dh_params();
+if (OBJ_txt2nid(id-on-dnsSRV) == NID_undef) {
+(void)OBJ_create(1.3.6.1.5.5.7.8.7, id-on-dnsSRV,
+ SRVName otherName form);
+}
+
return OK;
}
@@ -1902,5 +1907,7 @@ apr_status_t ssl_init_ModuleKill(void *data)
free_dh_params();
+OBJ_cleanup();
+
return APR_SUCCESS;
}
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c (revision 1690371)
+++ modules/ssl/ssl_util_ssl.c (working copy)
@@ -252,19 +252,48 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
return result;
}
+static void parse_otherName_value(apr_pool_t *p, ASN1_TYPE *value,
+ const char *onf, apr_array_header_t
**entries)
+{
+const char *str;
+int nid = onf ? OBJ_txt2nid(onf) : NID_undef;
+
+if (!value || (nid == NID_undef) || !*entries)
+ return;
+
+/*
+ * Currently supported otherName forms (values for onf):
+ * msUPN (1.3.6.1.4.1.311.20.2.3): Microsoft User Principal Name
+ * id-on-dnsSRV (1.3.6.1.5.5.7.8.7): SRVName, as specified in RFC 4985
+ */
+if ((nid == NID_ms_upn) (value-type == V_ASN1_UTF8STRING)
+(str = asn1_string_to_utf8(p, value-value.utf8string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+} else if (strEQ(onf, id-on-dnsSRV)
+ (value-type == V_ASN1_IA5STRING)
+ (str = asn1_string_to_utf8(p, value-value.ia5string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+}
+}
+
/*
* Return an array of subjectAltName entries of type type. If idx is -1,
* return all entries of the given type, otherwise return an array consisting
* of the n-th occurrence of that type only. Currently supported types:
* GEN_EMAIL (rfc822Name)
* GEN_DNS (dNSName)
+ * GEN_OTHERNAME (requires the otherName form [onf] argument to be supplied,
+ *see parse_otherName_value for the currently supported forms)
*/
-BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
-apr_array_header_t **entries)
+BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, const char *onf,
+int idx, apr_array_header_t **entries)
{
STACK_OF(GENERAL_NAME) *names;
+int nid = onf ? OBJ_txt2nid(onf) : NID_undef;
-if (!x509 || (type GEN_OTHERNAME) || (type GEN_RID) || (idx -1) ||
+if (!x509 || (type GEN_OTHERNAME) ||
+((type == GEN_OTHERNAME) (nid == NID_undef)) ||
+(type GEN_RID) || (idx -1) ||
!(*entries = apr_array_make(p, 0, sizeof(char * {
*entries = NULL;
return FALSE;
@@ -277,33 +306,43 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
for (i = 0; i sk_GENERAL_NAME_num(names); i++) {
name = sk_GENERAL_NAME_value(names, i);
-if (name-type == type) {
-if ((idx == -1) || (n == idx)) {
-switch (type) {
-case GEN_EMAIL:
-case GEN_DNS:
-utf8str = asn1_string_to_utf8(p, name-d.ia5);
-if (utf8str) {
-APR_ARRAY_PUSH(*entries, const char *) = utf8str;
-}
-break;
-default:
-/*
- * Not implemented right now:
- * GEN_OTHERNAME (otherName)
- * GEN_X400 (x400Address)
- * GEN_DIRNAME (directoryName)
- * GEN_EDIPARTY (ediPartyName)
-
Re: Using UPN from subjectAltName with SSLUserName
On 06/29/2015 03:14 PM, Jan Pazdziora wrote: On Mon, Jun 29, 2015 at 01:47:45PM +0200, Jan Pazdziora wrote: On Sun, Jun 28, 2015 at 05:11:57PM +0200, Kaspar Brand wrote: On 22.06.2015 10:37, Jan Pazdziora wrote: Please find a new patch attached which I hope covers all the parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*. Thanks. Your implementation assumes that only a single otherName form (msUPN) needs to be supported, but I would prefer to code it in a somewhat more extensible way. Does the attached patch work for you? As a practical way of Yes it does. My only question (and comments bellow) is about passing the oid rather than nid through the functions. I can see the string id-on-dnsSRV used twice in the patch in call OBJ_txt2nid and twice in call OBJ_txt2obj as well when ideally all which should be needed one OBJ_txt2nid(id-on-dnsSRV) and one OBJ_create if not found -- the rest could be done by comparing integers (nid). Unless I'm missing something about the oid/nid interaction. Ah, now I see it -- you look at the semantics of oid to compare value-type so nid would not be enough. How about just passing char * and doing all the mapping logic including possible OBJ_create in parse_otherName_value? My goal here is to have all the hard work of determining the semantics isolated in one place. Please see patch attached. Hi Kaspar, please could you find some time to review this patch? I can say that both proposed patches (your and Jan's) are equivalent when it comes to implementation functionality. Unfortunately, I don't have the OpenSSL knowledge to comment the differences on technical level, but I would also like to see this functionality in the trunk :). Regards, Jan Kaluza
Re: Using UPN from subjectAltName with SSLUserName
On Sun, Jun 28, 2015 at 05:11:57PM +0200, Kaspar Brand wrote:
On 22.06.2015 10:37, Jan Pazdziora wrote:
Please find a new patch attached which I hope covers all the
parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*.
Thanks. Your implementation assumes that only a single otherName form
(msUPN) needs to be supported, but I would prefer to code it in a
somewhat more extensible way.
Does the attached patch work for you? As a practical way of
Yes it does.
My only question (and comments bellow) is about passing the oid rather
than nid through the functions. I can see the string id-on-dnsSRV
used twice in the patch in call OBJ_txt2nid and twice in call OBJ_txt2obj
as well when ideally all which should be needed one
OBJ_txt2nid(id-on-dnsSRV) and one OBJ_create if not found -- the rest
could be done by comparing integers (nid). Unless I'm missing something
about the oid/nid interaction.
demonstrating generic support of otherName forms, I have
added the case of the SRVName otherName form (RFC 4985, for things like
_carddavs._tcp.example.com, exposed via SSL_SERVER_SAN_OTHER_dnsSRV_n).
Kaspar
Index: modules/ssl/ssl_engine_init.c
===
--- modules/ssl/ssl_engine_init.c (revision 1687983)
+++ modules/ssl/ssl_engine_init.c (working copy)
@@ -352,6 +352,11 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
init_dh_params();
+if (OBJ_txt2nid(id-on-dnsSRV) == NID_undef) {
+(void)OBJ_create(1.3.6.1.5.5.7.8.7, id-on-dnsSRV,
+ SRVName otherName form);
+}
Why do the OBJ_create call here and not in
ssl_var_lookup_ssl_cert_san, once it it clear it will be needed at
all?
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c(revision 1687983)
+++ modules/ssl/ssl_util_ssl.c(working copy)
@@ -252,19 +252,46 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
return result;
}
+static void parse_otherName_value(apr_pool_t *p, ASN1_TYPE *value,
+ ASN1_OBJECT *oid,
+ apr_array_header_t **entries)
+{
+const char *str;
+
+if (!value || !oid || !*entries)
+ return;
+
+/*
+ * Currently supported otherName forms:
+ * - msUPN (1.3.6.1.4.1.311.20.2.3): Microsoft User Principal Name
+ * - dnsSRV (1.3.6.1.5.5.7.8.7): SRVName, as specified in RFC 4985
+ */
+if ((OBJ_obj2nid(oid) == NID_ms_upn)
Since the OBJ_obj2nid is made always before the comparison, could it be
simpler to just convert to nid in ssl_var_lookup_ssl_cert_san and pass
int, instead passingof ASN1_OBJECT * to modssl_X509_getSAN and
parse_otherName_value?
+(value-type == V_ASN1_UTF8STRING)
+(str = asn1_string_to_utf8(p, value-value.utf8string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+} else if ((OBJ_obj2nid(oid) == OBJ_txt2nid(id-on-dnsSRV))
+ (value-type == V_ASN1_IA5STRING)
+ (str = asn1_string_to_utf8(p, value-value.ia5string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+}
+}
+
/*
* Return an array of subjectAltName entries of type type. If idx is -1,
* return all entries of the given type, otherwise return an array consisting
* of the n-th occurrence of that type only. Currently supported types:
* GEN_EMAIL (rfc822Name)
* GEN_DNS (dNSName)
+ * GEN_OTHERNAME (see parse_otherName_value for currently supported forms)
*/
-BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
-apr_array_header_t **entries)
+BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, ASN1_OBJECT
*oid,
+int idx, apr_array_header_t **entries)
{
STACK_OF(GENERAL_NAME) *names;
-if (!x509 || (type GEN_OTHERNAME) || (type GEN_RID) || (idx -1) ||
+if (!x509 || (type GEN_OTHERNAME) || (type == GEN_OTHERNAME !oid) ||
+(type GEN_RID) || (idx -1) ||
!(*entries = apr_array_make(p, 0, sizeof(char * {
*entries = NULL;
return FALSE;
@@ -277,33 +304,43 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
for (i = 0; i sk_GENERAL_NAME_num(names); i++) {
name = sk_GENERAL_NAME_value(names, i);
-if (name-type == type) {
-if ((idx == -1) || (n == idx)) {
-switch (type) {
-case GEN_EMAIL:
-case GEN_DNS:
-utf8str = asn1_string_to_utf8(p, name-d.ia5);
-if (utf8str) {
-APR_ARRAY_PUSH(*entries, const char *) = utf8str;
-}
-break;
-default:
-/*
- * Not
Re: Using UPN from subjectAltName with SSLUserName
On Mon, Jun 29, 2015 at 01:47:45PM +0200, Jan Pazdziora wrote:
On Sun, Jun 28, 2015 at 05:11:57PM +0200, Kaspar Brand wrote:
On 22.06.2015 10:37, Jan Pazdziora wrote:
Please find a new patch attached which I hope covers all the
parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*.
Thanks. Your implementation assumes that only a single otherName form
(msUPN) needs to be supported, but I would prefer to code it in a
somewhat more extensible way.
Does the attached patch work for you? As a practical way of
Yes it does.
My only question (and comments bellow) is about passing the oid rather
than nid through the functions. I can see the string id-on-dnsSRV
used twice in the patch in call OBJ_txt2nid and twice in call OBJ_txt2obj
as well when ideally all which should be needed one
OBJ_txt2nid(id-on-dnsSRV) and one OBJ_create if not found -- the rest
could be done by comparing integers (nid). Unless I'm missing something
about the oid/nid interaction.
Ah, now I see it -- you look at the semantics of oid to compare
value-type so nid would not be enough.
How about just passing char * and doing all the mapping logic
including possible OBJ_create in parse_otherName_value? My goal here
is to have all the hard work of determining the semantics isolated
in one place.
Please see patch attached.
--
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat
Index: modules/ssl/ssl_engine_init.c
===
--- modules/ssl/ssl_engine_init.c (revision 1688186)
+++ modules/ssl/ssl_engine_init.c (working copy)
@@ -1902,5 +1902,7 @@
free_dh_params();
+OBJ_cleanup();
+
return APR_SUCCESS;
}
Index: modules/ssl/ssl_engine_vars.c
===
--- modules/ssl/ssl_engine_vars.c (revision 1688186)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -665,6 +665,7 @@
{
int type, numlen;
apr_array_header_t *entries;
+const char *id = NULL;
if (strcEQn(var, Email_, 6)) {
type = GEN_EMAIL;
@@ -674,6 +675,20 @@
type = GEN_DNS;
var += 4;
}
+else if (strcEQn(var, OTHER_, 6)) {
+type = GEN_OTHERNAME;
+var += 6;
+if (strEQn(var, msUPN_, 6)) {
+var += 6;
+id = msUPN;
+}
+else if (strEQn(var, dnsSRV_, 7)) {
+var += 7;
+id = id-on-dnsSRV;
+}
+else
+ return NULL;
+}
else
return NULL;
@@ -682,7 +697,7 @@
if ((numlen 1) || (numlen 4) || (numlen != strlen(var)))
return NULL;
-if (modssl_X509_getSAN(p, xs, type, atoi(var), entries))
+if (modssl_X509_getSAN(p, xs, type, id, atoi(var), entries))
/* return the first entry from this 1-element array */
return APR_ARRAY_IDX(entries, 0, char *);
else
@@ -1032,12 +1047,15 @@
/* subjectAltName entries of the server certificate */
xs = SSL_get_certificate(ssl);
if (xs) {
-if (modssl_X509_getSAN(p, xs, GEN_EMAIL, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_EMAIL, NULL, -1, entries)) {
extract_san_array(t, SSL_SERVER_SAN_Email, entries, p);
}
-if (modssl_X509_getSAN(p, xs, GEN_DNS, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_DNS, NULL, -1, entries)) {
extract_san_array(t, SSL_SERVER_SAN_DNS, entries, p);
}
+if (modssl_X509_getSAN(p, xs, GEN_DNS, id-on-dnsSRV, -1, entries)) {
+extract_san_array(t, SSL_SERVER_SAN_OTHER_dnsSRV, entries, p);
+}
/* no need to free xs (refcount does not increase) */
}
@@ -1044,12 +1062,15 @@
/* subjectAltName entries of the client certificate */
xs = SSL_get_peer_certificate(ssl);
if (xs) {
-if (modssl_X509_getSAN(p, xs, GEN_EMAIL, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_EMAIL, NULL, -1, entries)) {
extract_san_array(t, SSL_CLIENT_SAN_Email, entries, p);
}
-if (modssl_X509_getSAN(p, xs, GEN_DNS, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_DNS, NULL, -1, entries)) {
extract_san_array(t, SSL_CLIENT_SAN_DNS, entries, p);
}
+if (modssl_X509_getSAN(p, xs, GEN_OTHERNAME, msUPN, -1, entries)) {
+extract_san_array(t, SSL_CLIENT_SAN_OTHER_msUPN, entries, p);
+}
X509_free(xs);
}
}
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c (revision 1688186)
+++ modules/ssl/ssl_util_ssl.c (working copy)
@@ -252,6 +252,46 @@
return result;
}
+static BOOL parse_otherName_value(apr_pool_t *p, OTHERNAME *othername,
+ const char *id,
+ apr_array_header_t **entries)
+{
+if (!othername ||
Re: Using UPN from subjectAltName with SSLUserName
On 22.06.2015 10:37, Jan Pazdziora wrote:
Please find a new patch attached which I hope covers all the
parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*.
Thanks. Your implementation assumes that only a single otherName form
(msUPN) needs to be supported, but I would prefer to code it in a
somewhat more extensible way.
Does the attached patch work for you? As a practical way of
demonstrating generic support of otherName forms, I have
added the case of the SRVName otherName form (RFC 4985, for things like
_carddavs._tcp.example.com, exposed via SSL_SERVER_SAN_OTHER_dnsSRV_n).
Kaspar
Index: modules/ssl/ssl_engine_init.c
===
--- modules/ssl/ssl_engine_init.c (revision 1687983)
+++ modules/ssl/ssl_engine_init.c (working copy)
@@ -352,6 +352,11 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_po
init_dh_params();
+if (OBJ_txt2nid(id-on-dnsSRV) == NID_undef) {
+(void)OBJ_create(1.3.6.1.5.5.7.8.7, id-on-dnsSRV,
+ SRVName otherName form);
+}
+
return OK;
}
@@ -1902,5 +1907,7 @@ apr_status_t ssl_init_ModuleKill(void *data)
free_dh_params();
+OBJ_cleanup();
+
return APR_SUCCESS;
}
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c (revision 1687983)
+++ modules/ssl/ssl_util_ssl.c (working copy)
@@ -252,19 +252,46 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
return result;
}
+static void parse_otherName_value(apr_pool_t *p, ASN1_TYPE *value,
+ ASN1_OBJECT *oid,
+ apr_array_header_t **entries)
+{
+const char *str;
+
+if (!value || !oid || !*entries)
+ return;
+
+/*
+ * Currently supported otherName forms:
+ * - msUPN (1.3.6.1.4.1.311.20.2.3): Microsoft User Principal Name
+ * - dnsSRV (1.3.6.1.5.5.7.8.7): SRVName, as specified in RFC 4985
+ */
+if ((OBJ_obj2nid(oid) == NID_ms_upn)
+(value-type == V_ASN1_UTF8STRING)
+(str = asn1_string_to_utf8(p, value-value.utf8string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+} else if ((OBJ_obj2nid(oid) == OBJ_txt2nid(id-on-dnsSRV))
+ (value-type == V_ASN1_IA5STRING)
+ (str = asn1_string_to_utf8(p, value-value.ia5string))) {
+APR_ARRAY_PUSH(*entries, const char *) = str;
+}
+}
+
/*
* Return an array of subjectAltName entries of type type. If idx is -1,
* return all entries of the given type, otherwise return an array consisting
* of the n-th occurrence of that type only. Currently supported types:
* GEN_EMAIL (rfc822Name)
* GEN_DNS (dNSName)
+ * GEN_OTHERNAME (see parse_otherName_value for currently supported forms)
*/
-BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
-apr_array_header_t **entries)
+BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, ASN1_OBJECT *oid,
+int idx, apr_array_header_t **entries)
{
STACK_OF(GENERAL_NAME) *names;
-if (!x509 || (type GEN_OTHERNAME) || (type GEN_RID) || (idx -1) ||
+if (!x509 || (type GEN_OTHERNAME) || (type == GEN_OTHERNAME !oid) ||
+(type GEN_RID) || (idx -1) ||
!(*entries = apr_array_make(p, 0, sizeof(char * {
*entries = NULL;
return FALSE;
@@ -277,33 +304,43 @@ char *modssl_X509_NAME_to_string(apr_pool_t *p, X5
for (i = 0; i sk_GENERAL_NAME_num(names); i++) {
name = sk_GENERAL_NAME_value(names, i);
-if (name-type == type) {
-if ((idx == -1) || (n == idx)) {
-switch (type) {
-case GEN_EMAIL:
-case GEN_DNS:
-utf8str = asn1_string_to_utf8(p, name-d.ia5);
-if (utf8str) {
-APR_ARRAY_PUSH(*entries, const char *) = utf8str;
-}
-break;
-default:
-/*
- * Not implemented right now:
- * GEN_OTHERNAME (otherName)
- * GEN_X400 (x400Address)
- * GEN_DIRNAME (directoryName)
- * GEN_EDIPARTY (ediPartyName)
- * GEN_URI (uniformResourceIdentifier)
- * GEN_IPADD (iPAddress)
- * GEN_RID (registeredID)
- */
-break;
+
+if (name-type != type)
+continue;
+
+switch (type) {
+case GEN_EMAIL:
+case GEN_DNS:
+if (((idx == -1) || (n == idx))
+(utf8str = asn1_string_to_utf8(p, name-d.ia5))) {
+APR_ARRAY_PUSH(*entries, const char *) = utf8str;
Re: Using UPN from subjectAltName with SSLUserName
On Sun, Jun 21, 2015 at 08:55:02AM +0200, Kaspar Brand wrote:
The point with the otherName SAN type is that it's yet another bag of
potentially arbitrary ASN.1 stuff, actually (not just simple strings, as
is the case for rfc822Name or dNSName):
GeneralName ::= CHOICE {
otherName [0] OtherName,
rfc822Name [1] IA5String,
dNSName [2] IA5String,
x400Address [3] ORAddress,
directoryName [4] Name,
ediPartyName[5] EDIPartyName,
uniformResourceIdentifier [6] IA5String,
iPAddress [7] OCTET STRING,
registeredID[8] OBJECT IDENTIFIER }
OtherName ::= SEQUENCE {
type-idOBJECT IDENTIFIER,
value [0] EXPLICIT ANY DEFINED BY type-id }
^^
(See RFC 7299 section 3.14 for the otherName forms defined by the PKIX
WG in the past.)
While Microsoft's UPN happens to be a very simple case (the value is
just a bare UTF8String here), this need not be true for other forms
of otherName.
Adding support for msUPN seems useful, but it's really something which
needs special-case coding in ssl_util_ssl.c:modssl_X509_getSAN(). I
suggest using SSL_{CLIENT,SERVER}_SAN_OTHER_msUPN_* for the variable
name(s), to make it clear that it's a subjectAltName entry of type
otherName. Then, in the code for GEN_OTHERNAME, specifically look for
this otherName form via NID_ms_upn - as only in this case, you
can be sure to expect a simple UTF8String in otherName-value
(strongSwan's openssl_x509.c might be a source of inspiration for coding
this).
Note that for exposing the msUPN variables with StdEnvVars, you also
need to adapt ssl_engine_vars.c:modssl_var_extract_san_entries().
Thank you for the analysis.
Please find a new patch attached which I hope covers all the
parts you've outlined, for SSL_CLIENT_SAN_OTHER_msUPN_*.
Yours,
--
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat
Index: docs/manual/mod/mod_ssl.xml
===
--- docs/manual/mod/mod_ssl.xml (revision 1686805)
+++ docs/manual/mod/mod_ssl.xml (working copy)
@@ -77,6 +77,7 @@
trtdcodeSSL_CLIENT_S_DN_/codeemx509/em/td tdstring/td
tdComponent of client's Subject DN/td/tr
trtdcodeSSL_CLIENT_SAN_Email_/codeemn/em/td tdstring/td
tdClient certificate's subjectAltName extension entries of type
rfc822Name/td/tr
trtdcodeSSL_CLIENT_SAN_DNS_/codeemn/em/td tdstring/td
tdClient certificate's subjectAltName extension entries of type
dNSName/td/tr
+trtdcodeSSL_CLIENT_SAN_OTHER_msUPN_/codeemn/em/td
tdstring/tdtdClient certificate's subjectAltName extension entries of
type otherName with Microsoft Universal Principal Name (OID
1.3.6.1.4.1.311.20.2.3)/td/tr
trtdcodeSSL_CLIENT_I_DN/code/td tdstring/td
tdIssuer DN of client's certificate/td/tr
trtdcodeSSL_CLIENT_I_DN_/codeemx509/em/td tdstring/td
tdComponent of client's Issuer DN/td/tr
trtdcodeSSL_CLIENT_V_START/code/tdtdstring/td
tdValidity of client's certificate (start time)/td/tr
Index: modules/ssl/ssl_engine_vars.c
===
--- modules/ssl/ssl_engine_vars.c (revision 1686805)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -674,6 +674,10 @@
type = GEN_DNS;
var += 4;
}
+else if (strcEQn(var, OTHER_msUPN_, 12)) {
+type = GEN_OTHERNAME;
+var += 12;
+}
else
return NULL;
@@ -1050,6 +1054,9 @@
if (modssl_X509_getSAN(p, xs, GEN_DNS, -1, entries)) {
extract_san_array(t, SSL_CLIENT_SAN_DNS, entries, p);
}
+if (modssl_X509_getSAN(p, xs, GEN_OTHERNAME, -1, entries)) {
+extract_san_array(t, SSL_CLIENT_SAN_OTHER_msUPN, entries, p);
+}
X509_free(xs);
}
}
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c (revision 1686805)
+++ modules/ssl/ssl_util_ssl.c (working copy)
@@ -258,6 +258,7 @@
* of the n-th occurrence of that type only. Currently supported types:
* GEN_EMAIL (rfc822Name)
* GEN_DNS (dNSName)
+ * GEN_OTHERNAME (otherName for UPN)
*/
BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
apr_array_header_t **entries)
@@ -287,10 +288,18 @@
APR_ARRAY_PUSH(*entries, const char *) = utf8str;
}
break;
+case GEN_OTHERNAME:
+if (name-d.otherName name-d.otherName-value
+ OBJ_obj2nid(name-d.otherName-type_id) ==
Re: Using UPN from subjectAltName with SSLUserName
On 19.06.2015 16:51, Jan Pazdziora wrote:
On Thu, Jun 18, 2015 at 12:22:21PM +0200, Yann Ylavic wrote:
I think a more generic way would to have something like
SSL_CLIENT_OID_oid_n, so that we wouldn't have to add a new field
each time.
In this case, that would be: SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n.
The point with the otherName SAN type is that it's yet another bag of
potentially arbitrary ASN.1 stuff, actually (not just simple strings, as
is the case for rfc822Name or dNSName):
GeneralName ::= CHOICE {
otherName [0] OtherName,
rfc822Name [1] IA5String,
dNSName [2] IA5String,
x400Address [3] ORAddress,
directoryName [4] Name,
ediPartyName[5] EDIPartyName,
uniformResourceIdentifier [6] IA5String,
iPAddress [7] OCTET STRING,
registeredID[8] OBJECT IDENTIFIER }
OtherName ::= SEQUENCE {
type-idOBJECT IDENTIFIER,
value [0] EXPLICIT ANY DEFINED BY type-id }
^^
(See RFC 7299 section 3.14 for the otherName forms defined by the PKIX
WG in the past.)
While Microsoft's UPN happens to be a very simple case (the value is
just a bare UTF8String here), this need not be true for other forms
of otherName.
Adding support for msUPN seems useful, but it's really something which
needs special-case coding in ssl_util_ssl.c:modssl_X509_getSAN(). I
suggest using SSL_{CLIENT,SERVER}_SAN_OTHER_msUPN_* for the variable
name(s), to make it clear that it's a subjectAltName entry of type
otherName. Then, in the code for GEN_OTHERNAME, specifically look for
this otherName form via NID_ms_upn - as only in this case, you
can be sure to expect a simple UTF8String in otherName-value
(strongSwan's openssl_x509.c might be a source of inspiration for coding
this).
Note that for exposing the msUPN variables with StdEnvVars, you also
need to adapt ssl_engine_vars.c:modssl_var_extract_san_entries().
Kaspar
Re: Using UPN from subjectAltName with SSLUserName
On 06/18/2015 12:22 PM, Yann Ylavic wrote: On Thu, Jun 18, 2015 at 11:49 AM, Jan Pazdziora jpazdzi...@redhat.com wrote: I'd appreciate any comments about suitability of such change, as well as the implementation. Specifically, I'm not sure if people will prefer the generic and currently proposed SSL_CLIENT_SAN_otherName_n which gets any value of otherName type, or perhaps going with SSL_CLIENT_SAN_UPN_n and checking the OID just for the UPNs. Based on that decision I plan to then respin the patch with documentation changes included. I think a more generic way would to have something like SSL_CLIENT_OID_oid_n, so that we wouldn't have to add a new field each time. In this case, that would be: SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n. I think that's nice idea. I can probably work on that. The only question is if we would like to have this generic way as additional feature, or we really want to use it instead of the SSL_CLIENT_SAN_otherName_n as proposed by Jan. I think that the common cases should have non-generic variable. The question is if otherName is the common case. Ideas? Regards, Yann. Regards, Jan Kaluza
Re: Using UPN from subjectAltName with SSLUserName
On Fri, Jun 19, 2015 at 11:56 AM, Yann Ylavic ylavic@gmail.com wrote: Instead of SSL_CLIENT_OID_, we could also have something like SSL_CLIENT__oid|shortname|fullname__n since the underlying mod_ssl code handles both (IIRC). I don't know if SAN_otherName/UPN have a short/long name though, but many have. Nope, SAN as an oid/long/short name, but no fancy name for its inner fields. But since openssl.cnf (man x509v3_config) accepts things like subjectAltName=email:my@other.address,RID:1.2.3.4,otherName:my.other.name, maybe we can do something...
Re: Using UPN from subjectAltName with SSLUserName
On Fri, Jun 19, 2015 at 11:32 AM, Jan Kaluža jkal...@redhat.com wrote: On 06/18/2015 12:22 PM, Yann Ylavic wrote: On Thu, Jun 18, 2015 at 11:49 AM, Jan Pazdziora jpazdzi...@redhat.com wrote: I'd appreciate any comments about suitability of such change, as well as the implementation. Specifically, I'm not sure if people will prefer the generic and currently proposed SSL_CLIENT_SAN_otherName_n which gets any value of otherName type, or perhaps going with SSL_CLIENT_SAN_UPN_n and checking the OID just for the UPNs. Based on that decision I plan to then respin the patch with documentation changes included. I think a more generic way would to have something like SSL_CLIENT_OID_oid_n, so that we wouldn't have to add a new field each time. In this case, that would be: SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n. I think that's nice idea. I can probably work on that. The only question is if we would like to have this generic way as additional feature, or we really want to use it instead of the SSL_CLIENT_SAN_otherName_n as proposed by Jan. I think that the common cases should have non-generic variable. The question is if otherName is the common case. Depends on whether one needs it or not I guess :) Ideas? Instead of SSL_CLIENT_OID_, we could also have something like SSL_CLIENT__oid|shortname|fullname__n since the underlying mod_ssl code handles both (IIRC). I don't know if SAN_otherName/UPN have a short/long name though, but many have.
Re: Using UPN from subjectAltName with SSLUserName
On Thu, Jun 18, 2015 at 12:22:21PM +0200, Yann Ylavic wrote:
On Thu, Jun 18, 2015 at 11:49 AM, Jan Pazdziora jpazdzi...@redhat.com wrote:
I'd appreciate any comments about suitability of such change, as well
as the implementation. Specifically, I'm not sure if people will
prefer the generic and currently proposed
SSL_CLIENT_SAN_otherName_n
which gets any value of otherName type, or perhaps going with
SSL_CLIENT_SAN_UPN_n
and checking the OID just for the UPNs. Based on that decision I plan
to then respin the patch with documentation changes included.
I think a more generic way would to have something like
SSL_CLIENT_OID_oid_n, so that we wouldn't have to add a new field
each time.
In this case, that would be: SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n.
Please find attached a patch which makes it possible to use
SSL_CLIENT_SAN_otherName_1.3.6.1.4.1.311.20.2.3_0
or
SSL_CLIENT_SAN_otherName_0
In the first case we use OBJ_create to create the object and then
compare its nid to OBJ_obj2nid result. In the second form we ignore
the OID.
I went with the SAN_otherName rather than OID to make it clear where
we get the value from. Of course, of you think that
SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n better and the correct place
to look for it is always in name-d.otherName, it should be easy to
change the name.
--
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat
Index: modules/ssl/ssl_engine_vars.c
===
--- modules/ssl/ssl_engine_vars.c (revision 1686420)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -663,8 +663,9 @@
static char *ssl_var_lookup_ssl_cert_san(apr_pool_t *p, X509 *xs, char *var)
{
-int type, numlen;
+int type, numlen, nid = NID_undef;
apr_array_header_t *entries;
+char *ptr;
if (strcEQn(var, Email_, 6)) {
type = GEN_EMAIL;
@@ -674,6 +675,17 @@
type = GEN_DNS;
var += 4;
}
+else if (strcEQn(var, otherName_, 10)) {
+type = GEN_OTHERNAME;
+var += 10;
+ptr = strchr(var, '_');
+if (ptr) {
+char *oid_str;
+oid_str = apr_pstrndup(p, var, (ptr - var));
+nid = OBJ_create(oid_str, NULL, NULL);
+var = ptr + 1;
+}
+}
else
return NULL;
@@ -682,7 +694,7 @@
if ((numlen 1) || (numlen 4) || (numlen != strlen(var)))
return NULL;
-if (modssl_X509_getSAN(p, xs, type, atoi(var), entries))
+if (modssl_X509_getSAN(p, xs, type, nid, atoi(var), entries))
/* return the first entry from this 1-element array */
return APR_ARRAY_IDX(entries, 0, char *);
else
@@ -1032,10 +1044,10 @@
/* subjectAltName entries of the server certificate */
xs = SSL_get_certificate(ssl);
if (xs) {
-if (modssl_X509_getSAN(p, xs, GEN_EMAIL, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_EMAIL, NID_undef, -1, entries)) {
extract_san_array(t, SSL_SERVER_SAN_Email, entries, p);
}
-if (modssl_X509_getSAN(p, xs, GEN_DNS, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_DNS, NID_undef, -1, entries)) {
extract_san_array(t, SSL_SERVER_SAN_DNS, entries, p);
}
/* no need to free xs (refcount does not increase) */
@@ -1044,10 +1056,10 @@
/* subjectAltName entries of the client certificate */
xs = SSL_get_peer_certificate(ssl);
if (xs) {
-if (modssl_X509_getSAN(p, xs, GEN_EMAIL, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_EMAIL, NID_undef, -1, entries)) {
extract_san_array(t, SSL_CLIENT_SAN_Email, entries, p);
}
-if (modssl_X509_getSAN(p, xs, GEN_DNS, -1, entries)) {
+if (modssl_X509_getSAN(p, xs, GEN_DNS, NID_undef, -1, entries)) {
extract_san_array(t, SSL_CLIENT_SAN_DNS, entries, p);
}
X509_free(xs);
Index: modules/ssl/ssl_util_ssl.c
===
--- modules/ssl/ssl_util_ssl.c (revision 1686420)
+++ modules/ssl/ssl_util_ssl.c (working copy)
@@ -258,8 +258,9 @@
* of the n-th occurrence of that type only. Currently supported types:
* GEN_EMAIL (rfc822Name)
* GEN_DNS (dNSName)
+ * GEN_OTHERNAME (otherName)
*/
-BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int idx,
+BOOL modssl_X509_getSAN(apr_pool_t *p, X509 *x509, int type, int nid, int idx,
apr_array_header_t **entries)
{
STACK_OF(GENERAL_NAME) *names;
@@ -287,10 +288,19 @@
APR_ARRAY_PUSH(*entries, const char *) = utf8str;
}
break;
+case GEN_OTHERNAME:
+if (name-d.otherName name-d.otherName-value
+ (nid == NID_undef
+
Re: Using UPN from subjectAltName with SSLUserName
On Thu, Jun 18, 2015 at 11:49 AM, Jan Pazdziora jpazdzi...@redhat.com wrote: I'd appreciate any comments about suitability of such change, as well as the implementation. Specifically, I'm not sure if people will prefer the generic and currently proposed SSL_CLIENT_SAN_otherName_n which gets any value of otherName type, or perhaps going with SSL_CLIENT_SAN_UPN_n and checking the OID just for the UPNs. Based on that decision I plan to then respin the patch with documentation changes included. I think a more generic way would to have something like SSL_CLIENT_OID_oid_n, so that we wouldn't have to add a new field each time. In this case, that would be: SSL_CLIENT_OID_1.3.6.1.4.1.311.20.2.3_n. Regards, Yann.
Using UPN from subjectAltName with SSLUserName
Hello, I've noticed that support for getting subjectAltName entries Email and Type landed in 2.4.13, via r1676087. We've come across another type in subjectAltName, Microsoft Universal Principal Name (OID 1.3.6.1.4.1.311.20.2.3) which would be useful to retrieve from the certificate and use for subsequent authorization and identity operations against Active Directory. I've filed https://bz.apache.org/bugzilla/show_bug.cgi?id=58020 When user authenticates with certificate which has their Microsoft Universal Principal Name in subjectAltName, that UPN cannot be used with SSLUserName for further access controls and included a patch which extends the original SAN support to otherName. I'd appreciate any comments about suitability of such change, as well as the implementation. Specifically, I'm not sure if people will prefer the generic and currently proposed SSL_CLIENT_SAN_otherName_n which gets any value of otherName type, or perhaps going with SSL_CLIENT_SAN_UPN_n and checking the OID just for the UPNs. Based on that decision I plan to then respin the patch with documentation changes included. Thank you, -- Jan Pazdziora Senior Principal Software Engineer, Identity Management Engineering, Red Hat
