> On Jan. 16, 2017, 10:39 p.m., Andrew Stitcher wrote:
> > proton-c/src/sasl/sasl.c, line 304
> > <https://reviews.apache.org/r/55585/diff/1/?file=1605967#file1605967line304>
> >
> >     I think that you are now going to get the logging happening twice. Once 
> > for the encryption before switching layers and again when the encryption 
> > layer is switched in.
> >     
> >     I suggest changing this log message to specify that this is input only 
> > encryption before the layers get switched to avoid logging confusion.
> >     
> >     Or maybe just removing the message completely.

It does already state that it is only for input. I would also be happy to 
remove it entirely if preferred.


- Gordon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55585/#review161793
-----------------------------------------------------------


On Jan. 16, 2017, 7:55 p.m., Gordon Sim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55585/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2017, 7:55 p.m.)
> 
> 
> Review request for qpid and Andrew Stitcher.
> 
> 
> Repository: qpid-proton-git
> 
> 
> Description
> -------
> 
> In pn_input_read_sasl, when a successful outcome frame is read, it will set 
> the desired_state to SASL_RECVED_OUTCOME_SUCCEED. This means that 
> pni_sasl_is_final_input_state() will return true. However from what I can 
> tell, the last_state, which is what is checked by 
> pni_sasl_is_final_output_state(), is only set to this same value when 
> pni_post_sasl_frame() is called, and the client will never need to call that 
> after receiving an outcome (as the sasl exchange is then over).
> 
> So if we get two calls to pn_input_read_sasl(), one to read the outcome, the 
> next to read the first encrypted data, *without* a pn_output_write_sasl() 
> between them, pni_sasl_is_final_input_state() will return true but 
> pni_sasl_is_final_output_state() will return false on the second read, which 
> results in the second recv call passing it to the passthru layer even though 
> it may be encrypted. 
> 
> This patch ensures that if the final input state has been reached for sasl, 
> and encryption was negotiated, further incoming data is always decoded (even 
> if we have not yet reached the final output state and so have not set 
> io_layers).
> 
> 
> Diffs
> -----
> 
>   proton-c/src/sasl/sasl.c 69fb6b2 
> 
> Diff: https://reviews.apache.org/r/55585/diff/
> 
> 
> Testing
> -------
> 
> Existing tests pass. Rerproducer for original issue - occasional failure of 
> proton client using DIGEST-MD5 against qpidd - passes reliably (i.e. while 
> ./simple_send.py -a guest:guest@localhost -m 1; do echo ok; done against 
> qpidd, with only DIGEST-MD5 allowed).
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>

Reply via email to