Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name-based virtual hosts not working as expected after upgrade from 1.3.7a to 1.3.7b #1369

Closed
m0vie opened this issue Dec 29, 2021 · 30 comments
Assignees
Milestone

Comments

@m0vie
Copy link

m0vie commented Dec 29, 2021

What I Did

Upgraded from 1.3.7a to 1.3.7b.

What I Expected/Wanted

A simple config (containing 2 VirtualHosts for the same IP / port) that worked fine on 1.3.7a does not anymore on 1.3.7b. Example config see below.

Before 8c3ad20:

$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com
CONNECTED(00000003)
...
$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername second.domain.com
CONNECTED(00000003)
...

After 8c3ad20:

$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com
CONNECTED(00000003)
140315084990272:error:14094458:SSL routines:ssl3_read_bytes:tlsv1 unrecognized name:../ssl/record/rec_layer_s3.c:1543:SSL alert number 112
...
$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername second.domain.com
CONNECTED(00000003)
...

proftpd.log:

localhost (localhost[127.0.0.1]): mod_tls/2.9: no matching server found for client-sent SNI 'first.domain.com', rejecting SSL/TLS connection

A git-bisect shows that this is a side-effect of 8c3ad20 (#1105 / #1109).

ProFTPD Version and Configuration

  Version: 1.3.7b (git)
  Platform: LINUX [Linux 5.13.0-19-generic x86_64]
  Built: Thu Dec 30 2021 00:14:05 CET
  Built With:
    configure  '--disable-option-checking' '--disable-silent-rules' '--disable-dependency-tracking' '--with-pkgconfig=lib/pkgconfig' '--with-includes=/usr/include/postgresql:/usr/include/mysql' '--sysconfdir=/etc/proftpd' '--localstatedir=/run' '--enable-sendfile' '--enable-facl' '--enable-dso' '--enable-autoshadow' '--enable-ctrls' '--enable-openssl' '--enable-ipv6' '--enable-nls' '--with-lastlog=/var/log/lastlog' '--enable-pcre' '--disable-strip' '--with-shared=mod_tls:mod_rewrite' 'build_alias=x86_64-linux-gnu'

  CFLAGS: -g2 -O2 -Wall -fno-omit-frame-pointer -fno-strict-aliasing -Werror=implicit-function-declaration
  LDFLAGS: -L$(top_srcdir)/lib -L$(top_builddir)/lib  -rdynamic
  LIBS:  -lpcreposix -lpcre -lssl -lcrypto -lcap  -lpam -lsupp -lnsl -lresolv -lresolv -lcrypt  -pthread

  Files:
    Configuration File:
      /etc/proftpd/proftpd.conf
    Pid File:
      /run/proftpd.pid
    Scoreboard File:
      /run/proftpd.scoreboard
    Header Directory:
      /usr/local/include/proftpd
    Shared Module Directory:
      /usr/local/libexec

  Info:
    + Max supported UID: 4294967295
    + Max supported GID: 4294967295

  Features:
    + Autoshadow support
    + Controls support
    + curses support
    - Developer support
    + DSO support
    + IPv6 support
    + Largefile support
    + Lastlog support
    - Memcache support
    + ncursesw support
    + NLS support
    + OpenSSL support (OpenSSL 1.1.1l  24 Aug 2021)
    + PCRE support
    + POSIX ACL support
    - Redis support
    + Sendfile support
    + Shadow file support
    - Sodium support
    + Trace support
    + xattr support

  Tunable Options:
    PR_TUNABLE_BUFFER_SIZE = 1024
    PR_TUNABLE_DEFAULT_RCVBUFSZ = 8192
    PR_TUNABLE_DEFAULT_SNDBUFSZ = 8192
    PR_TUNABLE_ENV_MAX = 2048
    PR_TUNABLE_GLOBBING_MAX_MATCHES = 100000
    PR_TUNABLE_GLOBBING_MAX_RECURSION = 8
    PR_TUNABLE_HASH_TABLE_SIZE = 40
    PR_TUNABLE_LOGIN_MAX = 256
    PR_TUNABLE_NEW_POOL_SIZE = 512
    PR_TUNABLE_PATH_MAX = 4096
    PR_TUNABLE_SCOREBOARD_BUFFER_SIZE = 80
    PR_TUNABLE_SCOREBOARD_SCRUB_TIMER = 30
    PR_TUNABLE_SELECT_TIMEOUT = 30
    PR_TUNABLE_TIMEOUTIDENT = 10
    PR_TUNABLE_TIMEOUTIDLE = 600
    PR_TUNABLE_TIMEOUTLINGER = 10
    PR_TUNABLE_TIMEOUTLOGIN = 300
    PR_TUNABLE_TIMEOUTNOXFER = 300
    PR_TUNABLE_TIMEOUTSTALLED = 3600
    PR_TUNABLE_XFER_SCOREBOARD_UPDATES = 10

LoadModule mod_tls.c

ServerType standalone
SystemLog /var/log/proftpd/proftpd.log
Port 0
SocketBindTight on

<Global>
	DeferWelcome on
	DefaultRoot ~
	RequireValidShell off
	AuthOrder mod_auth_file.c
	User proftpd
	Group nogroup
	TransferLog /var/log/proftpd/xferlog
	AllowOverwrite on
	Umask 002
	ShowSymlinks off
	ListOptions "-l"
	DenyFilter \*.*/

	TLSEngine on
	TLSRequired on
	TLSProtocol TLSv1.2 TLSv1.3
	TLSCipherSuite ALL:!COMPLEMENTOFDEFAULT:!eNULL:!SSLv3:!TLSv1:!TLSv1.1:!MEDIUM:!LOW:!PSK:!RSA@STRENGTH
	TLSOptions NoSessionReuseRequired
	TLSSessionTickets on
	TLSRenegotiate required off

	<Directory />
		HideFiles ^\.ht.*
		<Limit ALL>
			IgnoreHidden On
		</Limit>
	</Directory>
</Global>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias first.domain.com
	TLSRSACertificateFile /etc/certs/first.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/first.domain.com.key
	TLSCACertificateFile /etc/certs/first.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/first.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/first.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias second.domain.com
	TLSRSACertificateFile /etc/certs/second.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/second.domain.com.key
	TLSCACertificateFile /etc/certs/first.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/first.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/first.domain.com.group
</VirtualHost>

@m0vie
Copy link
Author

m0vie commented Dec 30, 2021

binding trace:

2021-12-30 01:01:14,517 [806901] <binding:4>: bound address 127.0.0.1, port 21 to socket fd 0
2021-12-30 01:01:14,517 [806901] <binding:29>: hashed address '127.0.0.1' to index 126
2021-12-30 01:01:14,517 [806901] <binding:8>: created ipbind 0x556866a74b50 for 127.0.0.1#21, server 0x556866a63768
2021-12-30 01:01:14,517 [806901] <binding:8>: created namebind 'first.domain.com' for 127.0.0.1#21, server 0x556866a63768
2021-12-30 01:01:14,517 [806901] <binding:17>: found ipbind 0x556866a74b50 (server 0x556866a63768) for 127.0.0.1#21
2021-12-30 01:01:14,517 [806901] <binding:17>: ipbind 0x556866a74b50 (server 0x556866a63768) for 127.0.0.1#21 has namebinds (1)
2021-12-30 01:01:14,517 [806901] <binding:17>: namebind #0: first.domain.com (inactive)
2021-12-30 01:01:14,517 [806901] <binding:29>: hashed address '127.0.0.1' to index 126
2021-12-30 01:01:14,517 [806901] <binding:8>: created ipbind 0x556866a76d10 for 127.0.0.1#21, server 0x556866a67028
2021-12-30 01:01:14,517 [806901] <binding:19>: found existing ipbind 0x556866a74b50 (server 0x556866a63768) at index 126 in iptable table, adding to 0x556866a76d10
2021-12-30 01:01:14,517 [806901] <binding:8>: created namebind 'second.domain.com' for 127.0.0.1#21, server 0x556866a67028
2021-12-30 01:01:14,517 [806901] <binding:17>: found ipbind 0x556866a76d10 (server 0x556866a67028) for 127.0.0.1#21
2021-12-30 01:01:14,517 [806901] <binding:17>: ipbind 0x556866a76d10 (server 0x556866a67028) for 127.0.0.1#21 has namebinds (1)
2021-12-30 01:01:14,517 [806901] <binding:17>: namebind #0: second.domain.com (inactive)
2021-12-30 01:01:14,517 [806901] <binding:25>: displaying ipbind table:
2021-12-30 01:01:14,517 [806901] <binding:25>:   index 126:
2021-12-30 01:01:14,517 [806901] <binding:25>:     ipbind 0x556866a76d10:
2021-12-30 01:01:14,517 [806901] <binding:25>:       address: 127.0.0.1#21
2021-12-30 01:01:14,517 [806901] <binding:25>:       server: 0x556866a67028
2021-12-30 01:01:14,517 [806901] <binding:25>:       namebinds:
2021-12-30 01:01:14,517 [806901] <binding:25>:       #1: 0x556866a76d48
2021-12-30 01:01:14,517 [806901] <binding:25>:         name: second.domain.com
2021-12-30 01:01:14,517 [806901] <binding:25>:         server: 0x556866a67028
2021-12-30 01:01:14,517 [806901] <binding:25>:     ipbind 0x556866a74b50:
2021-12-30 01:01:14,517 [806901] <binding:25>:       address: 127.0.0.1#21
2021-12-30 01:01:14,517 [806901] <binding:25>:       server: 0x556866a63768
2021-12-30 01:01:14,517 [806901] <binding:25>:       namebinds:
2021-12-30 01:01:14,517 [806901] <binding:25>:       #1: 0x556866a74b88
2021-12-30 01:01:14,517 [806901] <binding:25>:         name: first.domain.com
2021-12-30 01:01:14,517 [806901] <binding:25>:         server: 0x556866a63768

Shouldn't there only be 1 ipbind with 2 namebinds below it?

@Castaglia Castaglia self-assigned this Jan 8, 2022
@Castaglia
Copy link
Member

Indeed there should; this particular area is still in a bit of flux. It's possible that this issue of yours was addressed by 2526301 -- I'll try to reproduce it locally.

@m0vie
Copy link
Author

m0vie commented Jan 22, 2022

I just tried using the latest latest git (915624a) version, but there are still 2 ipbinds and the issue persists.

@Castaglia
Copy link
Member

I was not able to reproduce this behavior using the latest ProFTPD from master, which indicated that it was fixed at some point the past; I was able to reproduce this bug using 1.3.7c, so the fix was fairly recently.

Some digging turned up this commit; I verified that it does indeed address the issue. I'll be backporting this commit to the 1.3.7 branch.

Castaglia added a commit that referenced this issue Jan 30, 2022
…nch, so that namebased <VirtualHost> blocks work with TLS SNI properly once more.
@Castaglia
Copy link
Member

@m0vie when you have time, could you verify that #1388 (for the 1.3.7 branch, not master) addresses the issue for you?

@m0vie
Copy link
Author

m0vie commented Jan 31, 2022

Hmm, this concrete problem with two exact ServerName entries seems to be solved.

Although I still see 2 ipbinds on latest master.

However, there is another problem with a wildcard ServerName. E.g. ServerName *:

Click to expand
LoadModule mod_tls.c

ServerType standalone
SystemLog /var/log/proftpd/proftpd.log
Port 0
SocketBindTight on


<Global>
	DeferWelcome on
	DefaultRoot ~
	RequireValidShell off
	AuthOrder mod_auth_file.c
	User proftpd
	Group nogroup
	TransferLog /var/log/proftpd/xferlog
	AllowOverwrite on
	Umask 002
	ShowSymlinks off
	ListOptions "-l"
	DenyFilter \*.*/

	TLSEngine on
	TLSRequired on
	TLSProtocol TLSv1.2 TLSv1.3
	TLSCipherSuite ALL:!COMPLEMENTOFDEFAULT:!eNULL:!SSLv3:!TLSv1:!TLSv1.1:!MEDIUM:!LOW:!PSK:!RSA@STRENGTH
	TLSOptions NoSessionReuseRequired
	TLSSessionTickets on
	TLSRenegotiate required off

	<Directory />
		HideFiles ^\.ht.*
		<Limit ALL>
			IgnoreHidden On
		</Limit>
	</Directory>
</Global>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias first.domain.com
	TLSRSACertificateFile /etc/certs/first.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/first.domain.com.key
	TLSCACertificateFile /etc/certs/first.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/first.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/first.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias *
	TLSRSACertificateFile /etc/certs/second.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/second.domain.com.key
	TLSCACertificateFile /etc/certs/second.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/second.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/second.domain.com.group
</VirtualHost>

I'd expect this to behave as a catch-all fallback. That means:

  • The first one (first.domain.com) should only be used when that exact SNI is sent by the client.
  • The second one (*) should be used when any other SNI or no SNI at all is sent by the client.

However, this is not the case.
a) openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com
returns the cert for the * fallback

