Add BerBitString and improve "unused bit" handling

26 files changed, 373 insertions, 178 deletions

diff --git a/crypto/crypto.csproj b/crypto/crypto.csproj
index a1e217aca..3f942800c 100644
--- a/crypto/crypto.csproj
+++ b/crypto/crypto.csproj
@@ -249,6 +249,11 @@
                     BuildAction = "Compile"
                 />
                 <File
+                    RelPath = "src\asn1\BERBitString.cs"
+                    SubType = "Code"
+                    BuildAction = "Compile"
+                />
+                <File
                     RelPath = "src\asn1\BERGenerator.cs"
                     SubType = "Code"
                     BuildAction = "Compile"
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);
diff --git a/crypto/src/math/BigInteger.cs b/crypto/src/math/BigInteger.cs
index 2ca3da978..794f252e8 100644
--- a/crypto/src/math/BigInteger.cs
+++ b/crypto/src/math/BigInteger.cs
@@ -964,7 +964,7 @@ namespace Org.BouncyCastle.Math
         //
         // BitLen(value) is the number of bits in value.
         //
-        private static int BitLen(int w)
+        internal static int BitLen(int w)
         {
             uint v = (uint)w;
             uint t = v >> 24;
diff --git a/crypto/src/ocsp/BasicOCSPResp.cs b/crypto/src/ocsp/BasicOCSPResp.cs
index 4253726bb..dec3b0bc5 100644
--- a/crypto/src/ocsp/BasicOCSPResp.cs
+++ b/crypto/src/ocsp/BasicOCSPResp.cs
@@ -111,7 +111,7 @@ namespace Org.BouncyCastle.Ocsp
 
 		public byte[] GetSignature()
 		{
-			return resp.Signature.GetBytes();
+			return resp.GetSignatureOctets();
 		}
 
 		private IList GetCertList()
diff --git a/crypto/src/ocsp/OCSPReq.cs b/crypto/src/ocsp/OCSPReq.cs
index 84808e50a..29e8cc015 100644
--- a/crypto/src/ocsp/OCSPReq.cs
+++ b/crypto/src/ocsp/OCSPReq.cs
@@ -153,10 +153,10 @@ namespace Org.BouncyCastle.Ocsp
 			if (!this.IsSigned)
 				return null;
 
-			return req.OptionalSignature.SignatureValue.GetBytes();
+			return req.OptionalSignature.GetSignatureOctets();
 		}
 
-		private IList GetCertList()
+        private IList GetCertList()
 		{
 			// load the certificates if we have any
 
diff --git a/crypto/src/pkcs/Pkcs10CertificationRequest.cs b/crypto/src/pkcs/Pkcs10CertificationRequest.cs
index 1789f2a70..633a57ebe 100644
--- a/crypto/src/pkcs/Pkcs10CertificationRequest.cs
+++ b/crypto/src/pkcs/Pkcs10CertificationRequest.cs
@@ -344,7 +344,7 @@ namespace Org.BouncyCastle.Pkcs
 
                 Platform.Dispose(streamCalculator.Stream);
 
-                return ((IVerifier)streamCalculator.GetResult()).IsVerified(sigBits.GetBytes());
+                return ((IVerifier)streamCalculator.GetResult()).IsVerified(sigBits.GetOctets());
             }
             catch (Exception e)
             {
diff --git a/crypto/src/tsp/TimeStampResponseGenerator.cs b/crypto/src/tsp/TimeStampResponseGenerator.cs
index 8d798de67..b596f8d97 100644
--- a/crypto/src/tsp/TimeStampResponseGenerator.cs
+++ b/crypto/src/tsp/TimeStampResponseGenerator.cs
@@ -166,9 +166,8 @@ namespace Org.BouncyCastle.Tsp
         class FailInfo
             : DerBitString
         {
-            internal FailInfo(
-                int failInfoValue)
-                : base(GetBytes(failInfoValue), GetPadBits(failInfoValue))
+            internal FailInfo(int failInfoValue)
+                : base(failInfoValue)
             {
             }
         }
diff --git a/crypto/src/x509/X509Certificate.cs b/crypto/src/x509/X509Certificate.cs
index fc7f96aa9..472ef7308 100644
--- a/crypto/src/x509/X509Certificate.cs
+++ b/crypto/src/x509/X509Certificate.cs
@@ -237,10 +237,10 @@ namespace Org.BouncyCastle.X509
 		/// <returns>A byte array containg the signature of the certificate.</returns>
 		public virtual byte[] GetSignature()
 		{
-			return c.Signature.GetBytes();
+			return c.GetSignatureOctets();
 		}
 
-		/// <summary>
+        /// <summary>
 		/// A meaningful version of the Signature Algorithm. (EG SHA1WITHRSA)
 		/// </summary>
 		/// <returns>A sting representing the signature algorithm.</returns>
diff --git a/crypto/src/x509/X509Crl.cs b/crypto/src/x509/X509Crl.cs
index 53de3e91f..ee564dacb 100644
--- a/crypto/src/x509/X509Crl.cs
+++ b/crypto/src/x509/X509Crl.cs
@@ -211,7 +211,7 @@ namespace Org.BouncyCastle.X509
 
 		public virtual byte[] GetSignature()
 		{
-			return c.Signature.GetBytes();
+			return c.GetSignatureOctets();
 		}
 
 		public virtual string SigAlgName
diff --git a/crypto/src/x509/X509V2AttributeCertificate.cs b/crypto/src/x509/X509V2AttributeCertificate.cs
index 9376538a1..c41b31239 100644
--- a/crypto/src/x509/X509V2AttributeCertificate.cs
+++ b/crypto/src/x509/X509V2AttributeCertificate.cs
@@ -147,9 +147,14 @@ namespace Org.BouncyCastle.X509
 				throw new CertificateNotYetValidException("certificate not valid until " + NotBefore);
 		}
 
