[prev in list] [next in list] [prev in thread] [next in thread] 

List:       bouncycastle-crypto-dev
Subject:    Re: [dev-crypto] Patch to improve TLS logging
From:       Peter Dettman <peter.dettman () bouncycastle ! org>
Date:       2013-09-18 4:29:30
Message-ID: 52392C2A.60808 () bouncycastle ! org
[Download RAW message or body]

Thanks, Jakub.

The patch appears to be against an older version of the library, so the 
first suggestion would be to fork the github mirror 
(https://github.com/bcgit/bc-java) and send a pull request there. This 
is our preferred way to receive patches these days (where privacy 
considerations permit).

The most immediate issue with the patch will be that the IOException 
constructors taking 'cause' arguments were only added in 1.6. Actually 
we've already been working on how to get error-specific messages and 
stack-traces to the user, so there are now 
notifyAlertRaised/notifyAlertReceived methods on TlsPeer which the user 
can override to log these events. I've just committed some final changes 
to make this work semi-well.

Please take a look, and we'll look forward to future patches.

Regards,
Pete Dettman

On 17/09/2013 10:13 PM, Jakub Adamek wrote:
> Hello all, I have a small patch which improves TLS logging: It shows 
> you the exact call stack where the exception originated. What shall I 
> do to make this patch become a part of the official source code?
>
> Thanks, Jakub
>
> Index: TlsProtocol.java
> ===================================================================
> --- TlsProtocol.java    (revision 408)
> +++ TlsProtocol.java    (revision 409)
> @@ -384,7 +384,7 @@
>          {
>              if (!this.closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> e.getAlertDescription());
> +                this.failWithError(AlertLevel.fatal, 
> e.getAlertDescription(), e);
>              }
>              throw e;
>          }
> @@ -392,7 +392,7 @@
>          {
>              if (!this.closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error);
> +                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error, e);
>              }
>              throw e;
>          }
> @@ -400,7 +400,7 @@
>          {
>              if (!this.closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error);
> +                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error, e);
>              }
>              throw e;
>          }
> @@ -417,7 +417,7 @@
>          {
>              if (!this.closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> e.getAlertDescription());
> +                this.failWithError(AlertLevel.fatal, 
> e.getAlertDescription(), e);
>              }
>              throw e;
>          }
> @@ -425,7 +425,7 @@
>          {
>              if (!closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error);
> +                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error, e);
>              }
>              throw e;
>          }
> @@ -433,7 +433,7 @@
>          {
>              if (!closed)
>              {
> -                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error);
> +                this.failWithError(AlertLevel.fatal, 
> AlertDescription.internal_error,e );
>              }
>              throw e;
>          }
> @@ -506,6 +506,12 @@
>          return this.tlsInputStream;
>      }
>
> +    protected void failWithError(short alertLevel, short 
> alertDescription)
> +        throws IOException
> +    {
> +        failWithError(alertLevel, alertDescription, null);
> +    }
> +
>      /**
>       * Terminate this connection with an alert.
>       * <p/>
> @@ -515,7 +521,7 @@
>       * @param alertDescription The exact alert message.
>       * @throws IOException If alert was fatal.
>       */
> -    protected void failWithError(short alertLevel, short 
> alertDescription)
> +    protected void failWithError(short alertLevel, short 
> alertDescription, Exception e)
>          throws IOException
>      {
>          /*
> @@ -539,12 +545,12 @@
>              recordStream.close();
>              if (alertLevel == AlertLevel.fatal)
>              {
> -                throw new IOException(TLS_ERROR_MESSAGE);
> +                throw new IOException(TLS_ERROR_MESSAGE, e);
>              }
>          }
>          else
>          {
> -            throw new IOException(TLS_ERROR_MESSAGE);
> +            throw new IOException(TLS_ERROR_MESSAGE, e);
>          }
>      }
>
>


[prev in list] [next in list] [prev in thread] [next in thread]