b) If i swap the order of the two blocks in the config file, a request with no SNI at all returns the one for servername first.domain.com.

In 1.3.7a these scenarios worked as expected.

@Castaglia
Copy link
Member

Good feedback, much appreciated.

In my opinion, a ServerAlias * configuration would not work as a catch-all; that it might have done so in 1.3.7a was certainly not intentional or by design. Either the SNI sent by a client matches a ServerAlias in a <VirtualHost> section, or it does not. The "catch-all" is the <VirtualHost> initially selected for the TCP connection -- based on destination IP address/port, and perhaps the DefaultServer on directive.

This is the behavior that I would expect, and the behavior that I'll implement.

@Castaglia
Copy link
Member

Ah, right -- this is why a configuration of ServerAlias * matches everything. Hmm. It works, but not quite what I had in mind. I guess I'll need to document this better; the existing docs show an example of a glob match, but don't go into details about.

But you're right, a configuration of ServerAlias * would work as a fallback; it's then a question of ordering/matching. Do you expect:

  • first match wins, or
  • most specific match wins?

@m0vie
Copy link
Author

m0vie commented Jan 31, 2022

But you're right, a configuration of ServerAlias * would work as a fallback; it's then a question of ordering/matching. Do you expect:

  • first match wins, or
  • most specific match wins?

Hmm, good question, most specific would need to be clearly defined. E.g. what if there are two blocks

  • VirtualHost: *.com
  • VirtualHost: ftp.*
    ?

I guess fist match wins (in the order defined in the config file) would be clear and it would allow exactly that catch-all scenario.

  • VirtualHost: domain1.com domain1.org
  • VirtualHost: domain2.com domain2.org
  • VirtualHost: *.com
  • VirtualHost: *.org
  • VirtualHost: *

So a catch-all would be easy to implement and it would probably be compatible with what 1.3.7a has been doing (even if it was not intentional there).

And I'd expect that a request without SNI (e.g. using only the IP) would then also be handled by the last one.

Castaglia added a commit that referenced this issue Feb 13, 2022
…e ProFTPD internals, to maintain proper insertion order.
Castaglia added a commit that referenced this issue Feb 13, 2022
…oFTPD internals from the main branch to 1.3.7.
@Castaglia
Copy link
Member

@m0vie I worked on this a bit today; I think #1394 (backporting my current fixes to the 1.3.7 branch) should restore the expected behavior. Care to try it out?

Castaglia added a commit that referenced this issue Feb 13, 2022
…oFTPD internals from the main branch to 1.3.7.
@Castaglia
Copy link
Member

And I'd expect that a request without SNI (e.g. using only the IP) would then also be handled by the last one (ServerAlias *).

This expectation is a bit harder, since that particular functionality is supposed to be handled by the DefaultServer directive.

@m0vie
Copy link
Author

m0vie commented Feb 13, 2022

Things work well if there are only vhosts with explicitly defined ServerName directives.

However, I still am not able to make create a catch-all fallback that catches connections without SNI (just with the IP address).

If I define a 3rd vhost like this:

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias first.domain.com
	TLSRSACertificateFile /etc/certs/first.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/first.domain.com.key
	TLSCACertificateFile /etc/certs/first.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/first.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/first.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias second.domain.com
	TLSRSACertificateFile /etc/certs/second.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/second.domain.com.key
	TLSCACertificateFile /etc/certs/second.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/second.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/second.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	DefaultServer on
	TLSRSACertificateFile /etc/certs/dummy.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/dummy.domain.com.key
	TLSCACertificateFile /etc/certs/dummy.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/dummy.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/dummy.domain.com.group
</VirtualHost>

If I connect with an arbitrary SNI:

openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername adshfgakshdfjkasdhfkjsad

I end up on the 3rd vhost as intended.

But If I connect without SNI:

openssl s_client -connect 127.0.0.1:21 -starttls ftp

I still end up on the 1st one. This is not desired.

Also:
The DefaultServer has a major disadvantage: It can only be specified a single time globally.
But what If I have 2 disjoint IPs?

