Add BerBitString and improve "unused bit" handling

14 files changed, 244 insertions, 144 deletions

diff --git a/crypto/src/asn1/BERBitString.cs b/crypto/src/asn1/BERBitString.cs
new file mode 100644
index 000000000..d8cd00330
--- /dev/null
+++ b/crypto/src/asn1/BERBitString.cs
@@ -0,0 +1,43 @@
+using System;
+
+using Org.BouncyCastle.Utilities;
+
+namespace Org.BouncyCastle.Asn1
+{
+    public class BerBitString
+        : DerBitString
+    {
+        public BerBitString(byte[] data, int padBits)
+            : base(data, padBits)
+		{
+		}
+
+		public BerBitString(byte[] data)
+            : base(data)
+		{
+		}
+
+        public BerBitString(int namedBits)
+            : base(namedBits)
+        {
+        }
+
+        public BerBitString(Asn1Encodable obj)
+            : base(obj)
+		{
+		}
+
+        internal override void Encode(
+            DerOutputStream derOut)
+        {
+            if (derOut is Asn1OutputStream || derOut is BerOutputStream)
+            {
+                derOut.WriteEncoded(Asn1Tags.BitString, (byte)mPadBits, mData);
+            }
+            else
+            {
+                base.Encode(derOut);
+            }
+        }
+    }
+}
diff --git a/crypto/src/asn1/DerBitString.cs b/crypto/src/asn1/DerBitString.cs
index d5cb872bc..ad7a7e349 100644
--- a/crypto/src/asn1/DerBitString.cs
+++ b/crypto/src/asn1/DerBitString.cs
@@ -1,6 +1,8 @@
 using System;
+using System.Diagnostics;
 using System.Text;
 
+using Org.BouncyCastle.Math;
 using Org.BouncyCastle.Utilities;
 
 namespace Org.BouncyCastle.Asn1
@@ -11,83 +13,10 @@ namespace Org.BouncyCastle.Asn1
 		private static readonly char[] table
 			= { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };
 
-		private readonly byte[]	data;
-		private readonly int	padBits;
+		protected readonly byte[]   mData;
+		protected readonly int      mPadBits;
 
-		/**
-		 * return the correct number of pad bits for a bit string defined in
-		 * a 32 bit constant
-		 */
-		static internal int GetPadBits(
-			int bitString)
-		{
-			int val = 0;
-			for (int i = 3; i >= 0; i--)
-			{
-				//
-				// this may look a little odd, but if it isn't done like this pre jdk1.2
-				// JVM's break!
-				//
-				if (i != 0)
-				{
-					if ((bitString >> (i * 8)) != 0)
-					{
-						val = (bitString >> (i * 8)) & 0xFF;
-						break;
-					}
-				}
-				else
-				{
-					if (bitString != 0)
-					{
-						val = bitString & 0xFF;
-						break;
-					}
-				}
-			}
-
-			if (val == 0)
-			{
-				return 7;
-			}
-
-			int bits = 1;
-
-			while (((val <<= 1) & 0xFF) != 0)
-			{
-				bits++;
-			}
-
-			return 8 - bits;
-		}
-
-		/**
-		 * return the correct number of bytes for a bit string defined in
-		 * a 32 bit constant
-		 */
-		static internal byte[] GetBytes(
-			int bitString)
-		{
-			int bytes = 4;
-			for (int i = 3; i >= 1; i--)
-			{
-				if ((bitString & (0xFF << (i * 8))) != 0)
-				{
-					break;
-				}
-				bytes--;
-			}
-
-			byte[] result = new byte[bytes];
-			for (int i = 0; i < bytes; i++)
-			{
-				result[i] = (byte) ((bitString >> (i * 8)) & 0xFF);
-			}
-
-			return result;
-		}
-
-		/**
+        /**
 		 * return a Bit string from the passed in object
 		 *
 		 * @exception ArgumentException if the object cannot be converted.
@@ -126,15 +55,7 @@ namespace Org.BouncyCastle.Asn1
 			return FromAsn1Octets(((Asn1OctetString)o).GetOctets());
 		}
 
-		internal DerBitString(
-			byte	data,
-			int		padBits)
-		{
-			this.data = new byte[]{ data };
-			this.padBits = padBits;
-		}
-
-		/**
+        /**
 		 * @param data the octets making up the bit string.
 		 * @param padBits the number of extra bits at the end of the string.
 		 */
