Page MenuHomePhabricator
Feed All Stories

Jan 25 2019

epriestley added a comment to T13231: Duo MFA Support.

I think I have this working fairly reasonably, now. There are four major cases:

Jan 25 2019, 12:52 AM · Auth
aeiser added a comment to T13231: Duo MFA Support.

Another issue here is that Duo doesn't seem to have a way to prevent new user creation. YIf you /preauth a user, they get an enroll link whether they already have an account in Duo or not, and there's no apparent way to distinguish between "this creates a new user" and "this encourages an existing user, who has already been created but has not enrolled a device yet, to enroll".

Jan 25 2019, 12:25 AM · Auth
epriestley added a comment to T13231: Duo MFA Support.

Another issue is that we apparently (???) only get one shot at learning the user's (internal-to-Duo) User ID, and only if we're allowed to enroll them. The first call to /enroll gives us a user ID, subsequent calls do not and there's no other way to do the lookup. This isn't really a problem, it just means we're forced to use usernames everywhere (which are mutable/aliasable/etc/etc) when we could otherwise use nice stable user IDs.

Jan 25 2019, 12:13 AM · Auth

Jan 24 2019

epriestley added a comment to T13231: Duo MFA Support.

Another issue here is that Duo doesn't seem to have a way to prevent new user creation. If you /preauth a user, they get an enroll link whether they already have an account in Duo or not, and there's no apparent way to distinguish between "this creates a new user" and "this encourages an existing user, who has already been created but has not enrolled a device yet, to enroll".

Jan 24 2019, 11:43 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

Thanks, that's quite helpful!

Jan 24 2019, 11:26 PM · Auth
aeiser added a comment to T13231: Duo MFA Support.

If we want to synchronize to existing accounts, I think it's not good enough for us to pick the user's email address unless that's also the username the organization already uses -- in your case, is it?

Jan 24 2019, 10:56 PM · Auth
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 24 2019, 10:21 PM · Plans
epriestley added a revision to T13231: Duo MFA Support: Restricted Differential Revision.
Jan 24 2019, 10:21 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

I'm imaging that one use case is that you just want everyone to use MFA, and Duo is a more attractive approach than TOTP (since it's easier to use) or SMS (since it's more secure). In this case, you might just be using Duo as "Better TOTP", and the enrollment/management side isn't as important. For example, if we deployed Duo on secure or in the Phacility cluster as a default option, we'd use it in this mode.

Jan 24 2019, 7:53 PM · Auth
aeiser added a comment to T13231: Duo MFA Support.

FWIW - most users of this feature (especially after T13229) will be organizations that already have DUO in their environment, and likely already have users defined in DUO. Creating random usernames, or PHID's is probably not going to work for most of those organizations. Phabricator does require email's to be unique (no reuse) so that might be a better choice for initial userid setting?

Jan 24 2019, 7:16 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

When you delete a Duo factor from your phone app, you still get prompted to "auth", Duo just reports that you have no enrolled devices which support push.

Jan 24 2019, 6:54 PM · Auth
epriestley added a comment to T6012: Why doesn't Phabricator (or Arcanist, or libphutil) support Composer?.

Not exactly related, but PEAR got compromised: https://news.ycombinator.com/item?id=18987518

Jan 24 2019, 4:54 PM · Guides
epriestley added a comment to T13237: Stripping MFA configs with `bin/auth strip` should disable, not delete, the factor configurations.

Striping should probably "revoke" (disable + can not be re-enabled), vs the user "remove" button becoming a "disable" action.

Jan 24 2019, 3:30 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

Duo wants us to <img src="..." /> a QR code.

Jan 24 2019, 2:59 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

With a 15-second TTL, the user doesn't vanish or anything, and Duo does not let us do another enroll attempt even after the enroll time expires:

Jan 24 2019, 2:21 PM · Auth
epriestley added a comment to T13231: Duo MFA Support.

If you try to /enroll the same user PHID twice, the second /enroll fails:

Jan 24 2019, 2:12 PM · Auth
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

Rolling this mess up again:

Jan 24 2019, 1:43 PM · Plans
epriestley moved T13237: Stripping MFA configs with `bin/auth strip` should disable, not delete, the factor configurations from Backlog to MFA on the Auth board.
Jan 24 2019, 1:39 PM · Auth

Jan 23 2019

epriestley closed T920: Provide SMS Support, a subtask of T7338: Build more status tools for monitoring Phacility cluster health, as Resolved.
Jan 23 2019, 11:07 PM · Almanac, Phacility
epriestley closed T920: Provide SMS Support as Resolved.

I think this is more or less at the end of the line in terms of actionable work that doesn't make more sense being scoped in elsewhere.

Jan 23 2019, 11:07 PM · Restricted Project, Herald
epriestley closed T920: Provide SMS Support, a subtask of T4398: Implement two-factor authentication, as Resolved.
Jan 23 2019, 11:07 PM · Auth
epriestley added a revision to T13231: Duo MFA Support: Unknown Object (Differential Revision).
Jan 23 2019, 9:33 PM · Auth
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

This will probably roll forward into a "next iteration", but it would be nice-to-have to show users the numbers we expect to text them from, so they can whitelist/anticipate the messages.

Jan 23 2019, 8:52 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 8:00 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 7:12 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 6:40 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

managing contact numbers

Jan 23 2019, 6:32 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

Also, managing contact numbers needs to require MFA or evading SMS MFA is trivial. 🐈

Jan 23 2019, 6:07 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 5:50 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

Currently, I think attackers can CSRF the MFA enroll endpoint (/mfa/new/?provider=xyz). This could send you an SMS, soon. This is mostly harmless but potentially a little annoying with push factors like SMS/Duo. I'll get a CSRF gate on it.

Jan 23 2019, 4:51 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 4:42 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 3:29 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 23 2019, 3:12 PM · Plans
epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 23 2019, 2:59 PM · Restricted Project, Herald
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

Also, I think we still have an issue with rate limiting where:

Jan 23 2019, 12:23 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

So I don't forget about it: the default settings/ UI is now incorrectly showing Conpherence settings, not date and time settings. Clicking anything works properly.

Jan 23 2019, 12:19 PM · Plans
kerberizer awarded T3569: Make Phrequent somewhat usable a Like token.
Jan 23 2019, 9:58 AM · Phrequent

Jan 21 2019

epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 21 2019, 11:17 PM · Restricted Project, Herald
epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 21 2019, 11:13 PM · Restricted Project, Herald
epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 21 2019, 10:32 PM · Restricted Project, Herald
epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 21 2019, 8:52 PM · Restricted Project, Herald
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Restricted Differential Revision.
Jan 21 2019, 8:10 PM · Plans
epriestley closed T13238: Disallow MYSQLI_OPT_LOCAL_INFILE as Resolved by committing Restricted Diffusion Commit.
Jan 21 2019, 6:33 PM · Infrastructure, Security
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

The need for more UI support on the "MFA Required" flow is making me consider expanding scope here into knocking "Settings" back out to a full-width UI. I think that's not terribly hard. I'm maybe going to start there so it can get more testing as I go through everything else, and I can give up if it's harder than I think.

Jan 21 2019, 5:30 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

From D19975, two considerations:

Jan 21 2019, 5:26 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

T13227 also needs to happen this week.

Jan 21 2019, 5:19 PM · Plans
epriestley triaged T12921: Link to referenced object in transaction emails as Low priority.
Jan 21 2019, 5:17 PM · Mail
epriestley triaged T13240: Improve policy exception behavior for Spaces and exception-suppressed queries as Low priority.
Jan 21 2019, 4:46 PM · Infrastructure
epriestley added a revision to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE: Unknown Object (Differential Revision).
Jan 21 2019, 3:38 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

Thanks! I get the same behavior locally, I filed this upstream: https://bugs.php.net/bug.php?id=77496

Jan 21 2019, 3:34 PM · Infrastructure, Security
vrana added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

I can't get MYSQLI_OPT_LOCAL_INFILE to work on secure, either. I tried on secure001 and secure004 (where the database is not local). As far as I can tell, this option doesn't do anything, anywhere, ever?

Jan 21 2019, 8:27 AM · Infrastructure, Security
epriestley added a comment to T13082: Sometimes, mail about attaching revisions to tasks doesn't render the revision title.

I believe D19969 has now fixed this, although I'm not entirely certain, since it was never reliably reproducible in production so there's no way to really test or verify it.

Jan 21 2019, 5:09 AM · Transactions, Mail
epriestley closed T13082: Sometimes, mail about attaching revisions to tasks doesn't render the revision title as Resolved by committing Restricted Diffusion Commit.
Jan 21 2019, 5:03 AM · Transactions, Mail
joshuaspence added a watcher for Plans: joshuaspence.
Jan 21 2019, 1:40 AM

Jan 20 2019

epriestley lowered the priority of T13238: Disallow MYSQLI_OPT_LOCAL_INFILE from Low to Wishlist.

We're probably done here, but ideally the next steps are:

Jan 20 2019, 3:14 PM · Infrastructure, Security
epriestley closed T13239: null pointer exception in PhabricatorPeopleWelcomeController as Resolved by committing Restricted Diffusion Commit.
Jan 20 2019, 3:00 PM
epriestley added a comment to T13239: null pointer exception in PhabricatorPeopleWelcomeController.

Reproduction steps are:

Jan 20 2019, 3:00 PM
epriestley added a revision to T13239: null pointer exception in PhabricatorPeopleWelcomeController: Unknown Object (Differential Revision).
Jan 20 2019, 2:57 PM

Jan 19 2019

avivey created T13239: null pointer exception in PhabricatorPeopleWelcomeController.
Jan 19 2019, 11:31 PM
epriestley reopened T12921: Link to referenced object in transaction emails as "Open".

D19968 got us closer here, but the link targets aren't actually rendering properly.

Jan 19 2019, 1:08 PM · Mail
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

The fix for T12921: Link to referenced object in transaction emails (in D19968) is a bit buggy -- the links aren't generating as absolute links with a protocol and domain name. This is likely an easy fix, although I don't expect to get to it tonight.

Jan 19 2019, 4:20 AM · Plans
epriestley edited the content of Changelog.
Jan 19 2019, 4:17 AM
epriestley created 2019 Week 3 (Mid January).
Jan 19 2019, 4:16 AM

Jan 18 2019

epriestley closed T13176: Differential now has a "/D123/new/" link as Resolved.

No one seemed to hate this. I'm not sure it's seeing too much use, but we could grep some access logs eventually.

Jan 18 2019, 9:05 PM · Differential
epriestley closed T13193: Dropping a column which is a component of a UNIQUE KEY has surprising behavior in MariaDB as Resolved.

Nothing really actionable here.

Jan 18 2019, 9:03 PM · Database
epriestley added a revision to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE: Unknown Object (Differential Revision).
Jan 18 2019, 5:40 PM · Infrastructure, Security
epriestley added a revision to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE: Unknown Object (Differential Revision).
Jan 18 2019, 5:37 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

Maybe another point in favor of this claim is that the option does not work is the behavior of this:

Jan 18 2019, 5:02 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

I think that maybe mysql_nonapi.c just overrides the conn->options() call? Near line 269 of PHP 7.2.3:

Jan 18 2019, 4:57 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

I can't get MYSQLI_OPT_LOCAL_INFILE to work on secure, either. I tried on secure001 and secure004 (where the database is not local). As far as I can tell, this option doesn't do anything, anywhere, ever? I'm going to look at the source and see if I can figure out what's going on, but I'll back it out of D19998 if I can't find some evidence that it's useful.

Jan 18 2019, 4:49 PM · Infrastructure, Security
epriestley added a revision to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE: Unknown Object (Differential Revision).
Jan 18 2019, 4:37 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

I'm unable to get the MySQLi option MYSQLI_OPT_LOCAL_INFILE to actually work. Here's the script I'm using:

Jan 18 2019, 4:15 PM · Infrastructure, Security
epriestley added a comment to T13238: Disallow MYSQLI_OPT_LOCAL_INFILE.

It looks like we don't need to do anything about mysql on the CLI since this option is, thankfully, not enabled by default:

Jan 18 2019, 4:04 PM · Infrastructure, Security
epriestley triaged T13238: Disallow MYSQLI_OPT_LOCAL_INFILE as Low priority.
Jan 18 2019, 4:01 PM · Infrastructure, Security
epriestley triaged T13237: Stripping MFA configs with `bin/auth strip` should disable, not delete, the factor configurations as Low priority.
Jan 18 2019, 3:50 PM · Auth
epriestley created T13237: Stripping MFA configs with `bin/auth strip` should disable, not delete, the factor configurations.
Jan 18 2019, 3:47 PM · Auth
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 18 2019, 2:02 PM · Plans
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 18 2019, 2:32 AM · Plans

Jan 17 2019

epriestley changed the visibility for D291: Allow email subject prefixes to be configured.
Jan 17 2019, 8:17 PM
epriestley added a comment to T11760: Phacility Cluster: Ad-Hoc Query Log.

I ran a query against a subset of instances to see if any instances had configured metamta.*.subject-prefix options.

Jan 17 2019, 8:00 PM · Phacility
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 17 2019, 7:43 PM · Plans
avivey changed the visibility for T1394: Support creating audits with inline comments through the API.
Jan 17 2019, 7:17 PM · Conduit, Harbormaster, Audit
epriestley added a comment to T13115: Handle mail bounces inside Phabricator.

When the time comes, this should almost certainly also handle contact numbers for non-email media (T920 / D19988).

Jan 17 2019, 3:55 PM · Mail
epriestley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 17 2019, 3:48 PM · Restricted Project, Herald
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 17 2019, 2:47 PM · Plans
• dolftax added a comment to T6461: Adding comments and inline comments via Conduit.

Hey guys,

Jan 17 2019, 8:26 AM · Conduit

Jan 16 2019

epriestley closed T8636: Update "Configuring Inbound Email" to describe application email configuration in greater detail as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 9:56 PM · Documentation, Mail
epriestley closed T13234: Application email addresses may shadow user email addresses as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 9:28 PM · Mail, Security
epriestley closed T11782: Inconsistent mail notification subject ([Updated] vs [Merged]) depending on from which task a duplicate task was merged as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 9:27 PM · Mail, Maniphest, Bug Report
epriestley closed T12921: Link to referenced object in transaction emails as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 9:21 PM · Mail
epriestley closed T7477: Handle inbound email with phabricator address in the CC as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 8:28 PM · llvm, Restricted Project, Restricted Project, Mail
epriestley closed T13066: Refine inbound mail error behaviors, distinguishing between actual mail sender and acting-as sender as Resolved by committing Restricted Diffusion Commit.
Jan 16 2019, 8:28 PM · Mail
epriestley added a comment to T13235: Replace "SimpleEmailService" with a subclass of "PhutilAWSFuture" that surfaces errors more clearly.

D19955 has how I think this likely breaks down, I'm not planning to touch it in this iteration since the badness, while bad, is mostly in a box (hopefully) until the next PHPMailer CVE. All yours!

Jan 16 2019, 7:35 PM · Mail
epriestley updated darcy.
Jan 16 2019, 7:33 PM
amckinley added a comment to T13235: Replace "SimpleEmailService" with a subclass of "PhutilAWSFuture" that surfaces errors more clearly.

When I was messing around with SMS sending last week, I put a rough cut of this together that is capable of sending an SMS using SNS, which has almost the same API as SES: D19982. I can tackle this unless you're already in the middle of it.

Jan 16 2019, 7:11 PM · Mail
amckinley added a revision to T920: Provide SMS Support: Unknown Object (Differential Revision).
Jan 16 2019, 7:09 PM · Restricted Project, Herald
amckinley added a revision to T13235: Replace "SimpleEmailService" with a subclass of "PhutilAWSFuture" that surfaces errors more clearly: Unknown Object (Differential Revision).
Jan 16 2019, 7:09 PM · Mail
epriestley added a revision to T13222: 2018 Week 48-51 Bonus Content: Unknown Object (Differential Revision).
Jan 16 2019, 5:14 PM · Plans
epriestley added a comment to T13222: 2018 Week 48-51 Bonus Content.

SMS MFA

Jan 16 2019, 3:51 PM · Plans
epriestley added a revision to T7607: Invoicing emails probably need some work: Unknown Object (Differential Revision).
Jan 16 2019, 3:20 PM · Phortune, Phacility