You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "rnewson (via GitHub)" <gi...@apache.org> on 2023/01/22 17:17:46 UTC

[GitHub] [couchdb-pkg] rnewson opened a new pull request, #106: Use control character in sed lines with variables

rnewson opened a new pull request, #106:
URL: https://github.com/apache/couchdb-pkg/pull/106

   A cookie value with a '/' in it caused a sed error during postinst;
   
   ```
   sed: -e expression #1, char 53: unknown option to `s'
   ```
   
   We use a control character (RS - record separator) instead of / to reduce the chances of a collision with a valid cookie string.
   
   I applied the same change to nodename even though a / in node name would be an error, for consistency.
   
   <!-- Thank you for your contribution!
   
        Please file this form by replacing the Markdown comments
        with your text. If a section needs no action - remove it.
   
        Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
        of code collaboration. Positive feedback is represented +1 from committers
        and negative is a -1. The -1 also means veto, and needs to be addressed
        to proceed. Once there are no objections, the PR can be merged by a
        CouchDB committer.
   
        See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
   
   ## Overview
   
   <!-- Please give a short brief for the pull request,
        what problem it solves or how it makes things better. -->
   
   ## Testing recommendations
   
   <!-- Describe how we can test your changes.
        Does it provides any behaviour that the end users
        could notice? -->
   
   ## GitHub issue number
   
   <!-- If this is a significant change, please file a separate issue at:
        https://github.com/apache/couchdb-pkg/issues
        and include the number here and in commit message(s) using
        syntax like "Fixes #472" or "Fixes apache/couchdb#472".  -->
   
   ## Related Pull Requests
   
   <!-- If your changes affects multiple components in different
        repositories please put links to those pull requests here.  -->
   
   ## Checklist
   
   - [ ] Code is written and works correctly;
   - [ ] Changes are covered by tests;
   - [ ] Documentation reflects the changes;
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] rnewson commented on pull request #106: Use control character in sed lines with variables

Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106#issuecomment-1400102556

   you can specify a cookie with spaces if you do so correctly;
   
   ```
   -setcookie 'foo bar baz'
   ```
   
   Just the same as you would have to single quote that atom in erlang code itself.
   
   My modification at least allows the use of `/` and many other characters (including space) that previously might have been issues, so this is an improvement.
   
   I am not sure how to do the same on the RPM side as it appears not to be a shell script as such. I will try, however.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] nickva commented on pull request #106: Use control character in sed lines with variables

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106#issuecomment-1399575293

   The rpm side of things could have the problem: https://github.com/apache/couchdb-pkg/blob/c928bd6cce7ae05ad8b177afe5e6b628aaf378ee/rpm/SPECS/couchdb.spec.in#L156


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] nickva commented on pull request #106: allow more characters in cookie

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106#issuecomment-1401532175

   Testing with commit 567d1c4b3d9c4baa784957110ed7690f16131cae on a Debian Buster VM:
   
   I see `\n` escaped properly but some control characters get through. If users can type in `\` as one of the characters they could generate any of the control characters.
   
   cookie: `a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f`
   `-setcookie 'a b\n       ^M#{}()[]$&^!-=+?|//c\d\e\\f'`
   
   another example
   
   cookie: `a\n\t\ \x\\y\\\z//w///`
   `-setcookie 'a\n	 x\y\z//w///'`
   
   
   RPM test on CentOS 7
   
   It seems the replacement had stopped working both for the user supplied cookie in a variable or for the randomly generated one:
   
   ```
   [root@nvcentos7 ~]# export COUCHDB_COOKIE='a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f'
   [root@nvcentos7 ~]# echo ${COUCHDB_COOKIE}
   a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f
   [root@nvcentos7 ~]# rpm -i couchdb-3.3.1.1.1-1.el7.x86_64.rpm 
   Using defined COUCHDB_COOKIE value.
   ```
   ```
   # All nodes must share the same magic cookie for distributed Erlang to work.
   # Uncomment the following line and append a securely generated random value.
   # -setcookie
   ```
   
   For random generated one:
   ```
   [root@nvcentos7 ~]# rpm -i couchdb-3.3.1.1.1-1.el7.x86_64.rpm 
   Generating random cookie value.
   [root@nvcentos7 ~]# grep setcookie /opt/couchdb/etc/vm.args 
   # -setcookie
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] rnewson merged pull request #106: allow more characters in cookie

Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson merged PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] nickva commented on pull request #106: Use control character in sed lines with variables

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106#issuecomment-1399874968

   Setting the cookie worked. I rebuilt debian-buster deb with it and installed in a VM.
   
   deb install dialog:
   ```
   CouchDB Erlang magic cookie: a \nb#$/c/d@:1
   ```
   
   vm.args file
   ```
   -setcookie 'a 
   b#$/c/d@:1'
   ```
   CouchDB seems to start and I can access it via curl.
   
   However, unfortunately the single quotes seem to break our remsh script. 
   
   ```
   couchdb@debian10:~$ ./bin/remsh 
   Erlang/OTP 24 [erts-12.3.2.7] [source] [64-bit] [smp:2:2] [ds:2:2:10] [async-threads:1] [jit]
   
   *** ERROR: Shell process terminated! (^G to start new job) ***
   
   ```
   
   Likely due to assumptions how we parse the cookie value there https://github.com/apache/couchdb/blob/3181d928e060687e2a214192ba17c401811c6da3/rel/overlay/bin/remsh#L50-L57
   
   ```
   ARGS_FILE_COOKIE=$(awk '$1=="-setcookie"{print $2}' "$ARGS_FILE")
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [couchdb-pkg] nickva commented on pull request #106: allow more characters in cookie

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on PR #106:
URL: https://github.com/apache/couchdb-pkg/pull/106#issuecomment-1403154163

   Testing with [1b18b7c](https://github.com/apache/couchdb-pkg/commit/1b18b7cc18daf160c1618c203de0e6dfc616ccdf)
   
   RPM on CentOS 7 works as expected:
   
   Random new cookie:
   ```
   [root@nvcentos7 ~]# rpm -i couchdb-3.3.1.1.1-1.el7.x86_64.rpm 
   Generating random cookie value.
   [root@nvcentos7 ~]# grep setcookie /opt/couchdb/etc/vm.args 
   -setcookie '2MJoDeihWIblBBbSjiHAP5efEr5t8UoTCoiNLNBgf5Ju3Ry8'
   ```
   
   Custom cookie:
   ```
   [root@nvcentos7 ~]# export COUCHDB_COOKIE='a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f'
   [root@nvcentos7 ~]# rpm -i couchdb-3.3.1.1.1-1.el7.x86_64.rpm 
   Using defined COUCHDB_COOKIE value.
   [root@nvcentos7 ~]# grep setcookie /opt/couchdb/etc/vm.args 
   -setcookie 'a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f'
   ```
   
   remsh didn't work but that's expected, I re-wrote bin/remsh to use a similar vm.args file with just the `-setcookie` to be able to log in to verify that the cookie was set instead of being silently replaced with a weak default one:
   
   ```
   > erlang:get_cookie().
   'a b\\n\\t\\xd#{}()[]$&^!-=+?|//c\\\\d\\\\\\e\\\\\\\\f'
   
   > io:format("~s~n", [erlang:get_cookie()]).
   a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f
   ok
   ```
   
   Works on Debian Buster
   
   Install dialog: `a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f`
   
   ```
   sudo grep setcookie /opt/couchdb/etc/vm.args
   -setcookie 'a b\n\t\xd#{}()[]$&^!-=+?|//c\\d\\\e\\\\f'
   ```
   
   We'll have to fix remsh cookie parsing in the main repo before the next release. But cookie prompt and replacement in this repo works well. Thanks for the fix!
   
   +1
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org