Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6647248

Browse files
committed
Don't allow immediate interrupts during authentication anymore.
We used to handle authentication_timeout by settingImmediateInterruptOK to true during large parts of the authenticationphase of a new connection. While that happens to work acceptably inpractice, it's not particularly nice and has ugly corner cases.Previous commits converted the FE/BE communication to use latches andimplemented support for interrupt handling during bothsend/recv. Building on top of that work we can get rid ofImmediateInterruptOK during authentication, by immediately treatingtimeouts during authentication as a reason to die. As die interruptsare handled immediately during client communication that provides asensibly quick reaction time to authentication timeout.Additionally add a few CHECK_FOR_INTERRUPTS() to some more complexauthentication methods. More could be added, but this already shouldprovides a reasonable coverage.While it this overall increases the maximum time till a timeout isreacted to, it greatly reduces complexity and increasesreliability. That seems like a overall win. If the increase proves tobe noticeable we can deal with those cases by moving to nonblockingnetwork code and add interrupt checking there.Reviewed-By: Heikki Linnakangas
1 parentcec916f commit6647248

File tree

5 files changed

+41
-37
lines changed

5 files changed

+41
-37
lines changed

‎src/backend/libpq/auth.c

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -306,13 +306,6 @@ ClientAuthentication(Port *port)
306306
*/
307307
hba_getauthmethod(port);
308308

309-
/*
310-
* Enable immediate response to SIGTERM/SIGINT/timeout interrupts. (We
311-
* don't want this during hba_getauthmethod() because it might have to do
312-
* database access, eg for role membership checks.)
313-
*/
314-
ImmediateInterruptOK= true;
315-
/* And don't forget to detect one that already arrived */
316309
CHECK_FOR_INTERRUPTS();
317310

318311
/*
@@ -566,9 +559,6 @@ ClientAuthentication(Port *port)
566559
sendAuthRequest(port,AUTH_REQ_OK);
567560
else
568561
auth_failed(port,status,logdetail);
569-
570-
/* Done with authentication, so we should turn off immediate interrupts */
571-
ImmediateInterruptOK= false;
572562
}
573563

574564

@@ -580,6 +570,8 @@ sendAuthRequest(Port *port, AuthRequest areq)
580570
{
581571
StringInfoDatabuf;
582572

573+
CHECK_FOR_INTERRUPTS();
574+
583575
pq_beginmessage(&buf,'R');
584576
pq_sendint(&buf, (int32)areq,sizeof(int32));
585577

@@ -613,6 +605,8 @@ sendAuthRequest(Port *port, AuthRequest areq)
613605
*/
614606
if (areq!=AUTH_REQ_OK)
615607
pq_flush();
608+
609+
CHECK_FOR_INTERRUPTS();
616610
}
617611

618612
/*
@@ -851,6 +845,9 @@ pg_GSS_recvauth(Port *port)
851845
do
852846
{
853847
pq_startmsgread();
848+
849+
CHECK_FOR_INTERRUPTS();
850+
854851
mtype=pq_getbyte();
855852
if (mtype!='p')
856853
{
@@ -900,6 +897,8 @@ pg_GSS_recvauth(Port *port)
900897
maj_stat,min_stat,
901898
(unsignedint)port->gss->outbuf.length,gflags);
902899

900+
CHECK_FOR_INTERRUPTS();
901+
903902
if (port->gss->outbuf.length!=0)
904903
{
905904
/*
@@ -1396,6 +1395,9 @@ interpret_ident_response(const char *ident_response,
13961395
*IP addresses and port numbers are in network byte order.
13971396
*
13981397
*But iff we're unable to get the information from ident, return false.
1398+
*
1399+
*XXX: Using WaitLatchOrSocket() and doing a CHECK_FOR_INTERRUPTS() if the
1400+
*latch was set would improve the responsiveness to timeouts/cancellations.
13991401
*/
14001402
staticint
14011403
ident_inet(hbaPort*port)
@@ -1510,6 +1512,8 @@ ident_inet(hbaPort *port)
15101512
/* loop in case send is interrupted */
15111513
do
15121514
{
1515+
CHECK_FOR_INTERRUPTS();
1516+
15131517
rc=send(sock_fd,ident_query,strlen(ident_query),0);
15141518
}while (rc<0&&errno==EINTR);
15151519

@@ -1525,6 +1529,8 @@ ident_inet(hbaPort *port)
15251529

15261530
do
15271531
{
1532+
CHECK_FOR_INTERRUPTS();
1533+
15281534
rc=recv(sock_fd,ident_response,sizeof(ident_response)-1,0);
15291535
}while (rc<0&&errno==EINTR);
15301536

@@ -2413,6 +2419,10 @@ CheckRADIUSAuth(Port *port)
24132419
* call to select() with a timeout, since somebody can be sending invalid
24142420
* packets to our port thus causing us to retry in a loop and never time
24152421
* out.
2422+
*
2423+
* XXX: Using WaitLatchOrSocket() and doing a CHECK_FOR_INTERRUPTS() if
2424+
* the latch was set would improve the responsiveness to
2425+
* timeouts/cancellations.
24162426
*/
24172427
gettimeofday(&endtime,NULL);
24182428
endtime.tv_sec+=RADIUS_TIMEOUT;

‎src/backend/libpq/be-secure-openssl.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,11 @@ be_tls_open_server(Port *port)
377377
/* not allowed during connection establishment */
378378
Assert(!port->noblock);
379379

380+
/*
381+
* No need to care about timeouts/interrupts here. At this
382+
* point authentication_timeout still employs
383+
* StartupPacketTimeoutHandler() which directly exits.
384+
*/
380385
if (err==SSL_ERROR_WANT_READ)
381386
waitfor=WL_SOCKET_READABLE;
382387
else

‎src/backend/libpq/crypt.c

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,6 @@ md5_crypt_verify(const Port *port, const char *role, char *client_pass,
4747
Datumdatum;
4848
boolisnull;
4949

50-
/*
51-
* Disable immediate interrupts while doing database access. (Note we
52-
* don't bother to turn this back on if we hit one of the failure
53-
* conditions, since we can expect we'll just exit right away anyway.)
54-
*/
55-
ImmediateInterruptOK= false;
56-
5750
/* Get role info from pg_authid */
5851
roleTup=SearchSysCache1(AUTHNAME,PointerGetDatum(role));
5952
if (!HeapTupleIsValid(roleTup))
@@ -80,9 +73,6 @@ md5_crypt_verify(const Port *port, const char *role, char *client_pass,
8073
if (*shadow_pass=='\0')
8174
returnSTATUS_ERROR;/* empty password */
8275

83-
/* Re-enable immediate response to SIGTERM/SIGINT/timeout interrupts */
84-
ImmediateInterruptOK= true;
85-
/* And don't forget to detect one that already arrived */
8676
CHECK_FOR_INTERRUPTS();
8777

8878
/*

‎src/backend/tcop/postgres.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2880,7 +2880,11 @@ ProcessInterrupts(void)
28802880
/* As in quickdie, don't risk sending to client during auth */
28812881
if (ClientAuthInProgress&&whereToSendOutput==DestRemote)
28822882
whereToSendOutput=DestNone;
2883-
if (IsAutoVacuumWorkerProcess())
2883+
if (ClientAuthInProgress)
2884+
ereport(FATAL,
2885+
(errcode(ERRCODE_QUERY_CANCELED),
2886+
errmsg("canceling authentication due to timeout")));
2887+
elseif (IsAutoVacuumWorkerProcess())
28842888
ereport(FATAL,
28852889
(errcode(ERRCODE_ADMIN_SHUTDOWN),
28862890
errmsg("terminating autovacuum process due to administrator command")));
@@ -2959,17 +2963,6 @@ ProcessInterrupts(void)
29592963
}
29602964

29612965
QueryCancelPending= false;
2962-
if (ClientAuthInProgress)
2963-
{
2964-
ImmediateInterruptOK= false;/* not idle anymore */
2965-
LockErrorCleanup();
2966-
/* As in quickdie, don't risk sending to client during auth */
2967-
if (whereToSendOutput==DestRemote)
2968-
whereToSendOutput=DestNone;
2969-
ereport(ERROR,
2970-
(errcode(ERRCODE_QUERY_CANCELED),
2971-
errmsg("canceling authentication due to timeout")));
2972-
}
29732966

29742967
/*
29752968
* If LOCK_TIMEOUT and STATEMENT_TIMEOUT indicators are both set, we

‎src/backend/utils/init/postinit.c

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,18 +1099,24 @@ ShutdownPostgres(int code, Datum arg)
10991099
staticvoid
11001100
StatementTimeoutHandler(void)
11011101
{
1102+
intsig=SIGINT;
1103+
1104+
/*
1105+
* During authentication the timeout is used to deal with
1106+
* authentication_timeout - we want to quit in response to such timeouts.
1107+
*/
1108+
if (ClientAuthInProgress)
1109+
sig=SIGTERM;
1110+
11021111
#ifdefHAVE_SETSID
11031112
/* try to signal whole process group */
1104-
kill(-MyProcPid,SIGINT);
1113+
kill(-MyProcPid,sig);
11051114
#endif
1106-
kill(MyProcPid,SIGINT);
1115+
kill(MyProcPid,sig);
11071116
}
11081117

11091118
/*
11101119
* LOCK_TIMEOUT handler: trigger a query-cancel interrupt.
1111-
*
1112-
* This is identical to StatementTimeoutHandler, but since it's so short,
1113-
* we might as well keep the two functions separate for clarity.
11141120
*/
11151121
staticvoid
11161122
LockTimeoutHandler(void)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp