Bug #925: typing error in src/backend/libpq/be-secure.c ???

Started by PostgreSQL Bugs Listabout 23 years ago5 messagesbugs
Jump to latest
#1PostgreSQL Bugs List
pgsql-bugs@postgresql.org

Sergey N. Yatskevich (syatskevich@n21lab.gosniias.msk.ru) reports a bug with a severity of 4
The lower the number the more severe it is.

Short Description
typing error in src/backend/libpq/be-secure.c ???

Long Description
In src/backend/libpq/be-secure.c: secure_write
on SSL_ERROR_WANT_WRITE call secure_read instead
secure_write again. May be is this a typing error?

Sample Code

No file was uploaded with this report

#2Bruce Momjian
bruce@momjian.us
In reply to: PostgreSQL Bugs List (#1)
Re: Bug #925: typing error in src/backend/libpq/be-secure.c

Yep, typo. Patched to CVS current and backpatched to 7.3.X.

---------------------------------------------------------------------------

pgsql-bugs@postgresql.org wrote:

Sergey N. Yatskevich (syatskevich@n21lab.gosniias.msk.ru) reports a bug with a severity of 4
The lower the number the more severe it is.

Short Description
typing error in src/backend/libpq/be-secure.c ???

Long Description
In src/backend/libpq/be-secure.c: secure_write
on SSL_ERROR_WANT_WRITE call secure_read instead
secure_write again. May be is this a typing error?

Sample Code

No file was uploaded with this report

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+2-2
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: Bug #925: typing error in src/backend/libpq/be-secure.c

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Yep, typo. Patched to CVS current and backpatched to 7.3.X.

I think this fix is exactly backward. Why would SSL_write need to
return ERROR_WANT_WRITE? It couldn't. The correct fix is that
SSL_write might return ERROR_WANT_READ, for which reading would be
the right response.

BTW the real problem, both here and elsewhere in this file, is the
lack of a "default: elog-out" case in the switch statements. This
code will simply break if any unexpected case occurs.

regards, tom lane

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: Bug #925: typing error in src/backend/libpq/be-secure.c

Actually, the docs say SSL_read/write can error needing READ or WRITE:

http://www.openssl.org/docs/ssl/SSL_read.html#

The SSL_write docs are the same. I have applied the following patch to
handle both new cases. Does this help the SSL test program you have?

---------------------------------------------------------------------------

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Yep, typo. Patched to CVS current and backpatched to 7.3.X.

I think this fix is exactly backward. Why would SSL_write need to
return ERROR_WANT_WRITE? It couldn't. The correct fix is that
SSL_write might return ERROR_WANT_READ, for which reading would be
the right response.

BTW the real problem, both here and elsewhere in this file, is the
lack of a "default: elog-out" case in the switch statements. This
code will simply break if any unexpected case occurs.

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Attachments:

/bjm/difftext/plainDownload+6-0
#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: Bug #925: typing error in src/backend/libpq/be-secure.c

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Actually, the docs say SSL_read/write can error needing READ or WRITE:
http://www.openssl.org/docs/ssl/SSL_read.html#

Only when using a non-blocking BIO, which the backend had better not be
doing. But a few lines of useless code aren't a big deal...

The SSL_write docs are the same. I have applied the following patch to
handle both new cases. Does this help the SSL test program you have?

Where is the "default:" case to preserve sanity when the SSL call returns
an error code other than the ones the switch knows about? ISTM the lack
of this defense *is* a big deal.

regards, tom lane