<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias first.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias second.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	DefaultServer on <-- First DefaultServer
</VirtualHost>

<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias first.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias second.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	DefaultServer on <-- Catch-all for IP 222.222.222.222. But this is not allowed
</VirtualHost>

@Castaglia
Copy link
Member

Castaglia commented Feb 13, 2022

Currently, the implementation is this:

  • Find the first ipbind that matches the IP address/port to which the client connections, regardless of ServerAlias or not (as clients may not specify a name at all) -- this is why you end up with the first vhosts in your example
  • Iff the client uses TLS SNI, or FTP HOST command (or both), then we search for the namebinds (and possibly do pattern matching)

If your configuration doesn't have DefaultServer, and doesn't have a ServerAlias *, which vhost would you expect to be used? In the first-match-wins implementation, it's the first vhost in the config file that matches the IP address/port to which the client connections -- and that's what we currently have.

@m0vie
Copy link
Author

m0vie commented Feb 13, 2022

If your configuration doesn't have DefaultServer, and doesn't have a ServerAlias *

I don't know, I guess that would be undefined.

The point is: If I do have them configured I definitely don't what a request without SNI to end up on any of the other ones.

@Castaglia
Copy link
Member

Castaglia commented Feb 15, 2022

While it might be ugly, based on the first-match-wins implementation, here's what you would currently need to use:

<VirtualHost 111.111.111.111>
	Port 21
       # No ServerAlias here; this will be the first vhost found for any connection that doesn't use `HOST`, TLS SNI, etc.
</VirtualHost>
<VirtualHost 111.111.111.111>
       Port 21
	ServerAlias first.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias second.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias *
</VirtualHost>

<VirtualHost 222.222.222.222>
      Port 21
      # No ServerAlias here; this will be the first vhost found for any connection that doesn't use `HOST`, TLS SNI, etc.
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias first.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias second.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias *
</VirtualHost>

An alternative might to use something like mod_autohost.

@Castaglia
Copy link
Member

While we continue discussing how best to deal with the question of which <VirtualHost> configuration to use for the initial TCP connection, do you have any objections if I merge the existing 2 PRs, which at least make the master (and the 1.3.7 branch) handling of these configurations much better, less surprising?

@m0vie
Copy link
Author

m0vie commented Feb 20, 2022

No objections. Those definitely make sense and improve the SNI behavior.

The behavior for initial/non-SNI connections is a different issue.

@m0vie
Copy link
Author

m0vie commented Feb 20, 2022

While it might be ugly, based on the first-match-wins implementation, here's what you would currently need to use:

<VirtualHost 111.111.111.111>
	Port 21
       # No ServerAlias here; this will be the first vhost found for any connection that doesn't use `HOST`, TLS SNI, etc.
</VirtualHost>
<VirtualHost 111.111.111.111>
       Port 21
	ServerAlias first.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias second.domain.com
</VirtualHost>
<VirtualHost 111.111.111.111>
	Port 21
	ServerAlias *
</VirtualHost>

<VirtualHost 222.222.222.222>
      Port 21
      # No ServerAlias here; this will be the first vhost found for any connection that doesn't use `HOST`, TLS SNI, etc.
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias first.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias second.domain2.com
</VirtualHost>
<VirtualHost 222.222.222.222>
	Port 21
	ServerAlias *
</VirtualHost>

An alternative might to use something like mod_autohost.

This doesn't work.

With that config, using a connection with SNI

openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com

I end up on the first block (the one without ServerAlias).

