[RFC][PATCH 0/3] CIFS: Use OID registry and ASN.1 header

[RFC][PATCH 0/3] CIFS: Use OID registry and ASN.1 header

[David Howells]

Hi Steve,

Here are some patches that might be of interest for CIFS:

 (1) The first patch uses the OID registry I created for module signing's use
     of X.509 certs and potentially PKCS#7.  The lookup function finds the OIDs
     from its table (using a hash value to speed up the binary search) and maps
     known OIDs onto values in an enum.

 (2) The second and third patches make CIFS use the linux/asn1.h file for
     common ASN.1 constant definitions.  Note that ASN1_OJI is now ASN1_OID
     since 'OBJECT IDENTIFIER' is almost always abbreviated to 'OID'.

David
---
David Howells (3):
      CIFS: Use linux/asn1.h
      ASN.1: Define indefinite length marker constant
      CIFS: Use OID registry facility


 fs/cifs/Kconfig              |    1 
 fs/cifs/asn1.c               |  230 +++++++++---------------------------------
 include/linux/oid_registry.h |    6 +
 3 files changed, 59 insertions(+), 178 deletions(-)

[PATCH 1/3] CIFS: Use OID registry facility

[David Howells]

Use the OID registry facility from fs/cifs/asn1.c as that converts known OIDs
into an enum which should speed up comparison.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 fs/cifs/Kconfig              |    1 
 fs/cifs/asn1.c               |  156 ++++++++----------------------------------
 include/linux/oid_registry.h |    6 ++
 3 files changed, 36 insertions(+), 127 deletions(-)

diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
index 2075ddf..0f7a0a7 100644
--- a/fs/cifs/Kconfig
+++ b/fs/cifs/Kconfig
@@ -10,6 +10,7 @@ config CIFS
 	select CRYPTO_ECB
 	select CRYPTO_DES
 	select CRYPTO_SHA256
+	select OID_REGISTRY
 	help
 	  This is the client VFS module for the Common Internet File System
 	  (CIFS) protocol which is the successor to the Server Message Block
diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
index cfd1ce3..3a7b2b6 100644
--- a/fs/cifs/asn1.c
+++ b/fs/cifs/asn1.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
+#include <linux/oid_registry.h>
 #include "cifspdu.h"
 #include "cifsglob.h"
 #include "cifs_debug.h"
@@ -76,17 +77,6 @@
 #define ASN1_ERR_DEC_LENGTH_MISMATCH	4
 #define ASN1_ERR_DEC_BADVALUE		5
 
-#define SPNEGO_OID_LEN 7
-#define NTLMSSP_OID_LEN  10
-#define KRB5_OID_LEN  7
-#define KRB5U2U_OID_LEN  8
-#define MSKRB5_OID_LEN  7
-static unsigned long SPNEGO_OID[7] = { 1, 3, 6, 1, 5, 5, 2 };
-static unsigned long NTLMSSP_OID[10] = { 1, 3, 6, 1, 4, 1, 311, 2, 2, 10 };
-static unsigned long KRB5_OID[7] = { 1, 2, 840, 113554, 1, 2, 2 };
-static unsigned long KRB5U2U_OID[8] = { 1, 2, 840, 113554, 1, 2, 2, 3 };
-static unsigned long MSKRB5_OID[7] = { 1, 2, 840, 48018, 1, 2, 2 };
-
 /*
  * ASN.1 context.
  */
@@ -397,95 +387,10 @@ asn1_octets_decode(struct asn1_ctx *ctx,
 	return 1;
 } */
 
-static unsigned char
-asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid)
+static enum OID
+asn1_oid_decode(struct asn1_ctx *ctx, unsigned char *eoc)
 {
-	unsigned char ch;
-
-	*subid = 0;
-
-	do {
-		if (!asn1_octet_decode(ctx, &ch))
-			return 0;
-
-		*subid <<= 7;
-		*subid |= ch & 0x7F;
-	} while ((ch & 0x80) == 0x80);
-	return 1;
-}
-
-static int
-asn1_oid_decode(struct asn1_ctx *ctx,
-		unsigned char *eoc, unsigned long **oid, unsigned int *len)
-{
-	unsigned long subid;
-	unsigned int size;
-	unsigned long *optr;
-
-	size = eoc - ctx->pointer + 1;
-
-	/* first subid actually encodes first two subids */
-	if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
-		return 0;
-
-	*oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);
-	if (*oid == NULL)
-		return 0;
-
-	optr = *oid;
-
-	if (!asn1_subid_decode(ctx, &subid)) {
-		kfree(*oid);
-		*oid = NULL;
-		return 0;
-	}
-
-	if (subid < 40) {
-		optr[0] = 0;
-		optr[1] = subid;
-	} else if (subid < 80) {
-		optr[0] = 1;
-		optr[1] = subid - 40;
-	} else {
-		optr[0] = 2;
-		optr[1] = subid - 80;
-	}
-
-	*len = 2;
-	optr += 2;
-
-	while (ctx->pointer < eoc) {
-		if (++(*len) > size) {
-			ctx->error = ASN1_ERR_DEC_BADVALUE;
-			kfree(*oid);
-			*oid = NULL;
-			return 0;
-		}
-
-		if (!asn1_subid_decode(ctx, optr++)) {
-			kfree(*oid);
-			*oid = NULL;
-			return 0;
-		}
-	}
-	return 1;
-}
-
-static int
-compare_oid(unsigned long *oid1, unsigned int oid1len,
-	    unsigned long *oid2, unsigned int oid2len)
-{
-	unsigned int i;
-
-	if (oid1len != oid2len)
-		return 0;
-	else {
-		for (i = 0; i < oid1len; i++) {
-			if (oid1[i] != oid2[i])
-				return 0;
-		}
-		return 1;
-	}
+	return look_up_OID(ctx->pointer, eoc - ctx->pointer);
 }
 
 	/* BB check for endian conversion issues here */
@@ -497,8 +402,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	struct asn1_ctx ctx;
 	unsigned char *end;
 	unsigned char *sequence_end;
-	unsigned long *oid = NULL;
-	unsigned int cls, con, tag, oidlen, rc;
+	unsigned int cls, con, tag, rc;
 
 	/* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
 
@@ -519,12 +423,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (rc) {
 		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
 		    (cls == ASN1_UNI)) {
-			rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
-			if (rc) {
-				rc = compare_oid(oid, oidlen, SPNEGO_OID,
-						 SPNEGO_OID_LEN);
-				kfree(oid);
-			}
+			if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
+				rc = 0;
 		} else
 			rc = 0;
 	}
@@ -588,26 +488,28 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 			return 0;
 		}
 		if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
-			if (asn1_oid_decode(&ctx, end, &oid, &oidlen)) {
-
-				cFYI(1, "OID len = %d oid = 0x%lx 0x%lx "
-					"0x%lx 0x%lx", oidlen, *oid,
-					*(oid + 1), *(oid + 2), *(oid + 3));
-
-				if (compare_oid(oid, oidlen, MSKRB5_OID,
-						MSKRB5_OID_LEN))
-					server->sec_mskerberos = true;
-				else if (compare_oid(oid, oidlen, KRB5U2U_OID,
-						     KRB5U2U_OID_LEN))
-					server->sec_kerberosu2u = true;
-				else if (compare_oid(oid, oidlen, KRB5_OID,
-						     KRB5_OID_LEN))
-					server->sec_kerberos = true;
-				else if (compare_oid(oid, oidlen, NTLMSSP_OID,
-						     NTLMSSP_OID_LEN))
-					server->sec_ntlmssp = true;
-
-				kfree(oid);
+			enum OID oid = asn1_oid_decode(&ctx, end);
+			if (oid != OID__NR)
+				cFYI(1, "OID oid = %u", oid);
+			switch (oid) {
+			case OID_MSKRB5:
+				server->sec_mskerberos = true;
+				break;
+			case OID_KRB5U2U:
+				server->sec_kerberosu2u = true;
+				break;
+			case OID_KRB5:
+				server->sec_kerberos = true;
+				break;
+			case OID_NTLMSSP:
+				server->sec_ntlmssp = true;
+				break;
+			case OID__NR:
+				cFYI(1, "OID len = %ld oid = %*pX",
+				     end - ctx.pointer,
+				     (int)(end - ctx.pointer), ctx.pointer);
+			default:
+				break;
 			}
 		} else {
 			cFYI(1, "Should be an oid what is going on?");
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index 6926db7..4bff9c5 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -82,6 +82,12 @@ enum OID {
 	OID_authorityKeyIdentifier,	/* 2.5.29.35 */
 	OID_extKeyUsage,		/* 2.5.29.37 */
 
+	OID_SPNEGO,			/* 1.3.6.1.5.5.2 */
+	OID_NTLMSSP,			/* 1.3.6.1.4.1.311.2.2.10 */
+	OID_KRB5,			/* 1.2.840.113554.1.2.2 */
+	OID_KRB5U2U,			/* 1.2.840.113554.1.2.2.3 */
+	OID_MSKRB5,			/* 1.2.840.48018.1.2.2 */
+
 	OID__NR
 };
 

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Jeff Layton]

On Mon, 22 Oct 2012 15:23:26 +0100
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Use the OID registry facility from fs/cifs/asn1.c as that converts known OIDs
> into an enum which should speed up comparison.
> 
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
>  fs/cifs/Kconfig              |    1 
>  fs/cifs/asn1.c               |  156 ++++++++----------------------------------
>  include/linux/oid_registry.h |    6 ++
>  3 files changed, 36 insertions(+), 127 deletions(-)
> 
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 2075ddf..0f7a0a7 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -10,6 +10,7 @@ config CIFS
>  	select CRYPTO_ECB
>  	select CRYPTO_DES
>  	select CRYPTO_SHA256
> +	select OID_REGISTRY
>  	help
>  	  This is the client VFS module for the Common Internet File System
>  	  (CIFS) protocol which is the successor to the Server Message Block
> diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
> index cfd1ce3..3a7b2b6 100644
> --- a/fs/cifs/asn1.c
> +++ b/fs/cifs/asn1.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> +#include <linux/oid_registry.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
>  #include "cifs_debug.h"
> @@ -76,17 +77,6 @@
>  #define ASN1_ERR_DEC_LENGTH_MISMATCH	4
>  #define ASN1_ERR_DEC_BADVALUE		5
>  
> -#define SPNEGO_OID_LEN 7
> -#define NTLMSSP_OID_LEN  10
> -#define KRB5_OID_LEN  7
> -#define KRB5U2U_OID_LEN  8
> -#define MSKRB5_OID_LEN  7
> -static unsigned long SPNEGO_OID[7] = { 1, 3, 6, 1, 5, 5, 2 };
> -static unsigned long NTLMSSP_OID[10] = { 1, 3, 6, 1, 4, 1, 311, 2, 2, 10 };
> -static unsigned long KRB5_OID[7] = { 1, 2, 840, 113554, 1, 2, 2 };
> -static unsigned long KRB5U2U_OID[8] = { 1, 2, 840, 113554, 1, 2, 2, 3 };
> -static unsigned long MSKRB5_OID[7] = { 1, 2, 840, 48018, 1, 2, 2 };
> -
>  /*
>   * ASN.1 context.
>   */
> @@ -397,95 +387,10 @@ asn1_octets_decode(struct asn1_ctx *ctx,
>  	return 1;
>  } */
>  
> -static unsigned char
> -asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid)
> +static enum OID
> +asn1_oid_decode(struct asn1_ctx *ctx, unsigned char *eoc)
>  {
> -	unsigned char ch;
> -
> -	*subid = 0;
> -
> -	do {
> -		if (!asn1_octet_decode(ctx, &ch))
> -			return 0;
> -
> -		*subid <<= 7;
> -		*subid |= ch & 0x7F;
> -	} while ((ch & 0x80) == 0x80);
> -	return 1;
> -}
> -
> -static int
> -asn1_oid_decode(struct asn1_ctx *ctx,
> -		unsigned char *eoc, unsigned long **oid, unsigned int *len)
> -{
> -	unsigned long subid;
> -	unsigned int size;
> -	unsigned long *optr;
> -
> -	size = eoc - ctx->pointer + 1;
> -
> -	/* first subid actually encodes first two subids */
> -	if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
> -		return 0;
> -
> -	*oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);
> -	if (*oid == NULL)
> -		return 0;
> -
> -	optr = *oid;
> -
> -	if (!asn1_subid_decode(ctx, &subid)) {
> -		kfree(*oid);
> -		*oid = NULL;
> -		return 0;
> -	}
> -
> -	if (subid < 40) {
> -		optr[0] = 0;
> -		optr[1] = subid;
> -	} else if (subid < 80) {
> -		optr[0] = 1;
> -		optr[1] = subid - 40;
> -	} else {
> -		optr[0] = 2;
> -		optr[1] = subid - 80;
> -	}
> -
> -	*len = 2;
> -	optr += 2;
> -
> -	while (ctx->pointer < eoc) {
> -		if (++(*len) > size) {
> -			ctx->error = ASN1_ERR_DEC_BADVALUE;
> -			kfree(*oid);
> -			*oid = NULL;
> -			return 0;
> -		}
> -
> -		if (!asn1_subid_decode(ctx, optr++)) {
> -			kfree(*oid);
> -			*oid = NULL;
> -			return 0;
> -		}
> -	}
> -	return 1;
> -}
> -
> -static int
> -compare_oid(unsigned long *oid1, unsigned int oid1len,
> -	    unsigned long *oid2, unsigned int oid2len)
> -{
> -	unsigned int i;
> -
> -	if (oid1len != oid2len)
> -		return 0;
> -	else {
> -		for (i = 0; i < oid1len; i++) {
> -			if (oid1[i] != oid2[i])
> -				return 0;
> -		}
> -		return 1;
> -	}
> +	return look_up_OID(ctx->pointer, eoc - ctx->pointer);
>  }
>  
>  	/* BB check for endian conversion issues here */
> @@ -497,8 +402,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	struct asn1_ctx ctx;
>  	unsigned char *end;
>  	unsigned char *sequence_end;
> -	unsigned long *oid = NULL;
> -	unsigned int cls, con, tag, oidlen, rc;
> +	unsigned int cls, con, tag, rc;
>  
>  	/* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
>  
> @@ -519,12 +423,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (rc) {
>  		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
>  		    (cls == ASN1_UNI)) {
> -			rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
> -			if (rc) {
> -				rc = compare_oid(oid, oidlen, SPNEGO_OID,
> -						 SPNEGO_OID_LEN);
> -				kfree(oid);
> -			}
> +			if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
> +				rc = 0;
>  		} else
>  			rc = 0;
>  	}
> @@ -588,26 +488,28 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  			return 0;
>  		}
>  		if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
> -			if (asn1_oid_decode(&ctx, end, &oid, &oidlen)) {
> -
> -				cFYI(1, "OID len = %d oid = 0x%lx 0x%lx "
> -					"0x%lx 0x%lx", oidlen, *oid,
> -					*(oid + 1), *(oid + 2), *(oid + 3));
> -
> -				if (compare_oid(oid, oidlen, MSKRB5_OID,
> -						MSKRB5_OID_LEN))
> -					server->sec_mskerberos = true;
> -				else if (compare_oid(oid, oidlen, KRB5U2U_OID,
> -						     KRB5U2U_OID_LEN))
> -					server->sec_kerberosu2u = true;
> -				else if (compare_oid(oid, oidlen, KRB5_OID,
> -						     KRB5_OID_LEN))
> -					server->sec_kerberos = true;
> -				else if (compare_oid(oid, oidlen, NTLMSSP_OID,
> -						     NTLMSSP_OID_LEN))
> -					server->sec_ntlmssp = true;
> -
> -				kfree(oid);
> +			enum OID oid = asn1_oid_decode(&ctx, end);
> +			if (oid != OID__NR)
> +				cFYI(1, "OID oid = %u", oid);
> +			switch (oid) {
> +			case OID_MSKRB5:
> +				server->sec_mskerberos = true;
> +				break;
> +			case OID_KRB5U2U:
> +				server->sec_kerberosu2u = true;
> +				break;
> +			case OID_KRB5:
> +				server->sec_kerberos = true;
> +				break;
> +			case OID_NTLMSSP:
> +				server->sec_ntlmssp = true;
> +				break;
> +			case OID__NR:
> +				cFYI(1, "OID len = %ld oid = %*pX",
> +				     end - ctx.pointer,
> +				     (int)(end - ctx.pointer), ctx.pointer);
> +			default:
> +				break;
>  			}
>  		} else {
>  			cFYI(1, "Should be an oid what is going on?");
> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> index 6926db7..4bff9c5 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -82,6 +82,12 @@ enum OID {
>  	OID_authorityKeyIdentifier,	/* 2.5.29.35 */
>  	OID_extKeyUsage,		/* 2.5.29.37 */
>  
> +	OID_SPNEGO,			/* 1.3.6.1.5.5.2 */
> +	OID_NTLMSSP,			/* 1.3.6.1.4.1.311.2.2.10 */
> +	OID_KRB5,			/* 1.2.840.113554.1.2.2 */
> +	OID_KRB5U2U,			/* 1.2.840.113554.1.2.2.3 */
> +	OID_MSKRB5,			/* 1.2.840.48018.1.2.2 */
> +
>  	OID__NR
>  };
>  
> 