+        public virtual AlgorithmIdentifier SignatureAlgorithm
+        {
+            get { return cert.SignatureAlgorithm; }
+        }
+
 		public virtual byte[] GetSignature()
 		{
-			return cert.SignatureValue.GetBytes();
+            return cert.GetSignatureOctets();
 		}
 
         public virtual void Verify(
diff --git a/crypto/test/src/asn1/test/BitStringTest.cs b/crypto/test/src/asn1/test/BitStringTest.cs
index 3a2dc3156..fccaf8fa0 100644
--- a/crypto/test/src/asn1/test/BitStringTest.cs
+++ b/crypto/test/src/asn1/test/BitStringTest.cs
@@ -4,44 +4,132 @@ using System.IO;
 using NUnit.Framework;
 
 using Org.BouncyCastle.Asn1.X509;
+using Org.BouncyCastle.Utilities;
+using Org.BouncyCastle.Utilities.Encoders;
 using Org.BouncyCastle.Utilities.Test;
 
 namespace Org.BouncyCastle.Asn1.Tests
 {
     [TestFixture]
     public class BitStringTest
-        : ITest
+        : SimpleTest
     {
-        public ITestResult Perform()
+        private void DoTestZeroLengthStrings()
+        {
+            // basic construction
+            DerBitString s1 = new DerBitString(new byte[0], 0);
+
+            s1.GetBytes();
+
+            if (!Arrays.AreEqual(s1.GetEncoded(), Hex.Decode("030100")))
+            {
+                Fail("zero encoding wrong");
+            }
+
+            try
+            {
+                new DerBitString(null, 1);
+                Fail("exception not thrown");
+            }
+            catch (ArgumentNullException e)
+            {
+                //if (!"data cannot be null".Equals(e.Message))
+                //{
+                //    Fail("Unexpected exception");
+                //}
+            }
+
+            try
+            {
+                new DerBitString(new byte[0], 1);
+                Fail("exception not thrown");
+            }
+            catch (ArgumentException e)
+            {
+                //if (!"zero length data with non-zero pad bits".Equals(e.Message))
+                //{
+                //    Fail("Unexpected exception");
+                //}
+            }
+
+            try
+            {
+                new DerBitString(new byte[1], 8);
+                Fail("exception not thrown");
+            }
+            catch (ArgumentException e)
+            {
+                //if (!"pad bits cannot be greater than 7 or less than 0".Equals(e.Message))
+                //{
+                //    Fail("Unexpected exception");
+                //}
+            }
+
+            DerBitString s2 = new DerBitString(0);
+            if (!Arrays.AreEqual(s1.GetEncoded(), s2.GetEncoded()))
+            {
+                Fail("zero encoding wrong");
+            }
+        }
+
+        private void DoTestRandomPadBits()
+        {
+            byte[] test = Hex.Decode("030206c0");
+
+            byte[] test1 = Hex.Decode("030206f0");
+            byte[] test2 = Hex.Decode("030206c1");
+            byte[] test3 = Hex.Decode("030206c7");
+            byte[] test4 = Hex.Decode("030206d1");
+
+            EncodingCheck(test, test1);
+            EncodingCheck(test, test2);
+            EncodingCheck(test, test3);
+            EncodingCheck(test, test4);
+        }
+
+        private void EncodingCheck(byte[] derData, byte[] dlData)
+        {
+            if (Arrays.AreEqual(derData, Asn1Object.FromByteArray(dlData).GetEncoded()))
+            {
+                //Fail("failed DL check");
+                Fail("failed BER check");
+            }
+            if (!Arrays.AreEqual(derData, Asn1Object.FromByteArray(dlData).GetDerEncoded()))
+            {
+                Fail("failed DER check");
+            }
+        }
+
+        public override void PerformTest()
         {
             KeyUsage k = new KeyUsage(KeyUsage.DigitalSignature);
             if ((k.GetBytes()[0] != (byte)KeyUsage.DigitalSignature) || (k.PadBits != 7))
             {
-                return new SimpleTestResult(false, Name + ": failed digitalSignature");
+                Fail("failed digitalSignature");
             }
 
             k = new KeyUsage(KeyUsage.NonRepudiation);
             if ((k.GetBytes()[0] != (byte)KeyUsage.NonRepudiation) || (k.PadBits != 6))
             {
-                return new SimpleTestResult(false, Name + ": failed nonRepudiation");
+                Fail("failed nonRepudiation");
             }
 
             k = new KeyUsage(KeyUsage.KeyEncipherment);
             if ((k.GetBytes()[0] != (byte)KeyUsage.KeyEncipherment) || (k.PadBits != 5))
             {
-                return new SimpleTestResult(false, Name + ": failed keyEncipherment");
+                Fail("failed keyEncipherment");
             }
 
             k = new KeyUsage(KeyUsage.CrlSign);
             if ((k.GetBytes()[0] != (byte)KeyUsage.CrlSign)  || (k.PadBits != 1))
             {
-                return new SimpleTestResult(false, Name + ": failed cRLSign");
+                Fail("failed cRLSign");
             }
 
             k = new KeyUsage(KeyUsage.DecipherOnly);
             if ((k.GetBytes()[1] != (byte)(KeyUsage.DecipherOnly >> 8))  || (k.PadBits != 7))
             {
-                return new SimpleTestResult(false, Name + ": failed decipherOnly");
+                Fail("failed decipherOnly");
             }
 
 			// test for zero length bit string
@@ -51,27 +139,25 @@ namespace Org.BouncyCastle.Asn1.Tests
 			}
 			catch (IOException e)
 			{
-				return new SimpleTestResult(false, Name + ": " + e);
+				Fail(e.ToString());
 			}
 
-            return new SimpleTestResult(true, Name + ": Okay");
+            DoTestRandomPadBits();
+            DoTestZeroLengthStrings();
         }
 
-        public string Name
+        public override string Name
         {
 			get { return "BitString"; }
         }
 
-		public static void Main(
+        public static void Main(
             string[] args)
         {
-            ITest test = new BitStringTest();
-            ITestResult result = test.Perform();
-
-			Console.WriteLine(result);
+            RunTest(new BitStringTest());
         }
 
-		[Test]
+        [Test]
         public void TestFunction()
         {
             string resultText = Perform().ToString();
diff --git a/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs b/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
index 48af9e0f8..0cc1883ba 100644
--- a/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
+++ b/crypto/test/src/crypto/tls/test/TlsTestClientImpl.cs
@@ -128,14 +128,14 @@ namespace Org.BouncyCastle.Crypto.Tls.Tests
             Asn1EncodableVector v = new Asn1EncodableVector();
             v.Add(cert.TbsCertificate);
             v.Add(cert.SignatureAlgorithm);
-            v.Add(CorruptBitString(cert.Signature));
+            v.Add(CorruptSignature(cert.Signature));
 
             return X509CertificateStructure.GetInstance(new DerSequence(v));
         }
 
-        protected virtual DerBitString CorruptBitString(DerBitString bs)
+        protected virtual DerBitString CorruptSignature(DerBitString bs)
         {
-            return new DerBitString(CorruptBit(bs.GetBytes()));
+            return new DerBitString(CorruptBit(bs.GetOctets()));
         }
 
         protected virtual byte[] CorruptBit(byte[] bs)
diff --git a/crypto/test/src/test/PKCS10CertRequestTest.cs b/crypto/test/src/test/PKCS10CertRequestTest.cs
index 819439cd8..ba62db32f 100644
--- a/crypto/test/src/test/PKCS10CertRequestTest.cs
+++ b/crypto/test/src/test/PKCS10CertRequestTest.cs
@@ -210,7 +210,7 @@ namespace Org.BouncyCastle.Tests
             byte[] b = req.GetCertificationRequestInfo().GetEncoded();
             sig.BlockUpdate(b, 0, b.Length);
 
-            if (!sig.VerifySignature(req.Signature.GetBytes()))
+            if (!sig.VerifySignature(req.GetSignatureOctets()))
             {
                 Fail("signature not mapped correctly.");
             }
@@ -264,7 +264,7 @@ namespace Org.BouncyCastle.Tests
             byte[] b = req.GetCertificationRequestInfo().GetEncoded();
             sig.BlockUpdate(b, 0, b.Length);
 
-            if (!sig.VerifySignature(req.Signature.GetBytes()))
+            if (!sig.VerifySignature(req.GetSignatureOctets()))
             {
                 Fail("signature not mapped correctly.");
             }
@@ -325,7 +325,7 @@ namespace Org.BouncyCastle.Tests
             byte[] encoded = req.GetCertificationRequestInfo().GetEncoded();
             sig.BlockUpdate(encoded, 0, encoded.Length);
 
-            if (!sig.VerifySignature(req.Signature.GetBytes()))
+            if (!sig.VerifySignature(req.GetSignatureOctets()))
             {
                 Fail("signature not mapped correctly.");
             }