[security-dev 00859]: Re: Code review request: 6813340: X509Factory should not depend on is.available()==0

[Xuelei Fan]

Max,

I think about the readBERInternal() more, would you like add more robust 
codes?
1. checking that the tag code is less than 30;
2. checking that the tag form is constructed for indef-length.

Otherwise, looks fine for me.

Andrew

Weijun Wang wrote:
> The new webrev is at
> http://cr.openjdk.java.net/~weijun/6813340/webrev.03
>
> Changes compared to last webrev is:
>
> diff -r 59db2c7c37fa
> src/share/classes/sun/security/provider/X509Factory.java
> --- a/src/share/classes/sun/security/provider/X509Factory.java
> +++ b/src/share/classes/sun/security/provider/X509Factory.java
> @@ -113,7 +113,7 @@
>      private static int readFully(InputStream in, ByteArrayOutputStream
> bout,
>              int length) throws IOException {
>          int read = 0;
> -        byte[] buffer = new byte[length];
> +        byte[] buffer = new byte[2048];
>          while (length > 0) {
>              int n = in.read(buffer, 0, length);
>              if (n <= 0) {
> @@ -561,7 +561,9 @@
>              // Step 4: Consume the footer
>              while (true) {
>                  int next = is.read();
> -                if (next == -1 || next == end) {
> +                // Add next == '\n' for maximum safety, in case endline
> +                // is not consistent.
> +                if (next == -1 || next == end || next == '\n') {
>                      break;
>                  }
>              }
> @@ -625,6 +627,17 @@
>                  bout.write(highByte);
>                  bout.write(lowByte);
>                  length = (highByte << 8) | lowByte;
> +            } else if (n == 0x83) {
> +                int highByte = is.read();
> +                int midByte = is.read();
> +                int lowByte = is.read();
> +                if (lowByte == -1) {
> +                    throw new IOException("Incomplete BER/DER length
> info");
> +                }
> +                bout.write(highByte);
> +                bout.write(midByte);
> +                bout.write(lowByte);
> +                length = (highByte << 16) | (midByte << 8) | lowByte;
>              } else { // ignore longer length forms
>                  throw new IOException("Invalid BER/DER data (too huge?)");
>              }
>
> I didn't support 0x84 because strictly that would mean int32 is not
> enough and I need to use long and another readFully()... seems not
> worthy. Or I can check to make sure the first length byte is <0x80,
> that's also a little complicated.
>
> Thanks
> Max
>
> Xuelei Fan wrote:
>   
>> Xuelei Fan wrote:
>>     
>>>>> t to support indefinite-length, I think you can simply keep reading
>>>>> until get two zero bytes.
>>>>>           
>>>> As I understand, "0x80 0x06 0x07 0x01 0x00 0x00" is not an indef-len
>>>> BER.
>>>>         
>>> You're right, it is not a valid indef-len BER. I will look twice of
>>> readBERInternal() tomorrow.
>>>
>>>
>>>       
>> The process of parsing indef-len BER content looks fine for me.
>>
>> Andrew
>>