[PATCH] CertificateManager, SubjectAlternativeName handling

While trying to diagnose problems I’ve been having with s2s tls, I came across the SubjectAlternativeName invalid type error and had a look at the code. I noticed that while looking for otherName attributes, it does no checking to see if an otherName is a XMPP otherName but just blindly assumes that it is. There was also some horrible looking code to unwrap the TaggedObject layers around the UTF8String in the XMPP otherName. I went ahead and added code to check the otherName objectId and only attempt to use those that match the XMPP otherName OID. I also cleaned up the unwrapping code so that it is more readable. Patch attached below as otherName.patch.

I haven’t seen any discussion on the board about the error message that is returned for the invalid otherName types, but it seems like the error message serves no real purpose other than to possibly confuse people. In my specific case, my certificate has a DNS otherName on it as well as the XMPP otherName, which is a valid combination and it shown on a jabber.org wiki page. The fact that OpenFire logs even a debug message complaining about a non-applicable type seems wrong to me. OpenFire should just silently ignore the types it doesn’t know/care about and move on. I’ve attached a second patch below, invalidType.patch, that will remove the debug log for the invalid type and change the comment to say that other types are not applicable and will be silently ignored.

I think the otherName patch needs to be applied because it cleans the code up and it also restricts exactly what type of otherName attributes that OpenFire attempts to use. The invalidType patch I would like to see applied, but that is more open to debate as someone else might see a use for that log line that I haven’t envisioned.

These patched were generated against the head of the Subversion trunk, but should apply cleanly to the release branch as well.

Thanks.

Ignore this post, original problem fixed.

EDIT: I had an attachment problem on the original message and the patches did not get attached originally. They made it into this post and then I figured out how to move the attachments to the original message but can’t delete this specific post now.

Message was edited by: jkf for clarification.

Hey Jason,

Ignoring patches then. Let us know if you are seeing issues with certificates.

Thanks,

– Gato

I’m sorry… That second post was misleading. I had a problem with Safari not posting the attachments so I made a second post with the attachments in a different browser and them added them to the original post. The original post is still valid.

Hey Jason,

From the XMPP RFC:

If a JID for any kind of XMPP entity (e.g., client or server) is represented in a certificate, it MUST be represented as a UTF8String within an otherName entity inside the subjectAltName, using the http://ASN.1 (CCITT, “Recommendation X.208: Specification of Abstract Syntax Notation One (ASN.1),” 1988.) Object Identifier “id-on-xmppAddr” specified in Section 5.1.1 (ASN.1 Object Identifier for XMPP Address) of this document.

and Section 5.1.1 looks like:

The http://ASN.1 (CCITT, “Recommendation X.208: Specification of Abstract Syntax Notation One (ASN.1),” 1988.) Object Identifier “id-on-xmppAddr” described above is defined as follows:

id-pkix OBJECT IDENTIFIER ::= { iso(1) identified-organization(3)

dod(6) internet(1) security(5) mechanisms(5) pkix(7) }

id-on OBJECT IDENTIFIER ::= { id-pkix 8 } – other name forms

id-on-xmppAddr OBJECT IDENTIFIER ::= { id-on 5 }

XmppAddr ::= UTF8String

This Object Identifier MAY also be represented in dotted display format as “1.3.6.1.5.5.7.8.5”.

Note the MAY word in the display format as “1.3.6.1.5.5.7.8.5”. That means that it is valid to not use that display format in certificates and your patch is relying on that display format to recognize XMPP extensions. Anyway, is your patch fixing some error or is it another way of solving the same problem?

Thanks,

– Gato

The ASN.1 specification they present builds that exact OID. The MUST in the first quote says that that OID MUST be used to represent JIDs in a certificate. The MAY does not mean that other OIDs can be used to represent xmppAddr attributes, but that the OID itself may be represented in that format. That is also the format that the BouncyCastle library chooses to return the objectIdentifer of an otherName attribute. So I don’t see a problem there.

I was led to that code by the invalid type debug message while trying to troubleshoot a s2s tls problem. The code for that particular function is hideous and I thought I would lend a helping hand to an open-source project by cleaning it up and making it more understandable/maintainable. The fact that the code would accept ANY otherName and attempt to decode it as a UTF8String is also wrong. It should be looking specifically for xmppAddr otherNames before trying to decode, which is what my patch implements. Otherwise I can make your code throw ClassCastExceptions at will just by creating a cert with an otherName on it that has a non-UTF8String type and all I have to do is make your server access my cert to do it. By using DERUTF8String.getInstance to do the unwrapping, if it finds something other than a TaggedObject or a UTF8String, it will throw an exception of its own that can be caught and dealt with. Now that I think about it, the patch needs that added to cover that base. While my patch does not directly address any problems or issues I might be having, either consider it a bug fix by making the code more robust, or an enhancement by making it cleaner.

I’ve recently started using OpenFire as my home IM server after finding ejabberd a pain to configure. Being a software developer, I like to dig into the code to see how things work, and if I see something that is wrong or could be done better, I have no issues with submitting patches to make open-source projects I’m interested in better.

Just to follow up on this for the last time, is there any interest in the patches I’ve submitted? They not only fix potential, or hidden, problems that people may be having with XMPP otherNames on their certificates, but it also cleans the code up and makes it more maintainable and readable.

Thanks

Hey Jason,

FYI, I finally applied your patches for Openfire 3.6.3.

Thanks,

– Gato