[reSIProcate] Proposed changes to support optional RFC 5922 TLS Certificate matching rules
Aron Rosenberg
arosenberg at logitech.com
Sun Mar 13 17:05:32 CDT 2011
Based on Scott's feedback, here is a revised patch. It changes the setting
name and moves the setting into the BaseSecurity class, which is probably a
better place for the set function since users have to create a Security
object to use TLS anyway.
-Aron
Index: resip/stack/ssl/Security.cxx
===================================================================
--- resip/stack/ssl/Security.cxx (revision 9050)
+++ resip/stack/ssl/Security.cxx (working copy)
@@ -171,6 +171,8 @@
}
+// .amr. RFC 5922 mandates exact match only on certificates, so this is the
default, but RFC 2459 and RFC 3261 don't prevent wildcards, so disable if
you want that mode.
+bool BaseSecurity::mAllowWildcardCertificates = false;
BaseSecurity::CipherList
BaseSecurity::ExportableSuite("!SSLv2:aRSA+AES:aDSS+AES:@STRENGTH
:aRSA+3DES:aDSS+3DES:aRSA+RC4+MEDIUM:aDSS+RC4+MEDIUM:aRSA+DES:aDSS+DES:aRSA+RC4:aDSS+RC4");
BaseSecurity::CipherList
BaseSecurity::StrongestSuite("!SSLv2:aRSA+AES:aDSS+AES:@STRENGTH
:aRSA+3DES:aDSS+3DES");
@@ -2434,12 +2436,22 @@
return Data::Empty;
}
/**
- Matchtes subjectAltName and cnames
- @todo looks incomplete, make better
+ Applies the certificate and domain name matching rules
*/
int
BaseSecurity::matchHostName(const Data& certificateName, const Data&
domainName)
{
+ if(mAllowWildcardCertificates)
+ return matchHostNameWithWildcards(certificateName,domainName);
+ return isEqualNoCase(certificateName,domainName);
+}
+/**
+ Does a wildcard match on domain and certificate name
+ @todo looks incomplete, make better
+*/
+int
+BaseSecurity::matchHostNameWithWildcards(const Data& certificateName, const
Data& domainName)
+{
const char *dot = NULL;
const char *certName = certificateName.c_str();
Index: resip/stack/ssl/Security.hxx
===================================================================
--- resip/stack/ssl/Security.hxx (revision 9050)
+++ resip/stack/ssl/Security.hxx (working copy)
@@ -167,11 +167,15 @@
static bool isSelfSigned(const X509* cert);
- // match with wildcards
static int matchHostName(const Data& certificateName, const Data&
domainName);
// allow particular classes to acces the functions below
// friend class TlsConnection;
+
+ // Allow overriding of RFC 5922 rules on certificate matching.
+ static void setAllowWildcardCertificates(bool bEnable) {
mAllowWildcardCertificates = bEnable; }
+ static bool allowWildcardCertificates() { return
mAllowWildcardCertificates; }
+
public:
SSL_CTX* getTlsCtx ();
SSL_CTX* getSslCtx ();
@@ -216,6 +220,10 @@
Data getPrivateKeyPEM (PEMType type, const Data& name) const;
Data getPrivateKeyDER (PEMType type, const Data& name) const;
void addPrivateKeyPKEY(PEMType type, const Data& name, EVP_PKEY*
pKey, bool write) const;
+
+ // match with wildcards
+ static int matchHostNameWithWildcards(const Data& certificateName,
const Data& domainName);
+ static bool mAllowWildcardCertificates;
};
class Security : public BaseSecurity
Index: resip/stack/ssl/TlsConnection.cxx
===================================================================
--- resip/stack/ssl/TlsConnection.cxx (revision 9050)
+++ resip/stack/ssl/TlsConnection.cxx (working copy)
@@ -280,25 +280,12 @@
bool matches = false;
for(std::list<BaseSecurity::PeerName>::iterator it =
mPeerNames.begin(); it != mPeerNames.end(); it++)
{
- if(it->mType == BaseSecurity::CommonName)
+ if(BaseSecurity::matchHostName(it->mName,
who().getTargetDomain()))
{
- //allow wildcard match for subdomain name (RFC 2459)
- if(BaseSecurity::matchHostName(it->mName,
who().getTargetDomain()))
- {
- matches=true;
- break;
- }
- }
- else //it->mType == SubjectAltName
- {
- //no wildcards for SubjectAltName
- if(isEqualNoCase(it->mName, who().getTargetDomain()))
- {
matches=true;
break;
}
}
- }
if(!matches)
{
mTlsState = Broken;
On Sun, Mar 13, 2011 at 1:50 PM, Scott Godin <sgodin at sipspectrum.com> wrote:
> Hi Aron,
>
> I think the default needs to be flipped. I think RFC5922 support should be
> enabled by default, and you must explicitly turn the option off to get the
> non-RFC compliant behavior.
>
> I am a little concerned about people relying on wildcards in the
> CommonName today - but as long as we document the need to use the following
> setting - I think we are good:
> TlsConnection::mRFC5922CertificateRules = false;
>
> Scott
>
> ---------- Forwarded message ----------
> From: Aron Rosenberg <arosenberg at logitech.com>
> Date: Sun, Mar 13, 2011 at 4:31 PM
> Subject: [reSIProcate] Proposed changes to support optional RFC 5922 TLS
> Certificate matching rules
> To: resiprocate-devel <resiprocate-devel at resiprocate.org>
>
>
> All,
>
> Below is a proposed patch to allow RFC 5922 TLS certificate common and
> subj-alt-name matching rules. RFC 5922 mandates that wildcard matching MUST
> NOT be used. However, current resip code allows wildcards on the common name
> field.
>
> By default, this new mode is disabled. As part of this patch, the non 5922
> mode is changed to allow wildcard matching in all fields, since this mode is
> allowed within HTTP over TLS, not prohibited by 3261(and normal web browsers
> allow it too.
>
> If there are no complaints, I will commit sometime this week.
>
> -Aron
>
> Index: resip/stack/ssl/TlsConnection.cxx
> ===================================================================
> --- resip/stack/ssl/TlsConnection.cxx (revision 9058)
> +++ resip/stack/ssl/TlsConnection.cxx (working copy)
> @@ -24,6 +24,8 @@
>
> #define RESIPROCATE_SUBSYSTEM Subsystem::TRANSPORT
>
> +bool TlsConnection::mRFC5922CertificateRules = false;
> +
> TlsConnection::TlsConnection( Transport* transport, const Tuple& tuple,
> Socket fd, Security* security,
> bool server, Data domain,
> SecurityTypes::SSLType sslType ,
> @@ -280,25 +282,21 @@
> bool matches = false;
> for(std::list<BaseSecurity::PeerName>::iterator it =
> mPeerNames.begin(); it != mPeerNames.end(); it++)
> {
> - if(it->mType == BaseSecurity::CommonName)
> + if(mRFC5922CertificateRules)
> {
> - //allow wildcard match for subdomain name (RFC 2459)
> - if(BaseSecurity::matchHostName(it->mName,
> who().getTargetDomain()))
> + if(isEqualNoCase(it->mName, who().getTargetDomain()))
> {
> matches=true;
> break;
> }
> }
> - else //it->mType == SubjectAltName
> - {
> - //no wildcards for SubjectAltName
> - if(isEqualNoCase(it->mName, who().getTargetDomain()))
> + //.amr. allow wildcard match (RFC 2459), RFC 3261 doesn't prevent
> wildcards.
> + else if(BaseSecurity::matchHostName(it->mName,
> who().getTargetDomain()))
> {
> matches=true;
> break;
> }
> }
> - }
> if(!matches)
> {
> mTlsState = Broken;
> Index: resip/stack/ssl/TlsConnection.hxx
> ===================================================================
> --- resip/stack/ssl/TlsConnection.hxx (revision 9057)
> +++ resip/stack/ssl/TlsConnection.hxx (working copy)
> @@ -48,6 +48,8 @@
>
> typedef enum TlsState { Initial, Broken, Handshaking, Up } TlsState;
> static const char * fromState(TlsState);
> + static bool RFC5922CertificateRules() { return
> mRFC5922CertificateRules; }
> + static void setRFC5922CertificateRules(bool bEnable) {
> mRFC5922CertificateRules = bEnable; }
>
> private:
> /// No default c'tor
> @@ -67,6 +69,7 @@
> SSL* mSsl;
> BIO* mBio;
> std::list<BaseSecurity::PeerName> mPeerNames;
> + static bool mRFC5922CertificateRules;
> };
>
> }
>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20110313/fe701e27/attachment.htm>
More information about the resiprocate-devel
mailing list