From: Junio C Hamano <gitster@pobox.com>
To: M Hickford <mirth.hickford@gmail.com>
Cc: M Hickford via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v3] credential/libsecret: support password_expiry_utc
Date: Mon, 15 May 2023 11:14:55 -0700 [thread overview]
Message-ID: <xmqq353xa2cg.fsf@gitster.g> (raw)
In-Reply-To: <CAGJzqskMwOJkriH6serqdwAVYi+fftEL8ohJd-suP6v+OxB_bg@mail.gmail.com> (M. Hickford's message of "Mon, 15 May 2023 11:50:01 +0100")
M Hickford <mirth.hickford@gmail.com> writes:
>> +static const SecretSchema schema = {
>> + "org.git.Password",
>> + /* Ignore schema name for backwards compatibility with previous versions */
>> + SECRET_SCHEMA_DONT_MATCH_NAME,
>> + {
>> + { "user", SECRET_SCHEMA_ATTRIBUTE_STRING },
>> + { "object", SECRET_SCHEMA_ATTRIBUTE_STRING },
>> + { "protocol", SECRET_SCHEMA_ATTRIBUTE_STRING },
>> + { "port", SECRET_SCHEMA_ATTRIBUTE_INTEGER },
>> + { "server", SECRET_SCHEMA_ATTRIBUTE_STRING },
>> + { "password_expiry_utc", SECRET_SCHEMA_ATTRIBUTE_INTEGER },
>
> I've been testing this patch with credential-generating helper
> git-credential-helper. It works, but because libsecret overwrites
> items if and only if the attributes match exactly, you end up with
> many items in the secret store that differ only by expiry date. This
> is inelegant, and confusing to users. Please hold this patch, don't
> merge to master. A solution might be to store the expiry date as the
> secret of a separate item (even though the value is not confidential)
Thanks for stopping me. I'll mark the topic as "on hold".
It does sound problematic, but if we think about what is used as
keys and what is used as values, it does make a lot more sense to
store the expiry as part of a value. After all, we are not even
asking "give me the password that will expire in the most distant
future" or anything like that. We consult the database with "who
wants to access what server over which protocol at what port" as the
key and expect we find the suitable authentication material to use.
It would be best if we can treat the expiry date as an additional
attribute of that authentication material.
Do the methods to store and retrieve a password from the keyring
allow us to add such an extra attribute to the password? I have no
idea how the Gnome keyring API works, but is there a way to mark
each entry in the SecretSchemaAttributes as "this is used as a key"
vs "this is used as a value---do not match"? Would thinking along
such a line help?
Another possibility would be to store encoded concatenation of the
real password and expiration timestamp and decode them into two upon
retrieval. If we were the only user of the keystore, that may work,
but if we are sharing the keystore with other applications, it would
be a non-starter.
What do other application do, when using the keyring to store
expirable passwords with services that do let you know the
expiration time of the password? If they just ask the users again
only after finding out that the password did not work, perhaps we
should do the same, without being proactive and notice the expiry
ourselves? That is, instead of failing the access to the server
immediately upon seeing an auth failure, if the authentication
material is know to have expiration time, can we let the application
layer to ask the end-user to provide an refreshed password and try
again? For such a scheme, we do not have to store ever-changing
"password_expiry_utc" and contaminate the keyring with crufts whose
expiry dates are the only difference. Instead we can just have a
Boolean "does this site expire a valid password?" and use it to
behave differently, if desired, from sites for which the passwords
do not expire, perhaps?
next prev parent reply other threads:[~2023-05-15 18:17 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-14 21:32 [PATCH] credential/libsecret: support password_expiry_utc M Hickford via GitGitGadget
2023-03-25 7:36 ` [PATCH v2] " M Hickford via GitGitGadget
2023-05-04 17:42 ` Junio C Hamano
2023-05-05 7:00 ` M Hickford
2023-05-05 7:04 ` [PATCH v3] " M Hickford via GitGitGadget
2023-05-15 10:50 ` M Hickford
2023-05-15 18:14 ` Junio C Hamano [this message]
2023-05-16 8:03 ` M Hickford
2023-05-16 16:10 ` Junio C Hamano
2023-05-17 6:55 ` [PATCH v4] credential/libsecret: store new attributes M Hickford via GitGitGadget
2023-06-16 19:55 ` [PATCH v5] " M Hickford via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq353xa2cg.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=mirth.hickford@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).