In general, I'm all for moving CIFS out of the business of implementing
ASN.1 like this. This last bit of this patch was a bit confusing though.
It left me wondering where the actual definitions of these OIDs go.

It looks though like you're using a script to scrape the comments out
of the above enum to generate C files. That would certainly work, but
it seems like a fragile solution. Minor whitespace munging (like an
extra newline in there) could throw off the parsing...

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Shirish Pargaonkar]

On Mon, Oct 22, 2012 at 9:23 AM, David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Use the OID registry facility from fs/cifs/asn1.c as that converts known OIDs
> into an enum which should speed up comparison.
>
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>
>  fs/cifs/Kconfig              |    1
>  fs/cifs/asn1.c               |  156 ++++++++----------------------------------
>  include/linux/oid_registry.h |    6 ++
>  3 files changed, 36 insertions(+), 127 deletions(-)
>
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 2075ddf..0f7a0a7 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -10,6 +10,7 @@ config CIFS
>         select CRYPTO_ECB
>         select CRYPTO_DES
>         select CRYPTO_SHA256
> +       select OID_REGISTRY

How do I incorporate/add OID_REGISTRY as a config option?
I do not see that in my .config (3.7)

>         help
>           This is the client VFS module for the Common Internet File System
>           (CIFS) protocol which is the successor to the Server Message Block
> diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
> index cfd1ce3..3a7b2b6 100644
> --- a/fs/cifs/asn1.c
> +++ b/fs/cifs/asn1.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> +#include <linux/oid_registry.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
>  #include "cifs_debug.h"
> @@ -76,17 +77,6 @@
>  #define ASN1_ERR_DEC_LENGTH_MISMATCH   4
>  #define ASN1_ERR_DEC_BADVALUE          5
>
> -#define SPNEGO_OID_LEN 7
> -#define NTLMSSP_OID_LEN  10
> -#define KRB5_OID_LEN  7
> -#define KRB5U2U_OID_LEN  8
> -#define MSKRB5_OID_LEN  7
> -static unsigned long SPNEGO_OID[7] = { 1, 3, 6, 1, 5, 5, 2 };
> -static unsigned long NTLMSSP_OID[10] = { 1, 3, 6, 1, 4, 1, 311, 2, 2, 10 };
> -static unsigned long KRB5_OID[7] = { 1, 2, 840, 113554, 1, 2, 2 };
> -static unsigned long KRB5U2U_OID[8] = { 1, 2, 840, 113554, 1, 2, 2, 3 };
> -static unsigned long MSKRB5_OID[7] = { 1, 2, 840, 48018, 1, 2, 2 };
> -
>  /*
>   * ASN.1 context.
>   */
> @@ -397,95 +387,10 @@ asn1_octets_decode(struct asn1_ctx *ctx,
>         return 1;
>  } */
>
> -static unsigned char
> -asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid)
> +static enum OID
> +asn1_oid_decode(struct asn1_ctx *ctx, unsigned char *eoc)
>  {
> -       unsigned char ch;
> -
> -       *subid = 0;
> -
> -       do {
> -               if (!asn1_octet_decode(ctx, &ch))
> -                       return 0;
> -
> -               *subid <<= 7;
> -               *subid |= ch & 0x7F;
> -       } while ((ch & 0x80) == 0x80);
> -       return 1;
> -}
> -
> -static int
> -asn1_oid_decode(struct asn1_ctx *ctx,
> -               unsigned char *eoc, unsigned long **oid, unsigned int *len)
> -{
> -       unsigned long subid;
> -       unsigned int size;
> -       unsigned long *optr;
> -
> -       size = eoc - ctx->pointer + 1;
> -
> -       /* first subid actually encodes first two subids */
> -       if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
> -               return 0;
> -
> -       *oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);
> -       if (*oid == NULL)
> -               return 0;
> -
> -       optr = *oid;
> -
> -       if (!asn1_subid_decode(ctx, &subid)) {
> -               kfree(*oid);
> -               *oid = NULL;
> -               return 0;
> -       }
> -
> -       if (subid < 40) {
> -               optr[0] = 0;
> -               optr[1] = subid;
> -       } else if (subid < 80) {
> -               optr[0] = 1;
> -               optr[1] = subid - 40;
> -       } else {
> -               optr[0] = 2;
> -               optr[1] = subid - 80;
> -       }
> -
> -       *len = 2;
> -       optr += 2;
> -
> -       while (ctx->pointer < eoc) {
> -               if (++(*len) > size) {
> -                       ctx->error = ASN1_ERR_DEC_BADVALUE;
> -                       kfree(*oid);
> -                       *oid = NULL;
> -                       return 0;
> -               }
> -
> -               if (!asn1_subid_decode(ctx, optr++)) {
> -                       kfree(*oid);
> -                       *oid = NULL;
> -                       return 0;
> -               }
> -       }
> -       return 1;
> -}
> -
> -static int
> -compare_oid(unsigned long *oid1, unsigned int oid1len,
> -           unsigned long *oid2, unsigned int oid2len)
> -{
> -       unsigned int i;
> -
> -       if (oid1len != oid2len)
> -               return 0;
> -       else {
> -               for (i = 0; i < oid1len; i++) {
> -                       if (oid1[i] != oid2[i])
> -                               return 0;
> -               }
> -               return 1;
> -       }
> +       return look_up_OID(ctx->pointer, eoc - ctx->pointer);
>  }
>
>         /* BB check for endian conversion issues here */
> @@ -497,8 +402,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>         struct asn1_ctx ctx;
>         unsigned char *end;
>         unsigned char *sequence_end;
> -       unsigned long *oid = NULL;
> -       unsigned int cls, con, tag, oidlen, rc;
> +       unsigned int cls, con, tag, rc;
>
>         /* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
>
> @@ -519,12 +423,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>         if (rc) {
>                 if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
>                     (cls == ASN1_UNI)) {
> -                       rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
> -                       if (rc) {
> -                               rc = compare_oid(oid, oidlen, SPNEGO_OID,
> -                                                SPNEGO_OID_LEN);
> -                               kfree(oid);
> -                       }
> +                       if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
> +                               rc = 0;
>                 } else
>                         rc = 0;
>         }
> @@ -588,26 +488,28 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>                         return 0;
>                 }
>                 if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
> -                       if (asn1_oid_decode(&ctx, end, &oid, &oidlen)) {
> -
> -                               cFYI(1, "OID len = %d oid = 0x%lx 0x%lx "
> -                                       "0x%lx 0x%lx", oidlen, *oid,
> -                                       *(oid + 1), *(oid + 2), *(oid + 3));
> -
> -                               if (compare_oid(oid, oidlen, MSKRB5_OID,
> -                                               MSKRB5_OID_LEN))
> -                                       server->sec_mskerberos = true;
> -                               else if (compare_oid(oid, oidlen, KRB5U2U_OID,
> -                                                    KRB5U2U_OID_LEN))
> -                                       server->sec_kerberosu2u = true;
> -                               else if (compare_oid(oid, oidlen, KRB5_OID,
> -                                                    KRB5_OID_LEN))
> -                                       server->sec_kerberos = true;
> -                               else if (compare_oid(oid, oidlen, NTLMSSP_OID,
> -                                                    NTLMSSP_OID_LEN))
> -                                       server->sec_ntlmssp = true;
> -
> -                               kfree(oid);
> +                       enum OID oid = asn1_oid_decode(&ctx, end);
> +                       if (oid != OID__NR)
> +                               cFYI(1, "OID oid = %u", oid);
> +                       switch (oid) {
> +                       case OID_MSKRB5:
> +                               server->sec_mskerberos = true;
> +                               break;
> +                       case OID_KRB5U2U:
> +                               server->sec_kerberosu2u = true;
> +                               break;
> +                       case OID_KRB5:
> +                               server->sec_kerberos = true;
> +                               break;
> +                       case OID_NTLMSSP:
> +                               server->sec_ntlmssp = true;
> +                               break;
> +                       case OID__NR:
> +                               cFYI(1, "OID len = %ld oid = %*pX",
> +                                    end - ctx.pointer,
> +                                    (int)(end - ctx.pointer), ctx.pointer);
> +                       default:
> +                               break;
>                         }
>                 } else {
>                         cFYI(1, "Should be an oid what is going on?");
> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> index 6926db7..4bff9c5 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -82,6 +82,12 @@ enum OID {
>         OID_authorityKeyIdentifier,     /* 2.5.29.35 */
>         OID_extKeyUsage,                /* 2.5.29.37 */
>
> +       OID_SPNEGO,                     /* 1.3.6.1.5.5.2 */
> +       OID_NTLMSSP,                    /* 1.3.6.1.4.1.311.2.2.10 */
> +       OID_KRB5,                       /* 1.2.840.113554.1.2.2 */
> +       OID_KRB5U2U,                    /* 1.2.840.113554.1.2.2.3 */
> +       OID_MSKRB5,                     /* 1.2.840.48018.1.2.2 */
> +
>         OID__NR
>  };
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Jeff Layton]

On Mon, 22 Oct 2012 15:23:26 +0100
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Use the OID registry facility from fs/cifs/asn1.c as that converts known OIDs
> into an enum which should speed up comparison.
> 
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
>  fs/cifs/Kconfig              |    1 
>  fs/cifs/asn1.c               |  156 ++++++++----------------------------------
>  include/linux/oid_registry.h |    6 ++
>  3 files changed, 36 insertions(+), 127 deletions(-)
> 
> diff --git a/fs/cifs/Kconfig b/fs/cifs/Kconfig
> index 2075ddf..0f7a0a7 100644
> --- a/fs/cifs/Kconfig
> +++ b/fs/cifs/Kconfig
> @@ -10,6 +10,7 @@ config CIFS
>  	select CRYPTO_ECB
>  	select CRYPTO_DES
>  	select CRYPTO_SHA256
> +	select OID_REGISTRY
>  	help
>  	  This is the client VFS module for the Common Internet File System
>  	  (CIFS) protocol which is the successor to the Server Message Block
> diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
> index cfd1ce3..3a7b2b6 100644
> --- a/fs/cifs/asn1.c
> +++ b/fs/cifs/asn1.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> +#include <linux/oid_registry.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
>  #include "cifs_debug.h"
> @@ -76,17 +77,6 @@
>  #define ASN1_ERR_DEC_LENGTH_MISMATCH	4
>  #define ASN1_ERR_DEC_BADVALUE		5
>  
> -#define SPNEGO_OID_LEN 7
> -#define NTLMSSP_OID_LEN  10
> -#define KRB5_OID_LEN  7
> -#define KRB5U2U_OID_LEN  8
> -#define MSKRB5_OID_LEN  7
> -static unsigned long SPNEGO_OID[7] = { 1, 3, 6, 1, 5, 5, 2 };
> -static unsigned long NTLMSSP_OID[10] = { 1, 3, 6, 1, 4, 1, 311, 2, 2, 10 };
> -static unsigned long KRB5_OID[7] = { 1, 2, 840, 113554, 1, 2, 2 };
> -static unsigned long KRB5U2U_OID[8] = { 1, 2, 840, 113554, 1, 2, 2, 3 };
> -static unsigned long MSKRB5_OID[7] = { 1, 2, 840, 48018, 1, 2, 2 };
> -
>  /*
>   * ASN.1 context.
>   */
> @@ -397,95 +387,10 @@ asn1_octets_decode(struct asn1_ctx *ctx,
>  	return 1;
>  } */
>  
> -static unsigned char
> -asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid)
> +static enum OID
> +asn1_oid_decode(struct asn1_ctx *ctx, unsigned char *eoc)
>  {
> -	unsigned char ch;
> -
> -	*subid = 0;
> -
> -	do {
> -		if (!asn1_octet_decode(ctx, &ch))
> -			return 0;
> -
> -		*subid <<= 7;
> -		*subid |= ch & 0x7F;
> -	} while ((ch & 0x80) == 0x80);
> -	return 1;
> -}
> -
> -static int
> -asn1_oid_decode(struct asn1_ctx *ctx,
> -		unsigned char *eoc, unsigned long **oid, unsigned int *len)
> -{
> -	unsigned long subid;
> -	unsigned int size;
> -	unsigned long *optr;
> -
> -	size = eoc - ctx->pointer + 1;
> -
> -	/* first subid actually encodes first two subids */
> -	if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
> -		return 0;
> -
> -	*oid = kmalloc(size * sizeof(unsigned long), GFP_ATOMIC);
> -	if (*oid == NULL)
> -		return 0;
> -
> -	optr = *oid;
> -
> -	if (!asn1_subid_decode(ctx, &subid)) {
> -		kfree(*oid);
> -		*oid = NULL;
> -		return 0;
> -	}
> -
> -	if (subid < 40) {
> -		optr[0] = 0;
> -		optr[1] = subid;
> -	} else if (subid < 80) {
> -		optr[0] = 1;
> -		optr[1] = subid - 40;
> -	} else {
> -		optr[0] = 2;
> -		optr[1] = subid - 80;
> -	}
> -
> -	*len = 2;
> -	optr += 2;
> -
> -	while (ctx->pointer < eoc) {
> -		if (++(*len) > size) {
> -			ctx->error = ASN1_ERR_DEC_BADVALUE;
> -			kfree(*oid);
> -			*oid = NULL;
> -			return 0;
> -		}
> -
> -		if (!asn1_subid_decode(ctx, optr++)) {
> -			kfree(*oid);
> -			*oid = NULL;
> -			return 0;
> -		}
> -	}
> -	return 1;
> -}
> -
> -static int
> -compare_oid(unsigned long *oid1, unsigned int oid1len,
> -	    unsigned long *oid2, unsigned int oid2len)
> -{
> -	unsigned int i;
> -
> -	if (oid1len != oid2len)
> -		return 0;
> -	else {
> -		for (i = 0; i < oid1len; i++) {
> -			if (oid1[i] != oid2[i])
> -				return 0;
> -		}
> -		return 1;
> -	}
> +	return look_up_OID(ctx->pointer, eoc - ctx->pointer);
>  }
>  
>  	/* BB check for endian conversion issues here */
> @@ -497,8 +402,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	struct asn1_ctx ctx;
>  	unsigned char *end;
>  	unsigned char *sequence_end;
> -	unsigned long *oid = NULL;
> -	unsigned int cls, con, tag, oidlen, rc;
> +	unsigned int cls, con, tag, rc;
>  
>  	/* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
>  
> @@ -519,12 +423,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (rc) {
>  		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
>  		    (cls == ASN1_UNI)) {
> -			rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
> -			if (rc) {
> -				rc = compare_oid(oid, oidlen, SPNEGO_OID,
> -						 SPNEGO_OID_LEN);
> -				kfree(oid);
> -			}
> +			if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
> +				rc = 0;
>  		} else
>  			rc = 0;
>  	}
> @@ -588,26 +488,28 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  			return 0;
>  		}
>  		if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
> -			if (asn1_oid_decode(&ctx, end, &oid, &oidlen)) {
> -
> -				cFYI(1, "OID len = %d oid = 0x%lx 0x%lx "
> -					"0x%lx 0x%lx", oidlen, *oid,
> -					*(oid + 1), *(oid + 2), *(oid + 3));
> -
> -				if (compare_oid(oid, oidlen, MSKRB5_OID,
> -						MSKRB5_OID_LEN))
> -					server->sec_mskerberos = true;
> -				else if (compare_oid(oid, oidlen, KRB5U2U_OID,
> -						     KRB5U2U_OID_LEN))
> -					server->sec_kerberosu2u = true;
> -				else if (compare_oid(oid, oidlen, KRB5_OID,
> -						     KRB5_OID_LEN))
> -					server->sec_kerberos = true;
> -				else if (compare_oid(oid, oidlen, NTLMSSP_OID,
> -						     NTLMSSP_OID_LEN))
> -					server->sec_ntlmssp = true;
> -
> -				kfree(oid);
> +			enum OID oid = asn1_oid_decode(&ctx, end);
> +			if (oid != OID__NR)
> +				cFYI(1, "OID oid = %u", oid);
> +			switch (oid) {
> +			case OID_MSKRB5:
> +				server->sec_mskerberos = true;
> +				break;
> +			case OID_KRB5U2U:
> +				server->sec_kerberosu2u = true;
> +				break;
> +			case OID_KRB5:
> +				server->sec_kerberos = true;
> +				break;
> +			case OID_NTLMSSP:
> +				server->sec_ntlmssp = true;
> +				break;
> +			case OID__NR:
> +				cFYI(1, "OID len = %ld oid = %*pX",
> +				     end - ctx.pointer,
> +				     (int)(end - ctx.pointer), ctx.pointer);
> +			default:
> +				break;
>  			}
>  		} else {
>  			cFYI(1, "Should be an oid what is going on?");
> diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
> index 6926db7..4bff9c5 100644
> --- a/include/linux/oid_registry.h
> +++ b/include/linux/oid_registry.h
> @@ -82,6 +82,12 @@ enum OID {
>  	OID_authorityKeyIdentifier,	/* 2.5.29.35 */
>  	OID_extKeyUsage,		/* 2.5.29.37 */
>  
> +	OID_SPNEGO,			/* 1.3.6.1.5.5.2 */
> +	OID_NTLMSSP,			/* 1.3.6.1.4.1.311.2.2.10 */
> +	OID_KRB5,			/* 1.2.840.113554.1.2.2 */
> +	OID_KRB5U2U,			/* 1.2.840.113554.1.2.2.3 */
> +	OID_MSKRB5,			/* 1.2.840.48018.1.2.2 */
> +
>  	OID__NR
>  };
>  
> 

I forgot to mention that this patch looks fine to me after my earlier comments:

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 1/3] CIFS: Use OID registry facility

[David Howells]

Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> How do I incorporate/add OID_REGISTRY as a config option?
> I do not see that in my .config (3.7)

What do you mean?

David

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Shirish Pargaonkar]

On Mon, Oct 22, 2012 at 2:13 PM, David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> How do I incorporate/add OID_REGISTRY as a config option?
>> I do not see that in my .config (3.7)
>
> What do you mean?
>
> David

David, I do not see CONFIG_OID_RRGISTRY in .config.

Regards,

Shirish

Re: [PATCH 1/3] CIFS: Use OID registry facility

[David Howells]

Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> >> How do I incorporate/add OID_REGISTRY as a config option?
> >> I do not see that in my .config (3.7)
> >
> > What do you mean?
> >
> > David
> 
> David, I do not see CONFIG_OID_RRGISTRY in .config.

Do you see OID_REGISTRY in lib/Kconfig (probably near the end)?

David

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Shirish Pargaonkar]

On Sat, Oct 27, 2012 at 8:04 AM, David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> >> How do I incorporate/add OID_REGISTRY as a config option?
>> >> I do not see that in my .config (3.7)
>> >
>> > What do you mean?
>> >
>> > David
>>
>> David, I do not see CONFIG_OID_RRGISTRY in .config.
>
> Do you see OID_REGISTRY in lib/Kconfig (probably near the end)?
>
> David

Yes, I do it

config OID_REGISTRY

Re: [PATCH 1/3] CIFS: Use OID registry facility

[David Howells]

Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> In general, I'm all for moving CIFS out of the business of implementing
> ASN.1 like this. This last bit of this patch was a bit confusing though.
> It left me wondering where the actual definitions of these OIDs go.
> 
> It looks though like you're using a script to scrape the comments out
> of the above enum to generate C files.

That is correct.  See the comment preceding enum OID.  I should add something
to Documentation/ about this too.

> That would certainly work, but it seems like a fragile solution. Minor
> whitespace munging (like an extra newline in there) could throw off the
> parsing...

Extra whitespace and extra newlines shouldn't hurt, unless you mean splitting
a comment over multiple lines.

I could generate the enum too, though it's easy enough for anyone to check
that the OID they've just added is present in the table.

Actually, it can be made a lot more robust by assuming that every line with
"OID_" in it is an OID declaration, and everyone of those lines that doesn't
match the expected pattern gets an error.

David

Re: [PATCH 1/3] CIFS: Use OID registry facility

[Jeff Layton]

On Mon, 22 Oct 2012 20:19:25 +0100
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > In general, I'm all for moving CIFS out of the business of implementing
> > ASN.1 like this. This last bit of this patch was a bit confusing though.
> > It left me wondering where the actual definitions of these OIDs go.
> > 
> > It looks though like you're using a script to scrape the comments out
> > of the above enum to generate C files.
> 
> That is correct.  See the comment preceding enum OID.  I should add something
> to Documentation/ about this too.
> 
> > That would certainly work, but it seems like a fragile solution. Minor
> > whitespace munging (like an extra newline in there) could throw off the
> > parsing...
> 
> Extra whitespace and extra newlines shouldn't hurt, unless you mean splitting
> a comment over multiple lines.
> 
> I could generate the enum too, though it's easy enough for anyone to check
> that the OID they've just added is present in the table.
> 
> Actually, it can be made a lot more robust by assuming that every line with
> "OID_" in it is an OID declaration, and everyone of those lines that doesn't
> match the expected pattern gets an error.
> 
> David

I guess it just looked a little weird to me. The comment that matches a
enum element is actually part of the following element since it comes
after the comma. Based on the script, what matters is that the enum and
the comment are on the same line.

But...as long as it's all clearly documented, I guess I can't complain
too much. ;)

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

[PATCH 2/3] ASN.1: Define indefinite length marker constant

[David Howells]

Define a constant to hold the marker value seen in an indefinite-length
element.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 include/linux/asn1.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/asn1.h b/include/linux/asn1.h
index 5c3f4e4..eed6982 100644
--- a/include/linux/asn1.h
+++ b/include/linux/asn1.h
@@ -64,4 +64,6 @@ enum asn1_tag {
 	ASN1_LONG_TAG	= 31	/* Long form tag */
 };
 
+#define ASN1_INDEFINITE_LENGTH 0x80
+
 #endif /* _LINUX_ASN1_H */

Re: [PATCH 2/3] ASN.1: Define indefinite length marker constant

[Jeff Layton]

On Mon, 22 Oct 2012 15:23:33 +0100
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Define a constant to hold the marker value seen in an indefinite-length
> element.
> 
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
>  include/linux/asn1.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/asn1.h b/include/linux/asn1.h
> index 5c3f4e4..eed6982 100644
> --- a/include/linux/asn1.h
> +++ b/include/linux/asn1.h
> @@ -64,4 +64,6 @@ enum asn1_tag {
>  	ASN1_LONG_TAG	= 31	/* Long form tag */
>  };
>  
> +#define ASN1_INDEFINITE_LENGTH 0x80
> +
>  #endif /* _LINUX_ASN1_H */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Looks fine, though might make more sense to fold this into the next
patch since you actually make use of it there.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [PATCH 2/3] ASN.1: Define indefinite length marker constant

[David Howells]

Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Looks fine, though might make more sense to fold this into the next
> patch since you actually make use of it there.

The patch is also required for some other unrelated stuff.

David

[PATCH 3/3] CIFS: Use linux/asn1.h

[David Howells]

Use linux/asn1.h in CIFS's ASN.1 decoder to get common ASN.1 constants rather
than redefining them for itself.

Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---

 fs/cifs/asn1.c |   76 ++++++++++++++++++--------------------------------------
 1 file changed, 24 insertions(+), 52 deletions(-)

diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
index 3a7b2b6..2e46b81 100644
--- a/fs/cifs/asn1.c
+++ b/fs/cifs/asn1.c
@@ -22,6 +22,7 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
+#include <linux/asn1.h>
 #include <linux/oid_registry.h>
 #include "cifspdu.h"
 #include "cifsglob.h"
@@ -34,40 +35,6 @@
  *
  *****************************************************************************/
 
-/* Class */
-#define ASN1_UNI	0	/* Universal */
-#define ASN1_APL	1	/* Application */
-#define ASN1_CTX	2	/* Context */
-#define ASN1_PRV	3	/* Private */
-
-/* Tag */
-#define ASN1_EOC	0	/* End Of Contents or N/A */
-#define ASN1_BOL	1	/* Boolean */
-#define ASN1_INT	2	/* Integer */
-#define ASN1_BTS	3	/* Bit String */
-#define ASN1_OTS	4	/* Octet String */
-#define ASN1_NUL	5	/* Null */
-#define ASN1_OJI	6	/* Object Identifier  */
-#define ASN1_OJD	7	/* Object Description */
-#define ASN1_EXT	8	/* External */
-#define ASN1_ENUM	10	/* Enumerated */
-#define ASN1_SEQ	16	/* Sequence */
-#define ASN1_SET	17	/* Set */
-#define ASN1_NUMSTR	18	/* Numerical String */
-#define ASN1_PRNSTR	19	/* Printable String */
-#define ASN1_TEXSTR	20	/* Teletext String */
-#define ASN1_VIDSTR	21	/* Video String */
-#define ASN1_IA5STR	22	/* IA5 String */
-#define ASN1_UNITIM	23	/* Universal Time */
-#define ASN1_GENTIM	24	/* General Time */
-#define ASN1_GRASTR	25	/* Graphical String */
-#define ASN1_VISSTR	26	/* Visible String */
-#define ASN1_GENSTR	27	/* General String */
-
-/* Primitive / Constructed methods*/
-#define ASN1_PRI	0	/* Primitive */
-#define ASN1_CON	1	/* Constructed */
-
 /*
  * Error codes.
  */
@@ -155,7 +122,8 @@ asn1_tag_decode(struct asn1_ctx *ctx, unsigned int *tag)
 
 static unsigned char
 asn1_id_decode(struct asn1_ctx *ctx,
-	       unsigned int *cls, unsigned int *con, unsigned int *tag)
+	       enum asn1_class *cls, enum asn1_method *con,
+	       enum asn1_tag *tag)
 {
 	unsigned char ch;
 
@@ -166,7 +134,7 @@ asn1_id_decode(struct asn1_ctx *ctx,
 	*con = (ch & 0x20) >> 5;
 	*tag = (ch & 0x1F);
 
-	if (*tag == 0x1F) {
+	if (*tag == ASN1_LONG_TAG) {
 		if (!asn1_tag_decode(ctx, tag))
 			return 0;
 	}
@@ -181,7 +149,7 @@ asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned int *len)
 	if (!asn1_octet_decode(ctx, &ch))
 		return 0;
 
-	if (ch == 0x80)
+	if (ch == ASN1_INDEFINITE_LENGTH)
 		*def = 0;
 	else {
 		*def = 1;
@@ -212,7 +180,8 @@ asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned int *len)
 static unsigned char
 asn1_header_decode(struct asn1_ctx *ctx,
 		   unsigned char **eoc,
-		   unsigned int *cls, unsigned int *con, unsigned int *tag)
+		   enum asn1_class *cls, enum asn1_method *con,
+		   enum asn1_tag *tag)
 {
 	unsigned int def = 0;
 	unsigned int len = 0;
@@ -224,7 +193,7 @@ asn1_header_decode(struct asn1_ctx *ctx,
 		return 0;
 
 	/* primitive shall be definite, indefinite shall be constructed */
-	if (*con == ASN1_PRI && !def)
+	if (*con == ASN1_PRIM && !def)
 		return 0;
 
 	if (def)
@@ -402,7 +371,10 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	struct asn1_ctx ctx;
 	unsigned char *end;
 	unsigned char *sequence_end;
-	unsigned int cls, con, tag, rc;
+	unsigned int rc;
+	enum asn1_class cls;
+	enum asn1_method con;
+	enum asn1_tag tag;
 
 	/* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
 
@@ -412,7 +384,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding negTokenInit header");
 		return 0;
-	} else if ((cls != ASN1_APL) || (con != ASN1_CON)
+	} else if ((cls != ASN1_APPL) || (con != ASN1_CONS)
 		   || (tag != ASN1_EOC)) {
 		cFYI(1, "cls = %d con = %d tag = %d", cls, con, tag);
 		return 0;
@@ -421,8 +393,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	/* Check for SPNEGO OID -- remember to free obj->oid */
 	rc = asn1_header_decode(&ctx, &end, &cls, &con, &tag);
 	if (rc) {
-		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
-		    (cls == ASN1_UNI)) {
+		if ((tag == ASN1_OID) && (con == ASN1_PRIM) &&
+		    (cls == ASN1_UNIV)) {
 			if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
 				rc = 0;
 		} else
@@ -439,7 +411,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding negTokenInit");
 		return 0;
-	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
+	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)
 		   || (tag != ASN1_EOC)) {
 		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 0",
 		     cls, con, tag, end, *end);
@@ -450,7 +422,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding negTokenInit");
 		return 0;
-	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
+	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
 		   || (tag != ASN1_SEQ)) {
 		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 1",
 		     cls, con, tag, end, *end);
@@ -461,7 +433,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding 2nd part of negTokenInit");
 		return 0;
-	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
+	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)
 		   || (tag != ASN1_EOC)) {
 		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 0",
 		     cls, con, tag, end, *end);
@@ -473,7 +445,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	    (&ctx, &sequence_end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding 2nd part of negTokenInit");
 		return 0;
-	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
+	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
 		   || (tag != ASN1_SEQ)) {
 		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 1",
 		     cls, con, tag, end, *end);
@@ -487,7 +459,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 			cFYI(1, "Error decoding negTokenInit hdr exit2");
 			return 0;
 		}
-		if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
+		if ((tag == ASN1_OID) && (con == ASN1_PRIM)) {
 			enum OID oid = asn1_oid_decode(&ctx, end);
 			if (oid != OID__NR)
 				cFYI(1, "OID oid = %u", oid);
@@ -524,7 +496,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 			goto decode_negtoken_exit;
 		cFYI(1, "Error decoding last part negTokenInit exit3");
 		return 0;
-	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)) {
+	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)) {
 		/* tag = 3 indicating mechListMIC */
 		cFYI(1, "Exit 4 cls = %d con = %d tag = %d end = %p (%d)",
 			cls, con, tag, end, *end);
@@ -535,7 +507,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding last part negTokenInit exit5");
 		return 0;
-	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
+	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
 		   || (tag != ASN1_SEQ)) {
 		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d)",
 			cls, con, tag, end, *end);
@@ -545,7 +517,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding last part negTokenInit exit 7");
 		return 0;
-	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)) {
+	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)) {
 		cFYI(1, "Exit 8 cls = %d con = %d tag = %d end = %p (%d)",
 			cls, con, tag, end, *end);
 		return 0;
@@ -555,7 +527,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
 		cFYI(1, "Error decoding last part negTokenInit exit9");
 		return 0;
-	} else if ((cls != ASN1_UNI) || (con != ASN1_PRI)
+	} else if ((cls != ASN1_UNIV) || (con != ASN1_PRIM)
 		   || (tag != ASN1_GENSTR)) {
 		cFYI(1, "Exit10 cls = %d con = %d tag = %d end = %p (%d)",
 			cls, con, tag, end, *end);

Re: [PATCH 3/3] CIFS: Use linux/asn1.h

[Jeff Layton]

On Mon, 22 Oct 2012 15:23:41 +0100
David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:

> Use linux/asn1.h in CIFS's ASN.1 decoder to get common ASN.1 constants rather
> than redefining them for itself.
> 
> Signed-off-by: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> 
>  fs/cifs/asn1.c |   76 ++++++++++++++++++--------------------------------------
>  1 file changed, 24 insertions(+), 52 deletions(-)
> 
> diff --git a/fs/cifs/asn1.c b/fs/cifs/asn1.c
> index 3a7b2b6..2e46b81 100644
> --- a/fs/cifs/asn1.c
> +++ b/fs/cifs/asn1.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/slab.h>
> +#include <linux/asn1.h>
>  #include <linux/oid_registry.h>
>  #include "cifspdu.h"
>  #include "cifsglob.h"
> @@ -34,40 +35,6 @@
>   *
>   *****************************************************************************/
>  
> -/* Class */
> -#define ASN1_UNI	0	/* Universal */
> -#define ASN1_APL	1	/* Application */
> -#define ASN1_CTX	2	/* Context */
> -#define ASN1_PRV	3	/* Private */
> -
> -/* Tag */
> -#define ASN1_EOC	0	/* End Of Contents or N/A */
> -#define ASN1_BOL	1	/* Boolean */
> -#define ASN1_INT	2	/* Integer */
> -#define ASN1_BTS	3	/* Bit String */
> -#define ASN1_OTS	4	/* Octet String */
> -#define ASN1_NUL	5	/* Null */
> -#define ASN1_OJI	6	/* Object Identifier  */
> -#define ASN1_OJD	7	/* Object Description */
> -#define ASN1_EXT	8	/* External */
> -#define ASN1_ENUM	10	/* Enumerated */
> -#define ASN1_SEQ	16	/* Sequence */
> -#define ASN1_SET	17	/* Set */
> -#define ASN1_NUMSTR	18	/* Numerical String */
> -#define ASN1_PRNSTR	19	/* Printable String */
> -#define ASN1_TEXSTR	20	/* Teletext String */
> -#define ASN1_VIDSTR	21	/* Video String */
> -#define ASN1_IA5STR	22	/* IA5 String */
> -#define ASN1_UNITIM	23	/* Universal Time */
> -#define ASN1_GENTIM	24	/* General Time */
> -#define ASN1_GRASTR	25	/* Graphical String */
> -#define ASN1_VISSTR	26	/* Visible String */
> -#define ASN1_GENSTR	27	/* General String */
> -
> -/* Primitive / Constructed methods*/
> -#define ASN1_PRI	0	/* Primitive */
> -#define ASN1_CON	1	/* Constructed */
> -
>  /*
>   * Error codes.
>   */
> @@ -155,7 +122,8 @@ asn1_tag_decode(struct asn1_ctx *ctx, unsigned int *tag)
>  
>  static unsigned char
>  asn1_id_decode(struct asn1_ctx *ctx,
> -	       unsigned int *cls, unsigned int *con, unsigned int *tag)
> +	       enum asn1_class *cls, enum asn1_method *con,
> +	       enum asn1_tag *tag)
>  {
>  	unsigned char ch;
>  
> @@ -166,7 +134,7 @@ asn1_id_decode(struct asn1_ctx *ctx,
>  	*con = (ch & 0x20) >> 5;
>  	*tag = (ch & 0x1F);
>  
> -	if (*tag == 0x1F) {
> +	if (*tag == ASN1_LONG_TAG) {
>  		if (!asn1_tag_decode(ctx, tag))
>  			return 0;
>  	}
> @@ -181,7 +149,7 @@ asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned int *len)
>  	if (!asn1_octet_decode(ctx, &ch))
>  		return 0;
>  
> -	if (ch == 0x80)
> +	if (ch == ASN1_INDEFINITE_LENGTH)
>  		*def = 0;
>  	else {
>  		*def = 1;
> @@ -212,7 +180,8 @@ asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned int *len)
>  static unsigned char
>  asn1_header_decode(struct asn1_ctx *ctx,
>  		   unsigned char **eoc,
> -		   unsigned int *cls, unsigned int *con, unsigned int *tag)
> +		   enum asn1_class *cls, enum asn1_method *con,
> +		   enum asn1_tag *tag)
>  {
>  	unsigned int def = 0;
>  	unsigned int len = 0;
> @@ -224,7 +193,7 @@ asn1_header_decode(struct asn1_ctx *ctx,
>  		return 0;
>  
>  	/* primitive shall be definite, indefinite shall be constructed */
> -	if (*con == ASN1_PRI && !def)
> +	if (*con == ASN1_PRIM && !def)
>  		return 0;
>  
>  	if (def)
> @@ -402,7 +371,10 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	struct asn1_ctx ctx;
>  	unsigned char *end;
>  	unsigned char *sequence_end;
> -	unsigned int cls, con, tag, rc;
> +	unsigned int rc;
> +	enum asn1_class cls;
> +	enum asn1_method con;
> +	enum asn1_tag tag;
>  
>  	/* cifs_dump_mem(" Received SecBlob ", security_blob, length); */
>  
> @@ -412,7 +384,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding negTokenInit header");
>  		return 0;
> -	} else if ((cls != ASN1_APL) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_APPL) || (con != ASN1_CONS)
>  		   || (tag != ASN1_EOC)) {
>  		cFYI(1, "cls = %d con = %d tag = %d", cls, con, tag);
>  		return 0;
> @@ -421,8 +393,8 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	/* Check for SPNEGO OID -- remember to free obj->oid */
>  	rc = asn1_header_decode(&ctx, &end, &cls, &con, &tag);
>  	if (rc) {
> -		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
> -		    (cls == ASN1_UNI)) {
> +		if ((tag == ASN1_OID) && (con == ASN1_PRIM) &&
> +		    (cls == ASN1_UNIV)) {
>  			if (asn1_oid_decode(&ctx, end) != OID_SPNEGO)
>  				rc = 0;
>  		} else
> @@ -439,7 +411,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding negTokenInit");
>  		return 0;
> -	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)
>  		   || (tag != ASN1_EOC)) {
>  		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 0",
>  		     cls, con, tag, end, *end);
> @@ -450,7 +422,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding negTokenInit");
>  		return 0;
> -	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
>  		   || (tag != ASN1_SEQ)) {
>  		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 1",
>  		     cls, con, tag, end, *end);
> @@ -461,7 +433,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding 2nd part of negTokenInit");
>  		return 0;
> -	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)
>  		   || (tag != ASN1_EOC)) {
>  		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 0",
>  		     cls, con, tag, end, *end);
> @@ -473,7 +445,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	    (&ctx, &sequence_end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding 2nd part of negTokenInit");
>  		return 0;
> -	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
>  		   || (tag != ASN1_SEQ)) {
>  		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d) exit 1",
>  		     cls, con, tag, end, *end);
> @@ -487,7 +459,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  			cFYI(1, "Error decoding negTokenInit hdr exit2");
>  			return 0;
>  		}
> -		if ((tag == ASN1_OJI) && (con == ASN1_PRI)) {
> +		if ((tag == ASN1_OID) && (con == ASN1_PRIM)) {
>  			enum OID oid = asn1_oid_decode(&ctx, end);
>  			if (oid != OID__NR)
>  				cFYI(1, "OID oid = %u", oid);
> @@ -524,7 +496,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  			goto decode_negtoken_exit;
>  		cFYI(1, "Error decoding last part negTokenInit exit3");
>  		return 0;
> -	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)) {
> +	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)) {
>  		/* tag = 3 indicating mechListMIC */
>  		cFYI(1, "Exit 4 cls = %d con = %d tag = %d end = %p (%d)",
>  			cls, con, tag, end, *end);
> @@ -535,7 +507,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding last part negTokenInit exit5");
>  		return 0;
> -	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
> +	} else if ((cls != ASN1_UNIV) || (con != ASN1_CONS)
>  		   || (tag != ASN1_SEQ)) {
>  		cFYI(1, "cls = %d con = %d tag = %d end = %p (%d)",
>  			cls, con, tag, end, *end);
> @@ -545,7 +517,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding last part negTokenInit exit 7");
>  		return 0;
> -	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)) {
> +	} else if ((cls != ASN1_CONT) || (con != ASN1_CONS)) {
>  		cFYI(1, "Exit 8 cls = %d con = %d tag = %d end = %p (%d)",
>  			cls, con, tag, end, *end);
>  		return 0;
> @@ -555,7 +527,7 @@ decode_negTokenInit(unsigned char *security_blob, int length,
>  	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
>  		cFYI(1, "Error decoding last part negTokenInit exit9");
>  		return 0;
> -	} else if ((cls != ASN1_UNI) || (con != ASN1_PRI)
> +	} else if ((cls != ASN1_UNIV) || (con != ASN1_PRIM)
>  		   || (tag != ASN1_GENSTR)) {
>  		cFYI(1, "Exit10 cls = %d con = %d tag = %d end = %p (%d)",
>  			cls, con, tag, end, *end);
> 

Nice cleanup.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>