Castaglia added a commit that referenced this issue Feb 20, 2022
…e ProFTPD internals, to maintain proper insertion order.
Castaglia added a commit that referenced this issue Feb 20, 2022
…oFTPD internals from the main branch to 1.3.7.
Castaglia added a commit that referenced this issue Feb 20, 2022
Issue #1369: Tuning the handling of IP- and name-based bindings in th…
Castaglia added a commit that referenced this issue Feb 20, 2022
Issue #1369: Backporting much of the IP- and name-based tunings of Pr…
@gjaekel
Copy link

gjaekel commented Mar 29, 2022

Dear TJ,
switching to any newer than v1.3.7a also breaks my installation using two VHosts at the same IP. While using sftp to login, it choose the wrong VHost, because it uses the wrong auth file and after login, the client is at the wrong data root.

In the other hand, re-emerging v1.3.7.a (it's Gentoo), after login I'm now touching #1111, i.e. users may need to choose another MAC, which isn't acceptable.

How to get something working in-between at least, please? May I cherry-pick #1181 ?

@gjaekel
Copy link

gjaekel commented Mar 29, 2022

OK, after carefully reading #1111 and #1181 I understand and may confirm that applying the patch solves the issue with the MAC caused by gcc (gcc version 10.3.0 (Gentoo 10.3.0-r2 p3)). From this, I have a usable workaround by falling back on v1.3.7a.

But we have to solve the wrong vhost issue for >=v1.3.8 here; do you need any information from me to work on?

@Castaglia
Copy link
Member

Castaglia commented Mar 29, 2022

@gjaekel Can you provide the ProFTPD config you're using, so that I can better understand the issue you are seeing? I'm not sure I understand what you mean by:

have to solve the wrong vhost issue for >= v1.3.8

Also, SSH/SFTP connections are purely based on IP address/port; there is no name-based selection involved, as SSH does not have the protocol equivalent of TLS SNI or HTTP Host headers. Thus I'd like to better understand just how SFTP connections would be impacted, with your ProFTPD configuration, using ProFTPD 1.3.7b or later.

@gjaekel
Copy link

gjaekel commented Mar 30, 2022

Also, SSH/SFTP connections are purely based on IP address/port; there is no name-based selection involved, as SSH does not have the protocol equivalent of TLS SNI or HTTP Host headers.

TLTR: Yes - It can't work by definition. By sharing one IP for more than one vhost, we did a fundamental mistake

I remember and realize this over last hours, too. And therefore, I double-checked the issue facts reported to me. The solution is lame: For any reason, the different versions of proftpd evaluate the clashing vhost definitions in different order. Therefore, either the one or the other vhost is usable. The "missleading point" was that there is the same account with identical credentials used for both vhost. And in former times, we seems just to verify that to procedure of login, but not the data that is available afterwards. It never had have worked in the suggested way, I conclude.

And for historical reasons, in the production setup we use different IPs. Therefore this mistake never get eye-catching. BTW: Using ProFTPd, we only offer ssh-ftp/cp (and also WebDAV via an Apache httpd on the same domains sharing the same accounts file).

Sorry for the rumor.

@Castaglia
Copy link
Member

@gjaekel No worries, and thanks for the update. When it comes to address/port "collisions" (overuse) in the ProFTPD config file, over the years I've wavered between having ProFTPD refuse to start up all for such misconfigurations, or logging about the collision (and thus having these logs easily missed by administrators) and proceeding on. Currently, it's the latter behavior, which I felt to be less surprising, more admin-friendly -- but at the cost of missing such misconfigurations.

Hopefully, with the work being done in this ticket, the order in which vhosts are evaluated by ProFTPD will be more stable/consistent.

@Castaglia
Copy link
Member

@m0vie when you say

With that config, using a connection with SNI

openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com

I end up on the first block (the one without ServerAlias).

How are you verifying that you end up on the first vhost, the one that lacks the ServerAlias? In my local testing, using that config, the initial TCP connection ends up on the first vhost (as expected, with first-match-wins using only the TCP destination address/port to match that first 127.0.0.1:21 vhost), but the TLS handshake, using SNI first.domain.com, properly and successfully uses the first.domain.com vhost...

Castaglia added a commit that referenced this issue Apr 3, 2022
…as` directive, but does not have an explicit `ServerName`, then use the first `ServerAlias` as the `ServerName`.
@Castaglia Castaglia added this to the 1.3.8 milestone Apr 3, 2022
Castaglia added a commit that referenced this issue Apr 4, 2022
Issue #1369: If we encounter a <VirtualHost> that uses the `ServerAli…
@Castaglia
Copy link
Member

@m0vie @gjaekel I'd like to determine what's left to do on this ticket (code changes, doc changes, etc), since this is the last ticket before I do another release. Any updates/thoughts/improvements to be done, from your perspective?

@gjaekel
Copy link

gjaekel commented Apr 13, 2022

@Castaglia Very prudent!

In sum I tracked down an already existing conceptual mistake by our own and while investigating this, I stepped into this special gcc issue (#1111, #1181). And shot myself in the foot by having the same credentials for a test account on different vhosts. Nothing of this was caused by ProFTP itself in the root.

You may take this as a proof that it might be better to refuse startup on misconfigurations for some future release. There are more and more unattended updates happen by orchestration or similar, where nobody will read logs to discover "silent errors".

@m0vie
Copy link
Author

m0vie commented Apr 13, 2022

I think 8a186da fixed things for me:

<VirtualHost 127.0.0.1>
	Port 21
	TLSRSACertificateFile /etc/certs/_dummy_catch_all.crt
	TLSRSACertificateKeyFile /etc/certs/_dummy_catch_all.key
	AuthUserFile /etc/proftpd/authfiles/dummy.passwd
	AuthGroupFile /etc/proftpd/authfiles/dummy.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias first.domain.com
	TLSRSACertificateFile /etc/certs/first.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/first.domain.com.key
	TLSCACertificateFile /etc/certs/first.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/first.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/first.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias second.domain.com
	TLSRSACertificateFile /etc/certs/second.domain.com.cert.crt
	TLSRSACertificateKeyFile /etc/certs/second.domain.com.key
	TLSCACertificateFile /etc/certs/second.domain.com.ca.crt
	AuthUserFile /etc/proftpd/authfiles/second.domain.com.passwd
	AuthGroupFile /etc/proftpd/authfiles/second.domain.com.group
</VirtualHost>

<VirtualHost 127.0.0.1>
	Port 21
	ServerAlias *
	TLSRSACertificateFile /etc/certs/_dummy_catch_all.crt
	TLSRSACertificateKeyFile /etc/certs/_dummy_catch_all.key
	AuthUserFile /etc/proftpd/authfiles/dummy.passwd
	AuthGroupFile /etc/proftpd/authfiles/dummy.group
</VirtualHost>
$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername first.domain.com | grep CN
...
0 s:CN = first.domain.com
...

$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername second.domain.com | grep CN
...
0 s:CN = second.domain.com
...

$ openssl s_client -connect 127.0.0.1:21 -starttls ftp -servername another.domain.com | grep CN
...
0 s:CN = dummy_catchall
...

$ openssl s_client -connect 127.0.0.1:21 -starttls ftp | grep CN
...
0 s:CN = dummy_catchall
...

This is exactly what I'd expect and things looks deterministic now.

@Castaglia Castaglia changed the title [1.3.8 / 1.3.7b] ServerAliases / <VirtualHost> broken ('no matching server found for client-sent SNI') (caused by #1105 / #1109) Name-based virtual hosts not working as expected after upgrade from 1.3.7a to 1.3.7b Apr 13, 2022
@rlaager
Copy link

rlaager commented Apr 16, 2022

I tested with 8a186da backported on top of 1.3.7c and things seem to be good for me (re: issue #1105 that came back in the 1.3.7 branch).

To be clear, I have not tested 1.3.7c without 8a186da (because at this point, why), nor have I tested master/8a186da directly (because that's non-trivial for me to do in production).

@rlaager
Copy link

rlaager commented Apr 21, 2022

I tested 1.3.7c without 8a186da (because this determines whether I need a straight backport or to carry the patch). I confirmed 1.3.7c without 8a186da is broken for me. So 8a186da definitely fixes my issue (again).

@Castaglia
Copy link
Member

Considering this ticket addressed; we'll use new tickets for any other issues that come up. Thanks, all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants