Topics-2013-08-22: auth-user-pass-discussion.txt

File auth-user-pass-discussion.txt, 20.3 KB (added by Samuli Seppänen, 8 years ago)

Discussion about auth-user-pass, stdin, username-only pass file, etc.

Line 
1(21.32.19) waldner: (sorry to switch topic) do people think this is a bug: if one runs with "auth-user-pass somefile.txt" + "static-challenge foo 1", user+pass are read from the file but the user is not prompted for the response to the challenge
2(21.38.58) waldner: same if running with management-query-passwords, the response to the challenge is never asked
3(22.19.44) vpnHelper: RSS Update - testtrac: Always load intermediate certificates from a PKCS#12 file <https://github.com/OpenVPN/openvpn/commit/6481f879eb62cafa6ad652801b2b5c45e546ef44> || Add a note what setenv opt does for OpenVPN < 2.3.3 <https://github.com/OpenVPN/openvpn/commit/39dad37d5b13c4dc0614ab7b19fdae88c23de0a2> || Add support to ignore specific options. <https://github.com/OpenVPN/openvpn/commit/b685a1e6b012682ce7d6fb31960273b8f5213714> |
4(22.34.30) pekster: waldner: Without looking at the code, maybe you need --management-query-passwords too?
5(22.39.38) pekster: On an unrelated note, who's in charge (has access) to the openvpn.net community documentation page? The initial links under "Manuals" link to 2.1 and 2.0.x, and then even the 'Manuals' link lists them in oldest-to-latest order. Can we get links to 2.3 on the main URL layout?
6(23.02.01) mattock: pekster: in practice I'm the guy who fixes stuff on openvpn.net
7(23.02.37) mattock: I can check if adding 2.3 to the manuals list would be doable, but those lists are generated from categories (e.g. "Manuals" category) in Joomla
8(23.03.21) mattock: and as 2.3 man-page is not directly on openvpn.net for other technical reasons (=pain trying to get it to format correctly), I might have to make the manuals list static
9(23.03.31) mattock: -> TODO
10(23.03.45) pekster: Ah, I see. Or perhaps just remove the Joomla stuff completely?
11(23.03.49) mattock: yeah
12(23.03.56) pekster: The manuals page has links to the relevant manuals, including the more-appropriate 2.3 listing
13(23.04.25) cron2: ecrist/krzee: vpnhelper is doing it again
14(23.04.35) cron2: this is old news
15(23.11.34) waldner: pekster: as I said, using --management-query-password foes the same (buggy imho) thing
16(23.12.01) waldner: -1s/fo/do/
17(23.12.30) pekster: Oh, missed that part
18(23.12.34) pekster: Does it prompt for the input on stdin then?
19(23.12.43) waldner: neither
20(23.13.30) waldner: in the code, I see that it prompts for the challenge/respone (either on stdin or the management) only if auth-user-pass comes from stdin and not a file
21(23.13.50) waldner: perhaps I didn't make myself clear
22(23.13.59) pekster: Oh, this is on your client?
23(23.14.15) waldner: the behavior definitely *is* what I described, since I'm looking at the code
24(23.14.47) pekster: You won't ever use both --user-auth-pass with --static-challange . The first would be used on a client, the 2nd on a server
25(23.14.59) waldner: I was asking for opinions whether it's a bug (I think it is)
26(23.15.01) pekster: The client isn't challanging the server; the server is challenging the client
27(23.15.27) waldner: pekster: that's what I thought too before getting mad looking at the code
28(23.15.30) pekster: I don't think it's a bug at all; what possible reason would the client have for needing a challanage response from the server that the TLS checks don't already offer?
29(23.15.44) waldner: my tests indicate that static-challenge on the server does absolutely nothing at all.
30(23.15.54) waldner: I'd be glad to be proved wrong, though
31(23.16.30) pekster: Did you set up a relationship as described in the management-notes.txt documentation?
32(23.16.44) pekster: usecases are more helpful then "I've stared at the code and I think its buggy"
33(23.17.02) waldner: I think I've read that page a few tens times now
34(23.17.13) waldner: what do you mean by "relationship"?
35(23.17.23) waldner: (and yes of course I have a lab where I'm testing)
36(23.17.41) pekster: More useful is "a setup like this [reference to config of client & server, & associated auth scripts used] doesn't work, I expected X to happen, but Y happens." For bonus points, point to code sections/functions/whatever where you think the problem is, but that's not required for a "user-level" bugreport
37(23.18.17) waldner: sure, I can paste all the configs
38(23.19.41) waldner: (and I think static-challenge is for a client also because the function that prompts for it is the same that prompts for auth-user-pass)
39(23.20.12) pekster: No, that's described very clearly in the protocol docs
40(23.20.22) pekster: The server is the one sending the challange to the client
41(23.20.56) pekster: Oh, hmm, I guess that's wrong too :)
42(23.22.53) waldner: I don't find it clear at all
43(23.22.54) pekster: Yea, that --static-challange appears to only send a static reply back to the server; this will, AFAIK, only happen if the server sends a CRV1 request after initial authentication
44(23.23.03) waldner: what I understand is the result of my tests
45(23.23.31) waldner: no, that --static-challenge is what prompts the *client* *locally* for the response to the static challenge
46(23.23.54) waldner: then what the client enters is sent to the server, which can verify it via script or whatever
47(23.24.34) waldner: at least that's what I've seen in the tests, as I said it's clear as mud to me and more info is always welcome
48(23.24.47) pekster: Yea, I mistook the static for the dynamic bit :\
49(23.25.02) pekster: (ie: wondering how your client was "initiating" the challange)
50(23.25.36) pekster: So, yes, the bit about the 'Static Protocol' does indeed suggest the management interface will "respond as such when credentials are needed" ... but apparently that's not happening?
51(23.26.26) pekster: Ah, okay, so the reply to the prompt contains *both*
52(23.26.56) waldner: configs: http://pastebin.com/raw.php?i=M1ikVdAe
53(23.26.58) pekster: It's a single reply taht muxes them both together in base64
54(23.27.04) waldner: yes
55(23.28.04) pekster: You can't possibly be prompted for "one but not the other" in that format. Or do you mean the management prompt never gives you this bit: `PASSWORD:Need 'Auth' username/password SC:<ECHO>,<TEXT>
56(23.28.26) waldner: here are the two cases:
57(23.29.02) waldner: if you have auth-user-pass set to read from stdin, and static-challenge blah, then you are prompted on stdin for username, password, and response
58(23.29.10) waldner: correct, afaict
59(23.29.30) waldner: if, in addition, you have management-query-passwords, you are prompted for the same info in the management interface
60(23.29.37) waldner: again correct, I think
61(23.30.11) waldner: in the management, you have to enter the password + the challenge reply together and encoded, as explained in the doc
62(23.30.20) waldner: but that's ok, as it's usually done by an application
63(23.30.40) waldner: we're still talking about three pieces of information: user, pass, response
64(23.31.02) pekster: Oh, now I think I see
65(23.31.04) waldner: now there's an option to read username and password (but not response) from a file
66(23.31.18) waldner: and it's done with auth-user-pass somefile.txt
67(23.31.27) pekster: The management method is expecting "all bits" to be supplied directly into the management interface
68(23.31.37) waldner: if I do that, then I'd expect openvpn to take user/pass from the file, but still prompt for the challenge
69(23.31.40) pekster: Not de-coupling the user/pass from the response
70(23.31.43) waldner: pekster: yes
71(23.32.08) waldner: (it took me a while to figure out though)
72(23.32.10) pekster: Less of a "bug" and more of a "possibly useful feature" IMO
73(23.32.21) pekster: What's stopping your application from reading the external file in this case?
74(23.32.32) waldner: it's openvpn that reads the file
75(23.32.43) waldner: to avoid prompting the user for user/pass
76(23.32.44) pekster: That feels like a cleaner solution, but perhaps openvpn could gracefully error out or handle the situation
77(23.33.01) pekster: Yes. BUt what stops your application using the --management interface from doing it when you are using the management interface?
78(23.33.11) cron2: *sigh*
79(23.33.19) waldner: perhaps I'm not being clear
80(23.33.24) cron2: built a nice and shiny openvpn.exe from the wrong git branch
81(23.33.36) pekster: No, I think you are. Why does it matter "what' high-level program reads the user file?
82(23.34.01) waldner: it's not *what* program, that file is read *exclusively* by openvpn
83(23.34.12) pekster: Not if you do fopen($file) from your own app
84(23.34.22) waldner: sure, but why should I?
85(23.34.34) pekster: Becuase code in openvpn doesn't support it ;)
86(23.34.36) waldner: I'm using it as written on the tin, I don't want my app to read it
87(23.34.49) waldner: my app only would interact with the management
88(23.35.45) waldner: I'm moving from a situation where the user (or the app interacting with the management) supplies all three pieces of information (user, pass, response)...
89(23.35.53) pekster: And really, storing passwords in a plaintext file and *then* wanting to "improve security" with --response is a bit silly. Two steps backwards, one step forwards in terms of security
90(23.36.07) waldner: ...to a situation where I want openvpn to read two of them from a file, which it supports
91(23.36.18) waldner: pekster: that's not the point
92(23.36.28) pekster: But yes, a patch could in theory fix that, and if you'd like to hack at it, a contribution to the ML would likely yield some discussion on committing it
93(23.36.42) waldner: so you agree it's a bug?
94(23.36.43) pekster: I just think it's a slightly silly thing to want
95(23.36.52) waldner: perhaps yes
96(23.36.56) pekster: <@pekster> Less of a "bug" and more of a "possibly useful feature" IMO
97(23.37.21) pekster: I can't think of any sane reason to want that, from a security point of view
98(23.37.56) waldner: from a security point of view, then you also don't want to store a password in the file, but as I said it's not the point
99(23.39.07) ***pekster shrugs. Feel free to open a bugreport if you want, but my gut feeling is that there won't be a lot of interest in fixing it unless you plan to do much of the work in authoring a fix
100(23.39.23) waldner: and I'm in fact working on a patch that would allow to only put the username in the file and being prompted for the password
101(23.39.36) pekster: Maybe someone has more interest than I do, but it's not a combination of features I think I'm ever likely to want or promote the use of
102(23.39.49) waldner: indeed, see above
103(23.40.15) waldner: my goal is to allow supplying only the username from file, and still be prompted for password
104(23.40.42) waldner: but before I am able to do that, I need to understabd how it works *now*
105(23.40.55) waldner: hence the tests
106(23.41.47) waldner: as it stands now it's all or nothing: you either supply user *and* pass both from a file, or both interactively
107(23.42.09) pekster: Fixing that was discussed recently and thought of as a generally good idea
108(23.42.18) waldner: which is a bit annoying because the username is always the same
109(23.42.22) waldner: yes
110(23.42.45) waldner: it was agreed to do it via inlining of the file that would be otherwise supplied as argument to auth-user-pass
111(23.42.55) pekster: Allowing only the user to be specified via `--user-auth-pass user $external-file` syntax -- a patch for this feature would need to continue prompting on the mgmt interface if the password is to be queried there
112(23.43.07) pekster: Yes, inline or not doesn't matter
113(23.43.18) waldner: right
114(23.43.33) waldner: well,inlining would be an extra feature
115(23.43.41) pekster: But yes, a patch for this will, I expect, need to keep the *current* documented behaviour of "everything that's not supplied comes in via the --management interface"
116(23.43.53) waldner: which it doesn't
117(23.44.01) waldner: as I said at the very beginning
118(23.44.09) waldner: hence I think it's a bug
119(23.44.18) pekster: The patch for this feature hasn't been authored yet
120(23.44.25) waldner: no
121(23.44.34) pekster: You want username via file, and pass+challenge response over --managemnt, right?
122(23.44.41) pekster: That will be supported fine if this patch is authored properly
123(23.44.42) waldner: or stdin
124(23.45.00) pekster: or stdin, if --managemnet-query-passwords is not supplied, right
125(23.45.07) pekster: It simply won't ask for the 'Auth' username
126(23.45.10) pekster: Only the 'Auth' password
127(23.45.11) waldner: or user+pass via file, and response via stdin or management <--- this isn't happening right now and I think it should
128(23.45.50) pekster: That's not how the Static protocl is currently documented to work, no.
129(23.46.04) pekster: Again, author a patch if you want it changed, or open a bugreport and hope someone finds it more useful than I do
130(23.46.26) waldner: sure, as soon as I understand as it works :)
131(23.46.36) waldner: *how
132(23.46.42) waldner: or how it's supposed to
133(23.46.43) pekster: I wouldn't call this a bug since it's not documented to work like that in the first place
134(23.47.06) pekster: As it stands today, the static challenge protocol is inherently designed to mux together the password and response
135(23.47.09) waldner: it's not documented at all, but reading the (scarce) docs that's how I'd expect it to work
136(23.47.25) pekster: Maybe you don't like it, but the code is doing exactly what I read in the 'Static protocol:' part of the management notes
137(23.47.29) waldner: only if sent via management
138(23.47.46) waldner: if promted on stdin, it prompts three times
139(23.48.38) waldner: and only after the user has entered all three pieces of information, it creates the SCRV1: string and sends it to the server
140(23.48.47) pekster: Right. I think the deal is this feature was never intended to be used outside of the managemnet interface strictly
141(23.49.01) waldner: then the bug is that it allows to
142(23.49.18) pekster: Use stdin? I just don't know of anyone using it like that
143(23.49.58) waldner: I use it for username+password and find it convenient
144(23.50.17) pekster: I meant for the challenge/response stuff
145(23.50.56) waldner: well, if you're already entering user+pass, I don't see why you'd wanted to be prompted for the challenge somewhere else
146(23.51.48) pekster: Yes, my thoughts exactly. You're arguing really hard that it should so that
147(23.51.55) pekster: do*
148(23.52.15) waldner: no
149(23.52.26) pekster: You said what is broken is user+pass via file and static response via managment is broken. This is via "somewhere else"
150(23.52.47) waldner: I'm arguing that if you supply username or username + password via a file, then it should still prompt for the missing piece of information, and it doesn't
151(23.53.21) waldner: neither on stdin nor in the management
152(23.53.42) pekster: Yup. I get it. I just don't think it's useful. If what you really want is username from file and pass+response via any one place (stdin _OR_ management) then this will be supported when the not-yet-designed inline user-auth-pass support is written
153(23.54.16) waldner: yes, so I'm going to add it (hopefully)
154(23.54.58) waldner: it's not that it's useful or not, it's that if you are not prompted it's impossible to authenticate to the server
155(23.55.22) waldner: so you are forced to go back to interactively entering stuff
156(23.57.27) waldner: perhaps it's best discussed during a meeting
157(23.58.35) pekster: Sure, again, someone else might see this as a real benefit to a specific workflow that I'm not connecting with. I'm not against the feature, I'm just  not for it (a "+0" as it were)
158(23.59.46) waldner: my ultimate goal really is to automate supplying the username (and only that)
159(23.59.48) pekster: My logic: if you care enough about security to be using challenge-response, you have no business storing the password in a file." And remember, support for user -> file, pass+response -> (stdin || management) was already agreed on by a few folks here as useful. I see that
160(23.59.57) pekster: I already told you that was previously discussed and basically blessed
161(00.00.13) pekster: So if that's *all* you want, "yes, it was discussed and thought of as a good thing."
162(00.00.30) pekster: Can we stop re-hashing that bit now?
163(00.01.14) waldner: I think that to get there, the issue I described already too many times will have to be fixed as well
164(00.02.24) pekster: Um, huh? You want user in a file, so you "also" have to add support at doesn't to just that? Strange logic
165(00.03.00) waldner: user and password in a file is *already* supported
166(00.03.26) waldner: and there's nothing to prevent people doing that, and using a static challenge
167(00.03.35) pekster: Right. user-only won't change where the pass+response is entered if you author your patch properly
168(00.03.53) waldner: so we either prevent people doing that (which is fine by me), or if we allow it, it has to work correctly. Do we agree on this?
169(00.07.09) pekster: They're 2 separate issues. Issue 1 is supporting inline files and including support for user-name only via file. Issue 2 is using a user-file (inline or not) with Challenge/Response. I like the idea of fixing #1. I don't care about #2, but don't think it's a bad idea (just not useful)
170(00.07.19) pekster: Doing #1 "right" does not have any relationship with #2
171(00.07.30) pekster: You want both, but the features are not dependent on each other
172(00.07.54) waldner: #3 supplying username-only via file
173(00.08.02) pekster: I already included that in #1
174(00.08.08) waldner: which critically changes the way code works currently
175(00.08.27) pekster: I guess it really is a "third" independent issue :)
176(00.08.45) pekster: Yea, it's assumed right now that both come together, no doubt
177(00.08.59) waldner: from a functional point of view perhaps, but in the code it's all almos inextricably intertwined
178(00.11.52) waldner: the code as it stands assumes all-or-nothing, that is, that either both things are supplied via file or both via stdin, which means that things are done in a certain order based on that assumption, which will no longer be true
179(00.12.07) ender^ ha abbandonato la stanza (quit: Ping timeout: 246 seconds).
180(00.13.21) pekster: Right. The structure holding the user (when reading user but prompting for pass) will have defined one but not the other. Then code where both would normally be prompted need to support omitting the user but prompt for the pass still and add that data to the structure
181(00.14.15) waldner: yes, that too, there's a .defined member which will probably need to change
182(00.15.16) waldner: and to top it up, that function is also a bit overloaded with a variety of flags and invoked from other places (for example, to prompt for private key password)
183(00.16.04) pekster: Yea, in some places the code is complex, in other brilliant, and if you're unlucky, both at once ;)
184(00.16.26) waldner: that's probably it :)
185(00.17.13) pekster: Lots of functions end up being used from 'multiple places' under the original goal of re-using code and not re-writing shared parts. In some cases (many series of patches later) some of it is probably "too complex" now, but fixing it would take more time than "getting the next patch/support" added. So it continues.
186(00.17.29) waldner: how true
187(00.17.40) waldner: it's not a openvpn-only thing, of course
188(00.19.36) pekster: Anyway, good luck. tl;dr here is "yes, you probably want all 3 features we noted", but remember that they don't (directly) depend on one-another. In fact, it might make everything cleaner in the code to add support one-at-a-time if the changes end up impact a lot of places
189(00.20.06) pekster: Depends on what needs to change, and I only know some of it (you've clearly spent more time checking that out so far)
190(00.20.53) pekster: It's easier to review a 3-part-patchset than try to figure out "what feature this block of code change is supposed to fix"
191(00.21.43) waldner: agreed on that, I'll try to structure it that way if at all possible
192(00.22.52) waldner: I'd still like to discuss corner cases like the one I found during a meeting, though
193(00.23.33) waldner: so at least if something need to be fixed there's consensus on how the fix should behave
194(00.25.06) ender^ [~ender1@2a01:260:4094:1:42:42:42:42] è entrato nella stanza.
195(00.40.00) pekster: Sure, but they're all 3 separate issues. By all means let's bring them up
196(00.40.52) pekster: Your "corner case" is a when 1 or 2 of the 3 are implemented but not all 3
197(00.41.31) waldner: it's not the only one
198(00.41.46) waldner: but I won't annoy you with the others
199(00.45.01) pekster: matting (if you're still semi-coordinating the meeting announcements): No urgency, but tl;dr here is that the inline --auth-user-pass stuff has some tertiary bits involved that we might want to add to the next scheduled meeting. Integration with support for Challenge/Response and handling user but no pass are among some of the muddier points