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

Commit5513dc6

Browse files
committed
Improve error handling of HMAC computations
This is similar tob69aba7, except that this completes the work forHMAC with a new routine called pg_hmac_error() that would provide morecontext about the type of error that happened during a HMAC computation:- The fallback HMAC implementation in hmac.c relies on cryptohashes, soin some code paths it is necessary to return back the error generated bycryptohashes.- For the OpenSSL implementation (hmac_openssl.c), the logic is verysimilar to cryptohash_openssl.c, where the error context comes fromOpenSSL if one of its internal routines failed, with different errorcodes if something internal to hmac_openssl.c failed or was incorrect.Any in-core code paths that use the centralized HMAC interface arerelated to SCRAM, for errors that are unlikely going to happen, withonly SHA-256. It would be possible to see errors when computing someHMACs with MD5 for example and OpenSSL FIPS enabled, and this commitwould help in reporting the correct errors but nothing in core usesthat. So, at the end, no backpatch to v14 is done, at least for now.Errors in SCRAM related to the computation of the server key, storedkey, etc. need to pass down the potential error context string acrossmore layers of their respective call stacks for the frontend and thebackend, so each surrounding routine is adapted for this purpose.Reviewed-by: Sergey ShinderukDiscussion:https://postgr.es/m/Yd0N9tSAIIkFd+qi@paquier.xyz
1 parent379a28b commit5513dc6

File tree

10 files changed

+294
-55
lines changed

10 files changed

+294
-55
lines changed

‎src/backend/libpq/auth-scram.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@ pg_be_scram_build_secret(const char *password)
465465
pg_saslprep_rcrc;
466466
charsaltbuf[SCRAM_DEFAULT_SALT_LEN];
467467
char*result;
468+
constchar*errstr=NULL;
468469

469470
/*
470471
* Normalize the password with SASLprep. If that doesn't work, because
@@ -482,7 +483,8 @@ pg_be_scram_build_secret(const char *password)
482483
errmsg("could not generate random salt")));
483484

484485
result=scram_build_secret(saltbuf,SCRAM_DEFAULT_SALT_LEN,
485-
SCRAM_DEFAULT_ITERATIONS,password);
486+
SCRAM_DEFAULT_ITERATIONS,password,
487+
&errstr);
486488

487489
if (prep_password)
488490
pfree(prep_password);
@@ -509,6 +511,7 @@ scram_verify_plain_password(const char *username, const char *password,
509511
uint8computed_key[SCRAM_KEY_LEN];
510512
char*prep_password;
511513
pg_saslprep_rcrc;
514+
constchar*errstr=NULL;
512515

513516
if (!parse_scram_secret(secret,&iterations,&encoded_salt,
514517
stored_key,server_key))
@@ -539,10 +542,10 @@ scram_verify_plain_password(const char *username, const char *password,
539542

540543
/* Compute Server Key based on the user-supplied plaintext password */
541544
if (scram_SaltedPassword(password,salt,saltlen,iterations,
542-
salted_password)<0||
543-
scram_ServerKey(salted_password,computed_key)<0)
545+
salted_password,&errstr)<0||
546+
scram_ServerKey(salted_password,computed_key,&errstr)<0)
544547
{
545-
elog(ERROR,"could not compute server key");
548+
elog(ERROR,"could not compute server key: %s",errstr);
546549
}
547550

548551
if (prep_password)
@@ -1113,6 +1116,7 @@ verify_client_proof(scram_state *state)
11131116
uint8client_StoredKey[SCRAM_KEY_LEN];
11141117
pg_hmac_ctx*ctx=pg_hmac_create(PG_SHA256);
11151118
inti;
1119+
constchar*errstr=NULL;
11161120

11171121
/*
11181122
* Calculate ClientSignature. Note that we don't log directly a failure
@@ -1133,7 +1137,8 @@ verify_client_proof(scram_state *state)
11331137
strlen(state->client_final_message_without_proof))<0||
11341138
pg_hmac_final(ctx,ClientSignature,sizeof(ClientSignature))<0)
11351139
{
1136-
elog(ERROR,"could not calculate client signature");
1140+
elog(ERROR,"could not calculate client signature: %s",
1141+
pg_hmac_error(ctx));
11371142
}
11381143

11391144
pg_hmac_free(ctx);
@@ -1143,8 +1148,8 @@ verify_client_proof(scram_state *state)
11431148
ClientKey[i]=state->ClientProof[i] ^ClientSignature[i];
11441149

11451150
/* Hash it one more time, and compare with StoredKey */
1146-
if (scram_H(ClientKey,SCRAM_KEY_LEN,client_StoredKey)<0)
1147-
elog(ERROR,"could not hash stored key");
1151+
if (scram_H(ClientKey,SCRAM_KEY_LEN,client_StoredKey,&errstr)<0)
1152+
elog(ERROR,"could not hash stored key: %s",errstr);
11481153

11491154
if (memcmp(client_StoredKey,state->StoredKey,SCRAM_KEY_LEN)!=0)
11501155
return false;
@@ -1389,7 +1394,8 @@ build_server_final_message(scram_state *state)
13891394
strlen(state->client_final_message_without_proof))<0||
13901395
pg_hmac_final(ctx,ServerSignature,sizeof(ServerSignature))<0)
13911396
{
1392-
elog(ERROR,"could not calculate server signature");
1397+
elog(ERROR,"could not calculate server signature: %s",
1398+
pg_hmac_error(ctx));
13931399
}
13941400

13951401
pg_hmac_free(ctx);

‎src/common/hmac.c

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,21 @@
3838
#defineFREE(ptr) free(ptr)
3939
#endif
4040

41+
/* Set of error states */
42+
typedefenumpg_hmac_errno
43+
{
44+
PG_HMAC_ERROR_NONE=0,
45+
PG_HMAC_ERROR_OOM,
46+
PG_HMAC_ERROR_INTERNAL
47+
}pg_hmac_errno;
48+
4149
/* Internal pg_hmac_ctx structure */
4250
structpg_hmac_ctx
4351
{
4452
pg_cryptohash_ctx*hash;
4553
pg_cryptohash_typetype;
54+
pg_hmac_errnoerror;
55+
constchar*errreason;
4656
intblock_size;
4757
intdigest_size;
4858

@@ -73,6 +83,8 @@ pg_hmac_create(pg_cryptohash_type type)
7383
returnNULL;
7484
memset(ctx,0,sizeof(pg_hmac_ctx));
7585
ctx->type=type;
86+
ctx->error=PG_HMAC_ERROR_NONE;
87+
ctx->errreason=NULL;
7688

7789
/*
7890
* Initialize the context data. This requires to know the digest and
@@ -150,12 +162,16 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t len)
150162
/* temporary buffer for one-time shrink */
151163
shrinkbuf=ALLOC(digest_size);
152164
if (shrinkbuf==NULL)
165+
{
166+
ctx->error=PG_HMAC_ERROR_OOM;
153167
return-1;
168+
}
154169
memset(shrinkbuf,0,digest_size);
155170

156171
hash_ctx=pg_cryptohash_create(ctx->type);
157172
if (hash_ctx==NULL)
158173
{
174+
ctx->error=PG_HMAC_ERROR_OOM;
159175
FREE(shrinkbuf);
160176
return-1;
161177
}
@@ -164,6 +180,8 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t len)
164180
pg_cryptohash_update(hash_ctx,key,len)<0||
165181
pg_cryptohash_final(hash_ctx,shrinkbuf,digest_size)<0)
166182
{
183+
ctx->error=PG_HMAC_ERROR_INTERNAL;
184+
ctx->errreason=pg_cryptohash_error(hash_ctx);
167185
pg_cryptohash_free(hash_ctx);
168186
FREE(shrinkbuf);
169187
return-1;
@@ -184,6 +202,8 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t len)
184202
if (pg_cryptohash_init(ctx->hash)<0||
185203
pg_cryptohash_update(ctx->hash,ctx->k_ipad,ctx->block_size)<0)
186204
{
205+
ctx->error=PG_HMAC_ERROR_INTERNAL;
206+
ctx->errreason=pg_cryptohash_error(ctx->hash);
187207
if (shrinkbuf)
188208
FREE(shrinkbuf);
189209
return-1;
@@ -206,7 +226,11 @@ pg_hmac_update(pg_hmac_ctx *ctx, const uint8 *data, size_t len)
206226
return-1;
207227

208228
if (pg_cryptohash_update(ctx->hash,data,len)<0)
229+
{
230+
ctx->error=PG_HMAC_ERROR_INTERNAL;
231+
ctx->errreason=pg_cryptohash_error(ctx->hash);
209232
return-1;
233+
}
210234

211235
return0;
212236
}
@@ -226,11 +250,16 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t len)
226250

227251
h=ALLOC(ctx->digest_size);
228252
if (h==NULL)
253+
{
254+
ctx->error=PG_HMAC_ERROR_OOM;
229255
return-1;
256+
}
230257
memset(h,0,ctx->digest_size);
231258

232259
if (pg_cryptohash_final(ctx->hash,h,ctx->digest_size)<0)
233260
{
261+
ctx->error=PG_HMAC_ERROR_INTERNAL;
262+
ctx->errreason=pg_cryptohash_error(ctx->hash);
234263
FREE(h);
235264
return-1;
236265
}
@@ -241,6 +270,8 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t len)
241270
pg_cryptohash_update(ctx->hash,h,ctx->digest_size)<0||
242271
pg_cryptohash_final(ctx->hash,dest,len)<0)
243272
{
273+
ctx->error=PG_HMAC_ERROR_INTERNAL;
274+
ctx->errreason=pg_cryptohash_error(ctx->hash);
244275
FREE(h);
245276
return-1;
246277
}
@@ -264,3 +295,36 @@ pg_hmac_free(pg_hmac_ctx *ctx)
264295
explicit_bzero(ctx,sizeof(pg_hmac_ctx));
265296
FREE(ctx);
266297
}
298+
299+
/*
300+
* pg_hmac_error
301+
*
302+
* Returns a static string providing details about an error that happened
303+
* during a HMAC computation.
304+
*/
305+
constchar*
306+
pg_hmac_error(pg_hmac_ctx*ctx)
307+
{
308+
if (ctx==NULL)
309+
return_("out of memory");
310+
311+
/*
312+
* If a reason is provided, rely on it, else fallback to any error code
313+
* set.
314+
*/
315+
if (ctx->errreason)
316+
returnctx->errreason;
317+
318+
switch (ctx->error)
319+
{
320+
casePG_HMAC_ERROR_NONE:
321+
return_("success");
322+
casePG_HMAC_ERROR_INTERNAL:
323+
return_("internal error");
324+
casePG_HMAC_ERROR_OOM:
325+
return_("out of memory");
326+
}
327+
328+
Assert(false);/* cannot be reached */
329+
return_("success");
330+
}

‎src/common/hmac_openssl.c

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include"postgres_fe.h"
2121
#endif
2222

23+
24+
#include<openssl/err.h>
2325
#include<openssl/hmac.h>
2426

2527
#include"common/hmac.h"
@@ -50,17 +52,40 @@
5052
#defineFREE(ptr) free(ptr)
5153
#endif/* FRONTEND */
5254

55+
/* Set of error states */
56+
typedefenumpg_hmac_errno
57+
{
58+
PG_HMAC_ERROR_NONE=0,
59+
PG_HMAC_ERROR_DEST_LEN,
60+
PG_HMAC_ERROR_OPENSSL
61+
}pg_hmac_errno;
62+
5363
/* Internal pg_hmac_ctx structure */
5464
structpg_hmac_ctx
5565
{
5666
HMAC_CTX*hmacctx;
5767
pg_cryptohash_typetype;
68+
pg_hmac_errnoerror;
69+
constchar*errreason;
5870

5971
#ifndefFRONTEND
6072
ResourceOwnerresowner;
6173
#endif
6274
};
6375

76+
staticconstchar*
77+
SSLerrmessage(unsigned longecode)
78+
{
79+
if (ecode==0)
80+
returnNULL;
81+
82+
/*
83+
* This may return NULL, but we would fall back to a default error path if
84+
* that were the case.
85+
*/
86+
returnERR_reason_error_string(ecode);
87+
}
88+
6489
/*
6590
* pg_hmac_create
6691
*
@@ -78,6 +103,8 @@ pg_hmac_create(pg_cryptohash_type type)
78103
memset(ctx,0,sizeof(pg_hmac_ctx));
79104

80105
ctx->type=type;
106+
ctx->error=PG_HMAC_ERROR_NONE;
107+
ctx->errreason=NULL;
81108

82109
/*
83110
* Initialization takes care of assigning the correct type for OpenSSL.
@@ -152,7 +179,11 @@ pg_hmac_init(pg_hmac_ctx *ctx, const uint8 *key, size_t len)
152179

153180
/* OpenSSL internals return 1 on success, 0 on failure */
154181
if (status <=0)
182+
{
183+
ctx->errreason=SSLerrmessage(ERR_get_error());
184+
ctx->error=PG_HMAC_ERROR_OPENSSL;
155185
return-1;
186+
}
156187

157188
return0;
158189
}
@@ -174,7 +205,11 @@ pg_hmac_update(pg_hmac_ctx *ctx, const uint8 *data, size_t len)
174205

175206
/* OpenSSL internals return 1 on success, 0 on failure */
176207
if (status <=0)
208+
{
209+
ctx->errreason=SSLerrmessage(ERR_get_error());
210+
ctx->error=PG_HMAC_ERROR_OPENSSL;
177211
return-1;
212+
}
178213
return0;
179214
}
180215

@@ -196,35 +231,57 @@ pg_hmac_final(pg_hmac_ctx *ctx, uint8 *dest, size_t len)
196231
{
197232
casePG_MD5:
198233
if (len<MD5_DIGEST_LENGTH)
234+
{
235+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
199236
return-1;
237+
}
200238
break;
201239
casePG_SHA1:
202240
if (len<SHA1_DIGEST_LENGTH)
241+
{
242+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
203243
return-1;
244+
}
204245
break;
205246
casePG_SHA224:
206247
if (len<PG_SHA224_DIGEST_LENGTH)
248+
{
249+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
207250
return-1;
251+
}
208252
break;
209253
casePG_SHA256:
210254
if (len<PG_SHA256_DIGEST_LENGTH)
255+
{
256+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
211257
return-1;
258+
}
212259
break;
213260
casePG_SHA384:
214261
if (len<PG_SHA384_DIGEST_LENGTH)
262+
{
263+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
215264
return-1;
265+
}
216266
break;
217267
casePG_SHA512:
218268
if (len<PG_SHA512_DIGEST_LENGTH)
269+
{
270+
ctx->error=PG_HMAC_ERROR_DEST_LEN;
219271
return-1;
272+
}
220273
break;
221274
}
222275

223276
status=HMAC_Final(ctx->hmacctx,dest,&outlen);
224277

225278
/* OpenSSL internals return 1 on success, 0 on failure */
226279
if (status <=0)
280+
{
281+
ctx->errreason=SSLerrmessage(ERR_get_error());
282+
ctx->error=PG_HMAC_ERROR_OPENSSL;
227283
return-1;
284+
}
228285
return0;
229286
}
230287

@@ -252,3 +309,36 @@ pg_hmac_free(pg_hmac_ctx *ctx)
252309
explicit_bzero(ctx,sizeof(pg_hmac_ctx));
253310
FREE(ctx);
254311
}
312+
313+
/*
314+
* pg_hmac_error
315+
*
316+
* Returns a static string providing details about an error that happened
317+
* during a HMAC computation.
318+
*/
319+
constchar*
320+
pg_hmac_error(pg_hmac_ctx*ctx)
321+
{
322+
if (ctx==NULL)
323+
return_("out of memory");
324+
325+
/*
326+
* If a reason is provided, rely on it, else fallback to any error code
327+
* set.
328+
*/
329+
if (ctx->errreason)
330+
returnctx->errreason;
331+
332+
switch (ctx->error)
333+
{
334+
casePG_HMAC_ERROR_NONE:
335+
return_("success");
336+
casePG_HMAC_ERROR_DEST_LEN:
337+
return_("destination buffer too small");
338+
casePG_HMAC_ERROR_OPENSSL:
339+
return_("OpenSSL failure");
340+
}
341+
342+
Assert(false);/* cannot be reached */
343+
return_("success");
344+
}

0 commit comments

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