@@ -142,67 +63,154 @@ namespace Org.BouncyCastle.Asn1
 			byte[]	data,
 			int		padBits)
 		{
-			// TODO Deep copy?
-			this.data = data;
-			this.padBits = padBits;
+            if (data == null)
+                throw new ArgumentNullException("data");
+            if (padBits < 0 || padBits > 7)
+                throw new ArgumentException("must be in the range 0 to 7", "padBits");
+            if (data.Length == 0 && padBits != 0)
+                throw new ArgumentException("if 'data' is empty, 'padBits' must be 0");
+
+            this.mData = Arrays.Clone(data);
+			this.mPadBits = padBits;
 		}
 
 		public DerBitString(
 			byte[] data)
+            : this(data, 0)
 		{
-			// TODO Deep copy?
-			this.data = data;
 		}
 
-		public DerBitString(
+        public DerBitString(
+            int namedBits)
+        {
+            if (namedBits == 0)
+            {
+                this.mData = new byte[0];
+                this.mPadBits = 0;
+                return;
+            }
+
+            int bits = BigInteger.BitLen(namedBits);
+            int bytes = (bits + 7) / 8;
+
+            Debug.Assert(0 < bytes && bytes <= 4);
+
+            byte[] result = new byte[bytes];
+            --bytes;
+
+            for (int i = 0; i < bytes; i++)
+            {
+                result[i] = (byte)namedBits;
+                namedBits >>= 8;
+            }
+
+            Debug.Assert((namedBits & 0xFF) != 0);
+
+            result[bytes] = (byte)namedBits;
+
+            int pad = 0;
+            while ((namedBits & (1 << pad)) == 0)
+            {
+                ++pad;
+            }
+
+            Debug.Assert(pad < 8);
+
+            this.mData = result;
+            this.mPadBits = pad;
+        }
+
+        public DerBitString(
 			Asn1Encodable obj)
+            : this(obj.GetDerEncoded())
 		{
-			this.data = obj.GetDerEncoded();
-			//this.padBits = 0;
 		}
 
-		public byte[] GetBytes()
+        /**
+         * Return the octets contained in this BIT STRING, checking that this BIT STRING really
+         * does represent an octet aligned string. Only use this method when the standard you are
+         * following dictates that the BIT STRING will be octet aligned.
+         *
+         * @return a copy of the octet aligned data.
+         */
+        public virtual byte[] GetOctets()
+        {
+            if (mPadBits != 0)
+                throw new InvalidOperationException("attempt to get non-octet aligned data from BIT STRING");
+
+            return Arrays.Clone(mData);
+        }
+
+        public virtual byte[] GetBytes()
 		{
-			return data;
+            byte[] data = Arrays.Clone(mData);
+
+            // DER requires pad bits be zero
+            if (mPadBits > 0)
+            {
+                data[data.Length - 1] &= (byte)(0xFF << mPadBits);
+            }
+
+            return data;
 		}
 
-		public int PadBits
+        public virtual int PadBits
 		{
-			get { return padBits; }
+			get { return mPadBits; }
 		}
 
 		/**
 		 * @return the value of the bit string as an int (truncating if necessary)
 		 */
-		public int IntValue
+        public virtual int IntValue
 		{
 			get
 			{
-				int value = 0;
-
-				for (int i = 0; i != data.Length && i != 4; i++)
-				{
-					value |= (data[i] & 0xff) << (8 * i);
-				}
-
-				return value;
+                int value = 0, length = System.Math.Min(4, mData.Length);
+                for (int i = 0; i < length; ++i)
+                {
+                    value |= (int)mData[i] << (8 * i);
+                }
+                if (mPadBits > 0 && length == mData.Length)
+                {
+                    int mask = (1 << mPadBits) - 1;
+                    value &= ~(mask << (8 * (length - 1)));
+                }
+                return value;
 			}
 		}
 
-		internal override void Encode(
+        internal override void Encode(
 			DerOutputStream derOut)
 		{
-			byte[] bytes = new byte[GetBytes().Length + 1];
-
-			bytes[0] = (byte) PadBits;
-			Array.Copy(GetBytes(), 0, bytes, 1, bytes.Length - 1);
-
-			derOut.WriteEncoded(Asn1Tags.BitString, bytes);
+            if (mPadBits > 0)
+            {
+                int last = mData[mData.Length - 1];
+                int mask = (1 << mPadBits) - 1;
+
+                if ((last & mask) != 0)
+                {
+                    byte[] result = Arrays.Prepend(mData, (byte)mPadBits);
+
+                    /*
+                    * X.690-0207 11.2.1: Each unused bit in the final octet of the encoding of a bit string value shall be set to zero.
+                    * 
+                    * NOTE: 'pad' is constrained to be 0 if 'bytes' are empty, in which case this is a no-op. 
+                    */
+                    last ^= (last & mask);
+                    result[result.Length - 1] &= (byte)last;
+
+                    derOut.WriteEncoded(Asn1Tags.BitString, result);
+                    return;
+                }
+            }
+
+            derOut.WriteEncoded(Asn1Tags.BitString, (byte)mPadBits, mData);
 		}
 
-		protected override int Asn1GetHashCode()
+        protected override int Asn1GetHashCode()
 		{
-			return padBits.GetHashCode() ^ Arrays.GetHashCode(data);
+			return mPadBits.GetHashCode() ^ Arrays.GetHashCode(mData);
 		}
 
 		protected override bool Asn1Equals(
@@ -213,8 +221,8 @@ namespace Org.BouncyCastle.Asn1
 			if (other == null)
 				return false;
 
-			return this.padBits == other.padBits
-				&& Arrays.AreEqual(this.data, other.data);
+			return this.mPadBits == other.mPadBits
+				&& Arrays.AreEqual(this.mData, other.mData);
 		}
 
 		public override string GetString()
@@ -236,12 +244,23 @@ namespace Org.BouncyCastle.Asn1
 		internal static DerBitString FromAsn1Octets(byte[] octets)
 		{
 	        if (octets.Length < 1)
-	            throw new ArgumentException("truncated BIT STRING detected");
+	            throw new ArgumentException("truncated BIT STRING detected", "octets");
+
+            int padBits = octets[0];
+            byte[] data = Arrays.CopyOfRange(octets, 1, octets.Length);
+
+            if (padBits > 0 && padBits < 8 && data.Length > 0)
+            {
+                int last = data[data.Length - 1];
+                int mask = (1 << padBits) - 1;
+
+                if ((last & mask) != 0)
+                {
+                    return new BerBitString(data, padBits);
+                }
+            }
 
-			int padBits = octets[0];
-			byte[] data = new byte[octets.Length - 1];
-			Array.Copy(octets, 1, data, 0, data.Length);
-			return new DerBitString(data, padBits);
+            return new DerBitString(data, padBits);
 		}
 	}
 }
diff --git a/crypto/src/asn1/DerOutputStream.cs b/crypto/src/asn1/DerOutputStream.cs
index c03d9dc11..69d5d5f28 100644
--- a/crypto/src/asn1/DerOutputStream.cs
+++ b/crypto/src/asn1/DerOutputStream.cs
@@ -19,7 +19,7 @@ namespace Org.BouncyCastle.Asn1
 			if (length > 127)
 			{
 				int size = 1;
-				uint val = (uint) length;
+				uint val = (uint)length;
 
 				while ((val >>= 8) != 0)
 				{
@@ -43,18 +43,29 @@ namespace Org.BouncyCastle.Asn1
 			int		tag,
 			byte[]	bytes)
 		{
-			WriteByte((byte) tag);
+			WriteByte((byte)tag);
 			WriteLength(bytes.Length);
 			Write(bytes, 0, bytes.Length);
 		}
 
-		internal void WriteEncoded(
+        internal void WriteEncoded(
+            int     tag,
+            byte    first,
+            byte[]  bytes)
+        {
+            WriteByte((byte)tag);
+            WriteLength(bytes.Length + 1);
+            WriteByte(first);
+            Write(bytes, 0, bytes.Length);
+        }
+
+        internal void WriteEncoded(
 			int		tag,
 			byte[]	bytes,
 			int		offset,
 			int		length)
 		{
-			WriteByte((byte) tag);
+			WriteByte((byte)tag);
 			WriteLength(length);
 			Write(bytes, offset, length);
 		}
diff --git a/crypto/src/asn1/cmp/PKIFailureInfo.cs b/crypto/src/asn1/cmp/PKIFailureInfo.cs
index 896bf0992..75a3ff0d7 100644
--- a/crypto/src/asn1/cmp/PKIFailureInfo.cs
+++ b/crypto/src/asn1/cmp/PKIFailureInfo.cs
@@ -77,15 +77,14 @@ namespace Org.BouncyCastle.Asn1.Cmp
         /**
 		 * Basic constructor.
 		 */
-		public PkiFailureInfo(
-			int info)
-			:	base(GetBytes(info), GetPadBits(info))
+		public PkiFailureInfo(int info)
+			: base(info)
 		{
 		}
 
 		public PkiFailureInfo(
 			DerBitString info)
-			:	base(info.GetBytes(), info.PadBits)
+			: base(info.GetBytes(), info.PadBits)
 		{
 		}
 
diff --git a/crypto/src/asn1/misc/NetscapeCertType.cs b/crypto/src/asn1/misc/NetscapeCertType.cs
index d5db6523d..d809eae66 100644
--- a/crypto/src/asn1/misc/NetscapeCertType.cs
+++ b/crypto/src/asn1/misc/NetscapeCertType.cs
@@ -36,7 +36,7 @@ namespace Org.BouncyCastle.Asn1.Misc
          * e.g. (X509NetscapeCertType.sslCA | X509NetscapeCertType.smimeCA)
          */
         public NetscapeCertType(int usage)
-			: base(GetBytes(usage), GetPadBits(usage))
+			: base(usage)
         {
         }
 
diff --git a/crypto/src/asn1/ocsp/BasicOCSPResponse.cs b/crypto/src/asn1/ocsp/BasicOCSPResponse.cs
index dd666addf..064335ae8 100644
--- a/crypto/src/asn1/ocsp/BasicOCSPResponse.cs
+++ b/crypto/src/asn1/ocsp/BasicOCSPResponse.cs
@@ -94,7 +94,12 @@ namespace Org.BouncyCastle.Asn1.Ocsp
 			get { return signature; }
 		}
 
-		[Obsolete("Use Certs property instead")]
+        public byte[] GetSignatureOctets()
+        {
+            return signature.GetOctets();
+        }
+
+        [Obsolete("Use Certs property instead")]
 		public Asn1Sequence GetCerts()
         {
             return certs;
diff --git a/crypto/src/asn1/ocsp/Signature.cs b/crypto/src/asn1/ocsp/Signature.cs
index a07e7a709..df6f43332 100644
--- a/crypto/src/asn1/ocsp/Signature.cs
+++ b/crypto/src/asn1/ocsp/Signature.cs
@@ -80,7 +80,12 @@ namespace Org.BouncyCastle.Asn1.Ocsp
 			get { return signatureValue; }
 		}
 
-		public Asn1Sequence Certs
+        public byte[] GetSignatureOctets()
+        {
+            return signatureValue.GetOctets();
+        }
+
+        public Asn1Sequence Certs
 		{
 			get { return certs; }
 		}
diff --git a/crypto/src/asn1/pkcs/CertificationRequest.cs b/crypto/src/asn1/pkcs/CertificationRequest.cs
index 32b1612d2..35bdd56eb 100644
--- a/crypto/src/asn1/pkcs/CertificationRequest.cs
+++ b/crypto/src/asn1/pkcs/CertificationRequest.cs
@@ -73,7 +73,12 @@ namespace Org.BouncyCastle.Asn1.Pkcs
 			get { return sigBits; }
 		}
 
-		public override Asn1Object ToAsn1Object()
+        public byte[] GetSignatureOctets()
+        {
+            return sigBits.GetOctets();
+        }
+
+        public override Asn1Object ToAsn1Object()
         {
 			return new DerSequence(reqInfo, sigAlgId, sigBits);
         }
diff --git a/crypto/src/asn1/x509/AttributeCertificate.cs b/crypto/src/asn1/x509/AttributeCertificate.cs
index 5f85910da..41893b6b4 100644
--- a/crypto/src/asn1/x509/AttributeCertificate.cs
+++ b/crypto/src/asn1/x509/AttributeCertificate.cs
@@ -63,7 +63,12 @@ namespace Org.BouncyCastle.Asn1.X509
 			get { return signatureValue; }
 		}
 
-		/**
+        public byte[] GetSignatureOctets()
+        {
+            return signatureValue.GetOctets();
+        }
+
+        /**
          * Produce an object suitable for an Asn1OutputStream.
          * <pre>
          *  AttributeCertificate ::= Sequence {
diff --git a/crypto/src/asn1/x509/CertificateList.cs b/crypto/src/asn1/x509/CertificateList.cs
index 0412e0816..567cf132a 100644
--- a/crypto/src/asn1/x509/CertificateList.cs
+++ b/crypto/src/asn1/x509/CertificateList.cs
@@ -80,7 +80,12 @@ namespace Org.BouncyCastle.Asn1.X509
 			get { return sig; }
 		}
 
-		public int Version
+        public byte[] GetSignatureOctets()
+        {
+            return sig.GetOctets();
+        }
+
+        public int Version
 		{
 			get { return tbsCertList.Version; }
 		}
diff --git a/crypto/src/asn1/x509/KeyUsage.cs b/crypto/src/asn1/x509/KeyUsage.cs
index fef04e8b9..aeaffb708 100644
--- a/crypto/src/asn1/x509/KeyUsage.cs
+++ b/crypto/src/asn1/x509/KeyUsage.cs
@@ -53,9 +53,8 @@ namespace Org.BouncyCastle.Asn1.X509
          * allowed uses for the key.
          * e.g. (KeyUsage.keyEncipherment | KeyUsage.dataEncipherment)
          */
-        public KeyUsage(
-			int usage)
-			: base(GetBytes(usage), GetPadBits(usage))
+        public KeyUsage(int usage)
+			: base(usage)
         {
         }
 
diff --git a/crypto/src/asn1/x509/ReasonFlags.cs b/crypto/src/asn1/x509/ReasonFlags.cs
index f204c36aa..ad45e84ae 100644
--- a/crypto/src/asn1/x509/ReasonFlags.cs
+++ b/crypto/src/asn1/x509/ReasonFlags.cs
@@ -31,13 +31,12 @@ namespace Org.BouncyCastle.Asn1.X509
          * @param reasons - the bitwise OR of the Key Reason flags giving the
          * allowed uses for the key.
          */
-        public ReasonFlags(
-            int reasons)
-             : base(GetBytes(reasons), GetPadBits(reasons))
+        public ReasonFlags(int reasons)
+             : base(reasons)
         {
         }
 
-		public ReasonFlags(
+        public ReasonFlags(
             DerBitString reasons)
              : base(reasons.GetBytes(), reasons.PadBits)
         {
diff --git a/crypto/src/asn1/x509/SubjectPublicKeyInfo.cs b/crypto/src/asn1/x509/SubjectPublicKeyInfo.cs
index 8ce4b2762..477329b7e 100644
--- a/crypto/src/asn1/x509/SubjectPublicKeyInfo.cs
+++ b/crypto/src/asn1/x509/SubjectPublicKeyInfo.cs
@@ -75,7 +75,7 @@ namespace Org.BouncyCastle.Asn1.X509
          */
         public Asn1Object GetPublicKey()
         {
-			return Asn1Object.FromByteArray(keyData.GetBytes());
+			return Asn1Object.FromByteArray(keyData.GetOctets());
         }
 
 		/**
diff --git a/crypto/src/asn1/x509/X509CertificateStructure.cs b/crypto/src/asn1/x509/X509CertificateStructure.cs
index c8558ae61..6e7c85de6 100644
--- a/crypto/src/asn1/x509/X509CertificateStructure.cs
+++ b/crypto/src/asn1/x509/X509CertificateStructure.cs
@@ -119,6 +119,11 @@ namespace Org.BouncyCastle.Asn1.X509
             get { return sig; }
         }
 
+        public byte[] GetSignatureOctets()
+        {
+            return sig.GetOctets();
+        }
+
         public override Asn1Object ToAsn1Object()
         {
             return new DerSequence(tbsCert, sigAlgID, sig);